[Flow EVM] smart contract deployment for COAs #5269
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5269 +/- ##
==========================================
- Coverage 55.97% 55.70% -0.28%
==========================================
Files 1018 1006 -12
Lines 98189 97011 -1178
==========================================
- Hits 54963 54039 -924
+ Misses 39108 38911 -197
+ Partials 4118 4061 -57
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| // The Second way is through a new form of account called bridged accounts, | ||
| // The Second way is through a new form of account called cadence-owned-accounts (COAs), |
There was a problem hiding this comment.
I renamed to COA on files in the packages touched by this PR but left the stdlib and fvm/evm packages for another PR since it was changing so many lines.
There was a problem hiding this comment.
I only looked at the COA address derivation part. I made some suggestions (mainly adding comments) to clarify the reasoning behind the choices, so that they are documented somewhere:
- security reasoning behind choosing an address prefix of 12 bytes
- reasoning behind the index to postfix mapping
I am not sure whether you would need to add the reverse mapping (MOA address to index). If yes, let me know and I can add another PR.
Update: I would also add sanity tests about the size of the address prefix and the parity of the multiplier to avoid future changes break the assumptions, but that requires the constants to be made public (to be used in tests). Would you be fine with making them public? If yes I can also commit the tests.
suggestions for COA address
This PR updates the handler to deploy smart contracts for COA accounts,
I reused the same direct call sub-transaction type except the
toaddress is set in advance.I have to implement deployment to EVM at the specific address given Geth EVM doesn't allow this operation by default. Given that we use this implementation only for COA deployment we could heavily optimize it in the future.