BitGo / eth-multisig-v2

Multi-Sig Wallet v2, supporting original Wallet.sol methods with additional confirmAndExecute improvements to allow for single-transaction signing by multiple owners.
Apache License 2.0
271 stars 101 forks source link

Usability issue with sequence ids #11

Open justinbretting opened 5 years ago

justinbretting commented 5 years ago

This PR addresses an issue where broadcasting a tx with a sequence id exactly 10,000 higher than the lowest value in recentSequenceIds will result in subsequent calls to getNextSequenceId returning an unusable sequenceId because of the final check in tryInsertSequenceId.

To verify, reference the branch in https://github.com/justinbretting/eth-multisig-v2/pull/1, which shows how running npm run truffle-test will fail after a tx is successfully broadcast as described above.

Note that even in this case, funds are not locked. However, the user(s) will need to have advanced knowledge of prior transactions executed against the contract to understand the current state of recentSequenceIds so a usable seq id can be used.