Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/fix test allow validator recovery #44

Merged

Conversation

wshino
Copy link
Contributor

@wshino wshino commented Sep 8, 2024

For testing allowValidatorRecovery,
I created the validator2Address variable, it's not installed via isModuleInstalled.
allowValidatorRecovery needs to be installed of validator to succeed, but if the module installed function called, the validator already been registered.
I added the mockCall for isModuleInstalled function.

@SoraSuegami
Copy link
Contributor

Thank you for your PR!
Did you check why the failed tests returned "LinkedList_EntryAlreadyInList(0x27cc01A4676C73fe8b6d0933Ac991BfF1D77C4da)" error?

allowValidatorRecovery needs to be installed of validator to succeed, but if the module installed function called, the validator already been registered.

I cannot understand this part very well.

@wshino
Copy link
Contributor Author

wshino commented Sep 8, 2024

@SoraSuegami Af far as I see, the above failed details are following steps in test_AllowValidatorRecovery_When_SafeAddOwnerSelector:

  1. call setUp() in UnitBase()
  2. call instance.installModule with module is validatorAddress(0x27cc01A4676C73fe8b6d0933Ac991BfF1D77C4da)
  3. call onInstall function in UniversalEmailRecoveryModule
  4. call allowValidatorRecovery with module is validatorAddress(0x27cc01A4676C73fe8b6d0933Ac991BfF1D77C4da)
  5. call validators[0x8D778546350bCe5b88083612A87451EB0E6693b2].push(0x27cc01A4676C73fe8b6d0933Ac991BfF1D77C4da)
  6. call emailRecoveryModule.allowValidatorRecovery at allowValidatorRecovery.t.sol with validatorAddress(0x27cc01A4676C73fe8b6d0933Ac991BfF1D77C4da)
  7. call validators[0x8D778546350bCe5b88083612A87451EB0E6693b2].push(0x27cc01A4676C73fe8b6d0933Ac991BfF1D77C4da) -> fail

Basically onInstallModule registers validator address. So to test allowValidatorRecovery standalone, I need to add a validator without installing the module.

@SoraSuegami SoraSuegami merged commit 3d39050 into feat/audit-fix-08 Sep 8, 2024
4 checks passed
@SoraSuegami SoraSuegami deleted the feat/fix-test-allow-validator-recovery branch September 8, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants