-
Notifications
You must be signed in to change notification settings - Fork 111
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
refactor(fw,tests): fix address and hash types #422
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Had another look locally to see if I could find anything else, got matching fixtures too :D
Not sure what docs should be added, the only thing I can think of adding the usage of the This PR got me thinking: I wonder if we should force all addresses to be of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really welcome tidy-up to our types, makes much more sense. And also nice to move these to base_types.py
.
Agree with Spencer, not sure what we could add to the docs here.
Just verified and there aren't any. |
🗒️ Description
common/base_types.py
file, which includes basic types other complex types use, such asAddress
,Hash
, and other types that do not have any other dependency.to_address
,to_hash
andto_hash_bytes
in favor of using theAddress
andHash
classes directly.Address
to the list of types anOp
callable can receive, and, instead of treating theAddress
as bytecode, it automatically detects the type and adds the appropriateOp.PUSH20
.FixedSizeBytes
types can now be compared to strings and integersOp.PUSH20
across all tests when anAddress
type is used as a parameter of anOp
.Nice thing about this change is that it produces zero fixture changes, and slightly reduces lines of code.
TODO: Update docs.
🔗 Related Issues
Fixes #387
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.