cairo-contracts icon indicating copy to clipboard operation
cairo-contracts copied to clipboard

Verify signer control on `setPublicKey`

Open martriay opened this issue 1 year ago • 1 comments

Fixes #469, #818.

I propose not adding a 2step flow, but to add a signature: Span<felt252> param to setPublicKey (*) used to validate that we control the new owner, and that it "accepts" this ownership -- all in a single step.

To make this more user-friendly, i'm using SNIP-12 (aka Starknet's EIP712) and defining the AddOwner operation for the Account.add_owner application.

#[derive(Copy, Drop, Serde)]
struct AddOwner {
    account: ContractAddress,
    owner: felt252
}

Even though the account is already present in the hash as required by SNIP-12 and that owner is already known by the owner, I decided to make the struct overly explicit so users know what they're signing, even if it adds a bit of cost/redundancy/complexity.

If we move forward with this proposal, we should probably tackle #409 first.

*: i'd probably go even further an rename it set_owner, since that's the term we're using in the OwnerAdded and that I'm using here too.

martriay avatar Feb 22 '24 02:02 martriay

Maybe it's worth considering having a 2step mechanism anyway, since in the current design the flow requires a bit of coordination between old and new owner:

  1. new owner signs acceptance message
  2. new owner sends it to old owner
  3. old owner crafts and sends the tx to the account

While if it's two step:

  1. former owner proposes new owner
  2. new owner accepts ownership

martriay avatar Feb 22 '24 02:02 martriay

Closing in favor of #989

ericnordelo avatar May 14 '24 11:05 ericnordelo