Give clients on the wallet a way to get pending transactions

[bencxr]

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

[frozeman]

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

[bencxr]

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

[gavofyork]

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

[frozeman]

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.

[bencxr]

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.

[veox]

I feel the extra line was for visibility. Check line 50 above.

[bencxr]

alright, i'll leave it in

[veox]

m_pendingIndex.length is a dynamically-sized array. i would be an uint8 (as per the docs).

If m_pendingIndex.length > 255, this is an endless loop.

I haven't looked through all the dapp, but can it be guaranteed that this won't end in always-OOG?

[bencxr]

this is a uint

[veox]

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

[veox]

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

[bencxr]

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

[chriseth]

I think it would be better readable if you named the return parameters. Then it is also much harder to make a mistake an the actual return instruction.

[bencxr]

I named the return parameters.. but more for documentation than to replace 391.

[vbuterin]

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.

[chriseth]

@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.

[bencxr]

@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.