Skip to content
This repository has been archived by the owner on Oct 6, 2023. It is now read-only.

Fix activeOwnersCount initialization in MultiSigGeneric #222

Merged
merged 3 commits into from
Jul 24, 2023

Conversation

0xNeshi
Copy link
Contributor

@0xNeshi 0xNeshi commented Jul 24, 2023

Ticket(s): #199

Explanation of the solution

Variable activeOwnersCount should be initialized in MultiSigGeneric.initialize otherwise it always remains 0 (zero), causing unintended behavior later on (e.g. when adding/removing owners -> if we try to remove an owner, activeOwnersCount would be set to a negative value which is impossible with it being a uint256, causing a revert to happen).

It was necessary to redeploy CharityApplications and APTeamMultiSig proxies as well because their initialize methods had to be called again.

Instructions on making this work

  • run yarn or yarn install to install npm dependencies
  • run yarn test to verify all tests still pass
  • read activeOwnersCount for any of the multisig contracts
  • verify the actual value is the expected one (the "real" number of owners passed in initialize function)

@0xNeshi 0xNeshi added the bug Something isn't working label Jul 24, 2023
@0xNeshi 0xNeshi self-assigned this Jul 24, 2023
@0xNeshi 0xNeshi changed the title Fix activeOwnerStatus initialization in MultiSigGeneric Fix activeOwnersCount initialization in MultiSigGeneric Jul 24, 2023
@SovereignAndrey SovereignAndrey merged commit ab7a0a6 into master Jul 24, 2023
1 check failed
@SovereignAndrey SovereignAndrey deleted the fix-active-owner-status branch July 24, 2023 11:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants