-
Notifications
You must be signed in to change notification settings - Fork 180
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
[Flow EVM] COA ownership proof - part 2 #5342
Conversation
@@ -0,0 +1,92 @@ | |||
package precompiles_test |
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.
do we need to change the package name? why I'm asking because if not it seems we could keep encode/decode functions unexported.
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.
I didn't want people to use these methods yet, maybe we move them into abi package in the future.
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.
Left some comments, but looks good in general.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5342 +/- ##
==========================================
- Coverage 56.02% 55.96% -0.07%
==========================================
Files 965 1023 +58
Lines 89671 98820 +9149
==========================================
+ Hits 50241 55302 +5061
- Misses 35678 39353 +3675
- Partials 3752 4165 +413
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Mainly reviewed the Cadence-related code, which looks good 👍
// This package provides fast and efficient | ||
// utilities needed for abi encoding and decoding | ||
// encodings are mostly used for testing purpose | ||
// if more complex encoding and decoding is needed please | ||
// use the abi package and pass the ABIs, though | ||
// that has a performance overhead. |
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.
Move up, just above the package
declaration
This PR updates the cadence arch precompile to accept COA ownership proofs for verification.