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

chore: use medusa instead of echidna #87

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

0xteddybear
Copy link

@0xteddybear 0xteddybear commented Oct 15, 2024

this PR intends to make the boilerplate follow what we currently are doing for production repositories regarding fuzzing campaings:

  • using medusa instead of echidna
  • separating handlers and pure property checks by naming convention, while only allowing assertion-mode testing
  • discerning between guided and unguided handlers -> usually we do it in separate files, but I figured that's a lot of boilerplate for such a small testsuite, so I'm using different names instead
  • use default foundry cheatcodes instead of re-defining them

This does not include a workflow to run medusa in CI, which is currently a WIP on #86

@0xteddybear 0xteddybear marked this pull request as ready for review October 15, 2024 22:32
@gas1cent gas1cent changed the title Chore: use medusa instead of echidna chore: use medusa instead of echidna Oct 18, 2024
medusa.json Outdated Show resolved Hide resolved
medusa.json Outdated Show resolved Hide resolved
@0xteddybear 0xteddybear requested a review from gas1cent October 21, 2024 13:38
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
}
},
"propertyTesting": {
"enabled": false,
Copy link
Member

Choose a reason for hiding this comment

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

true by default (avoiding incidents)?

Copy link
Author

Choose a reason for hiding this comment

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

the idea is for the boilerplate to closely resemble what we run in production, and afaik we are disabling property testing in all projects to reduce complexity (2 kinds of tests vs 1) and help avoid mistakes where the property testing mode is disabled down the line and property tests are executed as assertion tests, causing a false negative, since that false negative would be present when the test is initially developed, making it evident

"targetContractsBalances": [],
"constructorArgs": {},
"deployerAddress": "0x30000",
"senderAddresses": [
Copy link
Member

Choose a reason for hiding this comment

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

I think we are not using these.

Copy link
Author

Choose a reason for hiding this comment

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

we are not taking advantage of the various sender addresses in the simple boilerplate example, but they are usually later used in our actor setups for bigger projects, so I lean on leaving them in

what should be the alternative? using only one? I believe medusa wont run if they key is deleted or has an empty array as a value

Copy link
Member

Choose a reason for hiding this comment

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

I would use just one only if somehow it enhance the performance.

Copy link
Author

Choose a reason for hiding this comment

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

afaik having one vs many doesnt affect performance (as long as having many users doesnt make the domain of possible states grow at a relevant rate, which is not the case for the humble greeter)

Copy link
Member

@gas1cent gas1cent left a comment

Choose a reason for hiding this comment

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

Agree with the above regarding sender addresses and disabling propertyTesting.

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.

4 participants