dapp-bin icon indicating copy to clipboard operation
dapp-bin copied to clipboard

Give clients on the wallet a way to get pending transactions

Open bencxr opened this issue 8 years ago • 11 comments

Give clients on the wallet a way to get the pending transactions that require the confirmation of other signers on the wallet.

New methods to help deal with multi user transactions: getPendingConfirmationsNeeded(bytes32 _operation) returns (uint) numPendingTransactions() returns (uint) getPendingTransaction(uint _index) returns (bytes32) getPendingTransactionToAddress(bytes32 _operation) returns (address) getPendingTransactionValue(bytes32 _operation) returns (uint) getPendingTransactionData(bytes32 _operation) returns (bytes)

Example usage and tests: https://github.com/BitGo/eth-multisig-v2/blob/master/test/wallet.js#L60

bencxr avatar Jun 14 '16 20:06 bencxr

Couldn't you simply return multiple return parameters and reduce this to two functions? In web3.js they will be returned as array of values, but you could name them. In the future we will allow named return parameters in web3.js

frozeman avatar Jun 30 '16 08:06 frozeman

you are right and i have made the change as you suggested!

bencxr avatar Jul 05 '16 08:07 bencxr

logs can be used to achieve the same thing without the extra chain-side code.

gavofyork avatar Aug 01 '16 18:08 gavofyork

Yes, but this way is prone to errors, needs complex log matching with chain-reorg detection and doesn't work if you only have the current state.

I worked with your contract since a year+ and build the wallet on top of it. These functions are exactly missing.

Matching logs is especially bad if you have the same operation hash in multiple logs. You can't know which one was already confirmed and which not. Trust me it's not fun building interfaces like this.

frozeman avatar Aug 01 '16 18:08 frozeman

I would add also that good interfaces to get previous logs (by event type) do not exist as far as I looked. This puts a burden of storing state on the wallet developer.

We should strive to make this contract interface intuitive to use for the developer to build on top of.

bencxr avatar Aug 01 '16 20:08 bencxr

Sorry for the barrage of copy-paste comments. Tired ATM.

veox avatar Aug 02 '16 08:08 veox

I can swear I saw var i in those loops y-day. @bencxr, did you rewrite history, or was I delusional?

veox avatar Aug 03 '16 19:08 veox

ok, addressed your comments. this diff uses uints, but I do have another with some var usage. thanks for your input though.

bencxr avatar Aug 08 '16 08:08 bencxr

I'll echo gav's comment that logs are a nicer approach. Multisig wallets should be a very universal tool that is eventually the type of account used by almost everyone; imo it's not acceptable to give them 100k gas of overhead.

vbuterin avatar Aug 24 '16 08:08 vbuterin

@vbuterin especially if multisig wallets are tools that are used by almost everyone, they can be used together with DELEGATECALL at basically no cost. From all our experience, logs are extremely unreliable, but even if that is fixed, I would say it is much safer to just add some query methods. Building up a copy of the current state through deltas from logs in the frontend is just not the same thing as just asking for the current state that is anyway present in the backend.

chriseth avatar Aug 24 '16 10:08 chriseth

@vbuterin I feel that @chriseth has made a good point on the DELEGATECALL. We use solc --clone over here and so far are very pleased with the result, huge amount of savings over our wallets. I wonder if we should have some way for all dapps that are accepted / merged in here to be deployed and addresses listed so that other contract browsers / creators can re-use the space.

bencxr avatar Aug 25 '16 23:08 bencxr