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/script tests #49

Merged
merged 7 commits into from
Sep 23, 2024
Merged

Feat/script tests #49

merged 7 commits into from
Sep 23, 2024

Conversation

wshino
Copy link
Contributor

@wshino wshino commented Sep 22, 2024

made test cases for scripts except for Deploy7579TestAccount.s.sol

### Test for scripts

```shell
pnpm test:script
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add --threads 1 to the test command because the tests for scripts might fail with multiple threads due to conflicts among env settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SoraSuegami pnpm test:script command already has this option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SoraSuegami I've added thread 1 option to foundry.toml

@SoraSuegami SoraSuegami changed the base branch from feat/body-parsing to matter-labs-audit-fixes September 22, 2024 22:00
@SoraSuegami
Copy link
Contributor

@wshino Could you also fix the conflict?

@SoraSuegami SoraSuegami merged commit 67e50c9 into matter-labs-audit-fixes Sep 23, 2024
7 checks passed
@SoraSuegami SoraSuegami deleted the feat/script-tests branch September 23, 2024 02:00
JohnGuilding pushed a commit that referenced this pull request Oct 8, 2024
* Remove email-wallet-sdk

* Update package version

* Update README

* Update pnpm commands

* Add tests for scripts

* Add threads = 1 to foundry.toml
Copy link
Collaborator

@JohnGuilding JohnGuilding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just got round to this. Noticed some missing test cases

* @dev Tests that deployment fails when both DKIM_REGISTRY and SIGNER environment variables are
* not set.
*/
function testFail_run_no_dkim_registry_no_signer() public {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testFail... may be deprecated in the future foundry-rs/foundry#4437. Don't think it is worth changing here now but FYI

/**
* @dev Tests the deployment process when the DKIM_REGISTRY environment variable is not set.
*/
function test_run_no_dkim_registry() public {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing cases for no emailAuthImpl, validatorAddr and _factory?

* @notice Tests the deployment and execution of the DeploySafeNativeRecovery script
* without a SIGNER configured.
*/
function test_run_no_signer() public {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test cases for no emailAuthImpl and commandHandler

/**
* @notice Tests the run function without a DKIM registry.
*/
function test_run_no_dkim_registry() public {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test cases for no dkimRegistrySigner and emailAuthImpl?

}

/// @notice Tests the deployment run without a DKIM registry
function test_run_no_dkim_registry() public {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mising test cases for no emailAuthImpl and _factory

JohnGuilding pushed a commit that referenced this pull request Oct 21, 2024
* Remove email-wallet-sdk

* Update package version

* Update README

* Update pnpm commands

* Add tests for scripts

* Add threads = 1 to foundry.toml
SoraSuegami added a commit that referenced this pull request Nov 3, 2024
* Use ether-email-auth for body parsing

* Replace "subject" with "command"

* Enable test on any branch

* chore: update .env.example

* Update ether-email-auth

* Update ether-email-auth

* Update ether-email-auth

* Update ether-email-auth

* Import ether-email-auth as npm package

* Add EmailRecoveryManagerZkSync

* Update version

* Fix EmailRecoveryManagerZkSync

* Fix renaming

* Fix remapping

* Fix EmailRecoveryManagerZkSync

* Fix README to replace "subject" with "command".

* fix compile warnings & format

* extract more logic to base test & rename variables

* remove unnecessary vm.prank

* Add more logic to base test & more linting

* Remove email-wallet-sdk

* Update package version

* Add how to debug errors to README

* Fix readme

* Add a CREATE2_SALT argument to DeploySafeRecovery script

* add guardian vote check & recovery hash to weight storage WIP

* Improve RecoveryRequest

* Fix all unit tests

* Feat/script tests (#49)

* Remove email-wallet-sdk

* Update package version

* Update README

* Update pnpm commands

* Add tests for scripts

* Add threads = 1 to foundry.toml

* Prevent malicious guardians threatening liveness via cooldown

* Uncomment all test assertions & use EnumerableSet

* Update processRecovery tests

* Add remaining tests from TODOs & add getter fn

* Update existing assertions with new state

* Add getAllGuardians getter function

* Make getter function view

* 10 Critical events are not observable

* Return false for canStartRecoveryRequest when threshold is zero

* Update ether-email-auth

* Update package version

* 10. EmailRecoveryContract is not compatible with zkSync

* clearRecoveryRequest properly in deinit

* Update ether-email-auth-contracts

* Update version to 0.0.10

* Update natpsec

* test-nexus-account

* Run tests with different command handlers

* Rename

* Add fuzz tests for hexToBytes32 unit tests

* Add tests for AccountHidingRecoveryCommandHandler

* Add safe module unit tests

* fuzz test getPreviousOwnerInLinkedList

* Add missing low risk functions

* Add script for account hiding

* Add the section which is how to solve errrors

* Use ForwardDKIMRegistry

* Remove ForwardDKIMRegistry

* Fix failing tests

* revert to all github workflows without nexus account

* 13 EmailRecoveryManager delay can be set to zero

* Fix failing test

* Fix lock file

* Recover lock file

* Change workflow

* Fix MockGroth16Verifier

* Update depndency

* Specify npm package of ether-email-auth

* Use salt value for deployment (#55)

* Use salt value for deployment

* Add minimumDelay

* Add CREATE2_SALT value into all deploy scripts

* Update depndencies

* Fix bug from updating modulekit

* Use StringUtils from ether-email-auth

* 0.0.12-preview

* 0.0.12

* Add kill switch functionality (#58)

* Add support section (#57)

* update @zk-email/ether-email-auth-contracts

* Update scripts

* Update scripts

* Fix script

* Fix script

* Add a new script

---------

Co-authored-by: Aditya Bisht <[email protected]>
Co-authored-by: JohnGuilding <[email protected]>
Co-authored-by: wshino <[email protected]>
Co-authored-by: John Guilding <[email protected]>
This was referenced Dec 10, 2024
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.

3 participants