Use EVMLocation type from flow-go-sdk
#5447
Conversation
We remove entirely the EVMLocation type from the evm types package, in favor of the type provided from flow-go-sdk. The TypeIDDecoder is also registered from flow-go-sdk, to make sure that Flow EVM events can always be CCF encoded/decoded.
There was a problem hiding this comment.
Thank you. I think this is a good solution, but architecturally speaking I don't like the approach on relying on Go-SDK for flow-go EVM to function correctly. I know this was not your decission and it is fine for now, but going forward if we decide to keep this event types, I'm not sure about it, here's why. What happens if someone else uses a different API client (js/java etc), we have the same issue I believe. I feel like if Cadence relies on this type registration to exists it shouldn't be up to the clients to register it. cc @turbolent
|
@m-Peter I released https://github.com/onflow/flow-go-sdk/releases/tag/v1.0.0-M6 which includes onflow/flow-go-sdk#583. Can you please update to that version? Also, from what it looks like (CI) there are still some cases where flow-go's @sideninja Agreed, Ardit and I had discussed this before, see https://discord.com/channels/613813861610684416/1108479699732152503/1207750237741916180. Given the current dependency chain, flow-go depending on flow-go-sdk, flow-go-sdk is the lowest where we can add this functionality, without having Cadence be made aware of EVM functionality. We can maybe revisit this in the future. |
|
@turbolent Updated in 4a1762b and fixed the failing test as well 🙏 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/stable-cadence #5447 +/- ##
==========================================================
+ Coverage 56.37% 57.48% +1.11%
==========================================================
Files 1030 782 -248
Lines 100057 78950 -21107
==========================================================
- Hits 56403 45382 -11021
+ Misses 39394 29990 -9404
+ Partials 4260 3578 -682
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
b4c7802
into
onflow:feature/stable-cadence
Follow up work from: onflow/flow-go-sdk#583
We remove entirely the
EVMLocationtype from thegithub.com/onflow/flow-go/fvm/evm/typespackage, in favor of the type provided fromflow-go-sdk.The
TypeIDDecoderis also registered fromflow-go-sdk, to make sure that Flow EVM events can always be CCF encoded/decoded, regardless of the combination of tooling used.