Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERC: Standard API for multisig wallet smart contracts #763

Closed
3sGgpQ8H opened this issue Nov 13, 2017 · 27 comments
Closed

ERC: Standard API for multisig wallet smart contracts #763

3sGgpQ8H opened this issue Nov 13, 2017 · 27 comments
Labels

Comments

@3sGgpQ8H
Copy link

3sGgpQ8H commented Nov 13, 2017

Standard API for multisig wallet smart contracts

This ERC proposes standard API for multisig wallets Ethereum smart contracts.

1. Motivation

Multisig wallets are widely-used class of Ethereum smart contracts. Multisig wallet is a smart contract that allows a group of people to collectively own Ethereum address and to execute transactions from it. This addresses know problem of plain Ethereum addresses controlled by private keys, where private key becomes single point of failure.
Currently, there is no common API nor implementation for multisig wallets. Being an owner of multiple multisig wallets with different APIs is a hard job, because one needs to use different tools/calls for the same operations on different wallet. This is inconvenient and error-prone. Standard API will make it possible to create convenient and secure tools one could make to deal with all his multisig wallets in the same way.

2. Use Cases

Proposed API supports two main use cases named “Public Flow” and “Private Flow”. Both use cases have the same roles: Suggester, Approvers, Executor, and the same goal: execute transaction via multisig wallet.

2.1. Public Flow

  1. Suggester suggest transaction to be executed
  2. Suggester reveals transaction details on chain, so everybody may see it
  3. Approvers approve transaction
  4. Executor executer transaction from multisig address

2.2. Private Flow

  1. Suggester suggest transaction to be executed
  2. Suggester sends transaction details to Approvers and Executor via private channels
  3. Approvers approve transaction
  4. Executor executer transaction from multisig address

3. Methods

We propose the following methods to be included into API standard:

3.1. suggest(bytes32)

Signature:
function suggest (bytes32 _hash) returns (uint256 id)
Description:

Suggest transaction with given hash to be executed.
Returns unique ID of suggested transactions.
Logs “Suggestion” event.
Reverts if transaction was not suggested, e.g. when _hash is zero or caller is not authorized to suggest transactions.
Hash is calculated via the following Solidity statement:

keccak256 (to, value, data, salt)

Here:

  • to – transaction destination address (address)
  • value – transaction value in Wei (uint256)
  • data – transaction data (bytes)
  • salt – zero for public flow, arbitrary number for private flow (uint256)

Hash may not be zero.
ID of suggested transactions are never reused.

3.2. approve(uint256)

Signature:
function approve (uint256 _id)
Description:

Approve pending suggested transaction with given ID.
Logs “Approval” event.
Reverts if transaction was not approved, e.g. when _id is not a valid ID of pending suggested transaction, caller is not authorized to approve transaction or caller has already approved this transaction.

3.3. revokeApproval(uint256 _id)

Signature:
function revokeApproval (uint256 _id)
Description:

Revoke approval from suggested transaction with given ID.
Logs “ApprovalRevocation” event.
Reverts if approval was not revoked from transaction, e.g. when _id is not a valid ID of pending suggested transaction, caller is not authorized to revoke approvals or called has not approved this transaction yet.

3.4. execute(uint256,address,uint256,bytes,uint256)

Signature:
function execute (uint256 _id, address _to, uint256 _value, bytes _data, uint256 _salt)
returns (bool success)
Description:

Execute suggested transaction with given ID and details.
Returns true if transaction was executed successfully, false if execution failed.
Logs “Execution” event.
Reverts if transaction was not executed, e.g. when _id is not a valid ID of pending suggested transaction, transaction didn't collect enough approvals yet, transaction details do not match hash passed to “suggest” method or caller is not authorized to execute transactions.

3.5. reveal(uint256, address, uint256, bytes)

Signature:
function reveal(uint256 _id, address _to, uint256 _value, bytes _data)
Description:

Reveal details of suggested transaction with given ID by logging “Revelation” event.
Logs “Revelation” event.
Reverts if transaction details were not revealed, e.g. when _id is not a valid ID of pending suggested transaction, transaction details (assuming zero salt) do not match hash passed to “suggest” method or caller it not authorized to reveal transaction details.
Note, that details of transactions whose hash was calculated with non-zero salt cannot be revealed.

3.6. getHash(uint256)

Signature:
function getHash (uint256 _id) constant returns (bytes32 hash)
Description:

Get hash of details of suggested transaction with given ID.
Reverts if _id is not a valid ID of pending suggested transaction.

3.7. isApproved(uint256)

Signature:
function isApproved (uint256 _id) constant returns (bool approved)
Description:

Tells whether suggested transaction with given ID has collected enough approvals to be executed.
Reverts if _id is not a valid ID of pending suggested transaction.

3.8. isApprovedBy(uint256,address)

Signature
function isApprovedBy (uint256 _id, address _owner) constant returns (bool approved)
Description

Tells whether suggested transaction with given ID has been approved by given owner.
Reverts if _id is not a valid ID of pending suggested transaction.

4. Events

We propose the following events to be included into API standard:

4.1. Suggestion(uint256,address,bytes32)

Signature:
event Suggestion (uint256 indexed id, address indexed owner, bytes32 hash)
Description

Logged when transaction with given ID and hash of details was suggested by given owner.

4.2. Approval(uint256,address)

Signature:
event Approval (uint256 indexed id, address indexed owner)
Description:

Logged when transaction with given ID was approved by given owner.

4.3. ApprovalRevocation(uint256,address)

Signature:
event ApprovalRevocation (uint256 indexed id, address indexed owner)
Description:

Logged when approval was revoked by given owner from transaction with given ID.

4.4. Execution(uint256,address,bool)

Signature:
event Execution (uint256 indexed id, address indexed owner, bool result)
Description:

Logged when transaction with given ID was executed by given owner and produced given result.

4.5. Revelation(uint256,address,address,uint256,bytes)

Signature:
event Revelation (
  uint256 indexed id,
  address indexed owner,
  address indexed to, uint256 value, bytes data)
Description:

Logged when given details (assuming zero salt) of transaction with given ID was revealed by given owner.

@3sGgpQ8H 3sGgpQ8H changed the title Standard API for multisig wallet smart contracts ERC: Standard API for multisig wallet smart contracts Nov 13, 2017
@Arachnid
Copy link
Contributor

Is it necessary to invent an entirely new API for this? Why not write it to conform to one or more of the existing implementations?

Also, personally I think the format that accepts a single TX with the signatures of a quorum of participants is a much better approach than having each participant individually submit an approval TX.

@3sGgpQ8H
Copy link
Author

@Arachnid

Why not write it to conform to one or more of the existing implementations?

Compatibility with existing implementations is surely good thing, but did you mean some particular existing implementation that has API good and generic enough to be promoted to EIP standard?

Also, personally I think the format that accepts a single TX with the signatures of a quorum of participants is a much better approach than having each participant individually submit an approval TX.

This makes it impossible to revoke signatures, and this requires owners to know each other's contact information and to communicate off-chain as in private flow described in my proposal, right?

@Arachnid
Copy link
Contributor

Compatibility with existing implementations is surely good thing, but did you mean some particular existing implementation that has API good and generic enough to be promoted to EIP standard?

There's a couple of others already. I don't understand why you wouldn't pick one instead of inventing something from scratch.

This makes it impossible to revoke signatures

I don't see this as an important feature, personally.

and this requires owners to know each other's contact information and to communicate off-chain as in private flow described in my proposal, right?

Yes - but we shouldn't restructure APIs around offchain coordination problems. I certainly think it's implausible that users of a multisig don't communicate with each other out of band.

@3sGgpQ8H
Copy link
Author

@Arachnid

There's a couple of others already. I don't understand why you wouldn't pick one instead of inventing something from scratch.

I think we probably could split this question into two:

  1. Do we actually need to standardize multisig wallet API at all?
  2. If answer to the first question is yes, should we just pick some existing API already implemented in significant number of existing multisig wallets and probably already supported by existing tools, or should we design new API?

In my opinion, answer to the first question is “yes” and my motivation is explained in “Motivation” section above.
Also, in my opinion, it is not possible to give generic answer to the second question, because “right” answer greatly depends on what APIs are implemented by existing wallets and as supported by existing tools.
If you agree that answer to the first question is yes, and you know existing multisig wallet API that could be picked as a standard, you probably need to propose this API to be standardized and start discussion about this.
I just don't know such API, that may be standardized as is, and, from compatibility point of view, there is no big difference between existing multisig wallet that is slightly incompatible with the standard, and multisig wallet that uses completely different API. Anyway such wallet will be incompatible with tools that rely on the standard, so, if existing API cannot be standardized as is, it does not make much sense for me to keep standard by any means close to such existing APIs. Of cause there is also no reason to make is deliberately different.

@3sGgpQ8H
Copy link
Author

@Arachnid

I don't see this as an important feature, personally.

Standard multisig wallet deployed by Mist do have such feature. Not sure how widely it is used, but at least authors of standard wallet probably saw some reason to include it.

Yes - but we shouldn't restructure APIs around offchain coordination problems. I certainly think it's implausible that users of a multisig don't communicate with each other out of band.

The actual question is whether should we be restrictive about on-chain coordination, i.e. should we forbid on-chain coordination flow (which is currently default flow for standard multisig wallet deployed by Mist) at API level? Multisig wallets coordinated on-chain will anyway exist, and EIP standard cannot change this fact. If standard will not support on-chain coordination, all such wallets will just be non-standard. In my understanding, the goal of the standard is to improve compatibility between smart contracts and tools, not to force people to use approaches preferred by the authors of the standard.

@Arachnid
Copy link
Contributor

Contract authors and users will use the approach recommended by an EIP. You cannot ignore the impact an approved standard will have on common practice when deciding how to author it.

@3sGgpQ8H
Copy link
Author

Did I get right, that in your opinion API for multisig wallet that collects approvals on-chain should not be standardized, because right approach is to collect approvals off-chain as for bitcoin multisig wallets?

@Arachnid
Copy link
Contributor

Reasonable people can differ on whether onchain or offchain approvals are the better approach. What I'm saying is that you shouldn't conclude you should do it one way or the other based entirely on what competing implementations would exist. Write the best standard you can and encourage people to adopt it.

@3sGgpQ8H
Copy link
Author

I see. Anyway, multisig wallets that collect approvals off-chain are completely out of scope for this proposal, because I didn't find elegant way to support both, on-chain and off-chain approvals collecting in one single API. To support both, public and private flows, was already challenging enough.
I totally agree, that bitcoin-style multisig wallets that collect approvals completely off-chain are useful, despite they are rarely encountered these days, and probably their API is worth standardizing as well.

@alex-miller-0
Copy link

I would very much like to see a multisig API standard, but I think this one is too complex. I recommend Christina Lundkvist's simple-multisig implementation, which has instantiation as this:

function SimpleMultiSig(uint threshold_, address[] owners_)

and exactly one function:

function execute(uint8[] sigV, bytes32[] sigR, bytes32[] sigS, address destination, uint value, bytes data)

I think there is real value to standardizing an interface so that wallet providers can easily add multisig functionality (I think we all need more multisigs in our lives), but IMO standardizing other features at this stage is dangerous.

@3esmit
Copy link
Contributor

3esmit commented Nov 15, 2017

I think the standard API should be for "Multi-signature" transaction authorization, and should only include the methods regarding this, and the management of owners can be set to other EIP, because we can have multisigs that are not necessary to be owner's changeable.
This EIP is related to #745

@3sGgpQ8H
Copy link
Author

@alex-miller-0

I recommend Christina Lundkvist's simple-multisig implementation

I agree that such wallets that collect signatures completely off-chain are useful and it does make sense to standardize API for such wallets, though discussion about such API is out of scope in this proposal, because this proposal is about different kind of multisig wallets.

About API of Christina's wallet, it seems that the way how it prevents transaction re-execution effectively does not allow collecting signatures for several transactions in parallel, limiting usages of such wallet to the following two cases:

  1. All owners sit in the same room
  2. Transactions are executed very rarely, i.e. average time between transactions is much bigger than average time needed to collect signatures for single transaction

While these two cases are not uncommon and Christina's wallet will definitely be found useful by many teams, limitations mentioned above probably prevent Christina's wallet API from becoming common standard.

@3sGgpQ8H
Copy link
Author

3sGgpQ8H commented Nov 15, 2017

@3esmit

I think the standard API should be for "Multi-signature" transaction authorization, and should only include the methods regarding this, and the management of owners can be set to other EIP

Totally agree, and proposed API is just of this kind.

@alex-miller-0
Copy link

limiting usages of such wallet to the following two cases

I don't see the design as limited by those use cases at all. Once a message is signed, it can be passed between wallets (a.k.a. second-layer applications) over http. Transactions could also be executed autonomously by your software if you have codified transfer limits and authorized co-signers.

Again, standards should be the lowest level possible. On-chain approval functions are an unnecessary (and expensive) complexity IMO.

@3sGgpQ8H
Copy link
Author

3sGgpQ8H commented Nov 15, 2017

@alex-miller-0

I don't see the design as limited by those use cases at all.

This may not be obvious, but these limitations are (probably unintended) consequences of how the API prevents signed transaction from being executed more than once. The wallet assigns sequentially incremented nonce value to every transaction being executed, and this value is a part of transaction data that ought to be signed. So you have to know transaction nonce when you are signing it, while nonce is assigned only when transaction is executed. Of cause it is impossible to know transaction nonce fore sure before transaction has been executed, so you have to guess. And if you are wrong, you will have to start over collecting signatures.
Obvious strategy for guessing nonce is to use nonce of the latest executed transaction plus one, but this works well only if there is at most one pending transaction at any given time moment.
If there are some pending transactions already and you want to start collecting signatures for yet another one, you basically have two choices: either to use highest nonce among pending transactions plus one, or to ignore pending transactions and use nonce of latest executed transaction plus one. Both ways are bad. The former will fail if any of existing pending transactions will not be approved or will take too much time to be approved. The latter will fail, if another transaction with the same nonce as you picked for your transaction, will be executed ahead of your transaction.
So, if there are several transactions collecting signatures in parallel, the wallet becomes ineffective.
Thus, to use it effectively all owners have to sit in one room to be able to resolve collisions quickly, or transactions should be executed rarely enough to make collisions improbable.
Otherwise, at the time you finally collected enough signatures for your transaction, you will probably realize, that nonce you picked for your transaction is either already used by another executed transaction, or not yet reached, so you have to either start over, or wait for unpredictable amount of time, or convince owners to approve enough no-op transactions to pump up nonce to desired value.

@3sGgpQ8H
Copy link
Author

@Arachnid

Contract authors and users will use the approach recommended by an EIP. You cannot ignore the impact an approved standard will have on common practice when deciding how to author it.

Standards surely impact people, that's why standard author should be very careful and should not use his power to force other people to share standard author's personal preferences. There are too many cases already when people use standard approach just because it is standard, while personally they don't like it and there are better approaches widely known.

@khovratovich
Copy link
Contributor

@Arachnid

This makes it impossible to revoke signatures

I don't see this as an important feature, personally.

Suppose that an owner submits a transaction by mistake. If he can not revoke his approval, the transaction remains in circulation forever, and the effective threshold of the multisig wallet will be one owner smaller.

@3sGgpQ8H
Copy link
Author

3sGgpQ8H commented Nov 16, 2017

@Arachnid

Suppose that an owner submits a transaction by mistake.

I agree that transactions submitted by mistake are not a problem because such transactions will never collect enough signatures. Actually I've never meant such scenario when I was telling about revocation as a valuable feature. For me, usual scenario where revocation would be helpful looks like this:

  1. Lets assume there is 2-of-3 multisig wallet collectively owned by Alice, Bob and Carol
  2. Alice suggests transaction and signs it
  3. The transaction appears to be profitable for Alice, but not for Bob nor Carol, so it fails to collect enough signatures
  4. Long time passes and circumstances changed drastically
  5. Now that transaction, suggested by Alice long time ago, is profitable for Bob, but unacceptable for Alice and Carol
  6. Bob signs the transaction and executes it

With revocation feature available, Alice would revoke her signature either after realizing that her transaction has no chances to be approved, or after realizing that her transaction is not profitable for her anymore.

People do change their opinions and circumstances do change, so I would prefer to be able to revoke my signature from not yet executed transaction if I changed my mind. Especially this is important for controversial transactions that fail to quickly collect enough signatures and may stuck in half-signed state forever.

@stefek99
Copy link

#763 (comment)

we can have multisigs that are not necessary to be owner's changeable

I believe that by default multisigs should allow to change owners.

I was doing some research / reading / education and collected some links in a blogpost: https://steemit.com/ethereum/@genesisre/ethereum-multisig-wallet-interacting-with-contracts-on-behalf-of-multisig

(we are in such early days)

@elenadimitrova
Copy link

elenadimitrova commented Feb 13, 2018

Just did some comparative gas cost calculations between SimpleMultiSig and MultiSigWallet for reference:
SimpleMultiSig only ever needs one transaction (which contains all signatures)

multisig.execute -> 75,413 gas (for 2 signatures)
                 -> 84,949 gas (for 3 signatures)

MultiSigWallet needs an initial transaction submission and then as many confirmations as required, i.e. the confirmTransaction 50,110 gas cost should be multiplied by the required number of approvals -1.

multisig.submitTransaction -> 167,202 gas
multisig.confirmTransaction -> 50,110 gas (not enough signatures gathered)
multisig.confirmTransaction -> 79,857 gas (enough signatures gathered, execute the tx)

@alex-miller-0
Copy link

@elenadimitrova Cool stats. How many signatures went into the 75k SimpleMultiSig cost? Every additional signature should require ~3k gas due to the ecrecover plus a few computations.

@elenadimitrova
Copy link

@alex-miller-0 you're right, that test was using 2 signatures so I updated to 3 and cost went up with ~10,000 gas. Updated the stats to reflect that.

@3esmit
Copy link
Contributor

3esmit commented Feb 14, 2018

I think that offchain ethereum signed messages can be included in the same function, that would call internal methods that are the same used by the regular ethereum signed transaction calls to the contract, that fill _from with msg.sender instead of recovered address, and this can be part of another ERC.
I think this ERC should coordinate only in defining the common method calls, if you need offchain signed ethereum messages, then you should implement other interface for doing it. This same interface could be used for other types of contracts that want to accept ethereum signed messages. See this example https://gist.github.com/3esmit/cbfa213c87c151334f819864dce47d2c
one file is just a simple multisig (does just basic multisigning stuff)
other just implement the presigning feature.

@alex-miller-0
Copy link

alex-miller-0 commented Feb 14, 2018

@elenadimitrova very interesting that it would take 10k gas for an extra signature since ECRECOVER should only be 3000 gas (source). I suspect recasting lastAdd is making up the difference.

@harish-narayanasamy
Copy link

Why the method Sendmultisig is listing the transaction as internal transaction?

@github-actions
Copy link

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Dec 20, 2021
@github-actions
Copy link

github-actions bot commented Jan 3, 2022

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

@github-actions github-actions bot closed this as completed Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants