Multisig

[immrsd]

Fixed #21

• [x] Tests
• [ ] Documentation
• [x] Added entry to CHANGELOG.md
• [ ] Tried the feature on a public network

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 95.70552% with 7 lines in your changes missing coverage. Please review.

Project coverage is 92.02%. Comparing base (02c9ce6) to head (a7c569e). Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/governance/src/multisig/multisig.cairo	96.47%	5 Missing :warning:
...ckages/governance/src/multisig/storage_utils.cairo	85.71%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1193 +/- ## ========================================== + Coverage 91.56% 92.02% +0.46% ========================================== Files 47 49 +2 Lines 1233 1392 +159 ========================================== + Hits 1129 1281 +152 - Misses 104 111 +7 ``` | [Files with missing lines](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin) | Coverage Δ | | |---|---|---| | [packages/account/src/account.cairo](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?src=pr&el=tree&filepath=packages%2Faccount%2Fsrc%2Faccount.cairo&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin#diff-cGFja2FnZXMvYWNjb3VudC9zcmMvYWNjb3VudC5jYWlybw==) | `98.11% <ø> (ø)` | | | [.../governance/src/timelock/timelock\_controller.cairo](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?src=pr&el=tree&filepath=packages%2Fgovernance%2Fsrc%2Ftimelock%2Ftimelock_controller.cairo&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin#diff-cGFja2FnZXMvZ292ZXJuYW5jZS9zcmMvdGltZWxvY2svdGltZWxvY2tfY29udHJvbGxlci5jYWlybw==) | `88.70% <100.00%> (ø)` | | | [packages/governance/src/utils/call\_impls.cairo](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?src=pr&el=tree&filepath=packages%2Fgovernance%2Fsrc%2Futils%2Fcall_impls.cairo&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin#diff-cGFja2FnZXMvZ292ZXJuYW5jZS9zcmMvdXRpbHMvY2FsbF9pbXBscy5jYWlybw==) | `78.57% <100.00%> (ø)` | | | [packages/testing/src/constants.cairo](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?src=pr&el=tree&filepath=packages%2Ftesting%2Fsrc%2Fconstants.cairo&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin#diff-cGFja2FnZXMvdGVzdGluZy9zcmMvY29uc3RhbnRzLmNhaXJv) | `100.00% <100.00%> (ø)` | | | [...ckages/governance/src/multisig/storage\_utils.cairo](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?src=pr&el=tree&filepath=packages%2Fgovernance%2Fsrc%2Fmultisig%2Fstorage_utils.cairo&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin#diff-cGFja2FnZXMvZ292ZXJuYW5jZS9zcmMvbXVsdGlzaWcvc3RvcmFnZV91dGlscy5jYWlybw==) | `85.71% <85.71%> (ø)` | | | [packages/governance/src/multisig/multisig.cairo](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?src=pr&el=tree&filepath=packages%2Fgovernance%2Fsrc%2Fmultisig%2Fmultisig.cairo&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin#diff-cGFja2FnZXMvZ292ZXJuYW5jZS9zcmMvbXVsdGlzaWcvbXVsdGlzaWcuY2Fpcm8=) | `96.47% <96.47%> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin). Last update [02c9ce6...a7c569e](https://app.codecov.io/gh/OpenZeppelin/cairo-contracts/pull/1193?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenZeppelin).

[ericnordelo]

Also, if a signer is removed, should pending transactions be canceled since the signers that may have confirmed may have been the removed ones, and the confirmations of the transaction are not decreased while the quorum may have been? cc @andrew-fleming.

[immrsd]

Also, if a signer is removed, should pending transactions be canceled since the signers that may have confirmed may have been the removed ones, and the confirmations of the transaction are not decreased while the quorum may have been? cc @andrew-fleming.

That may be an issue indeed, good call. To my mind, introducing a new tx Canceled state and keeping track of all pending txs would overcomplicate the implementation and make it more error-prone. I addressed this issue by making confirmations a computable property that's not stored in the storage. It's calculated by iterating through all current signers and checking if a signer has confirmed the tx.

Only execute function becomes a bit more expensive in terms of computation, but it also removes storage variable for tracking number of confirmations and the necessity to update it in the confirm_transaction and revoke_confirmation functions