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

[EVM] Add 1.0 bridging interface to COA #5664

Closed

Conversation

sisyphusSmiling
Copy link
Contributor

@sisyphusSmiling sisyphusSmiling commented Apr 12, 2024

Closes: #5663

This PR introduces bridge functionality to the COA resource, enabling a caller to bridge owned NFTs & FTs between VMs. These requests are routed from the calling COA to a BridgeRouter implementation which is stored in the EVM contract account. This resource encapsulates and safeguards a Capability on a BridgeAccessor implementation - defined in a bridge contract and stored in the bridge account - to which the BridgeRouter routes bridge requests.

The BridgeAccessor (not in scope for this PR but can be seen in onflow/flow-evm-bridge#28) then passes routed calls through to the VM bridge. Via these new interfaces and their stored implementations, the EVM contract can be integrated with the bridge.

Note that simply updating the EVM contract with these changes isn't sufficient to integrate with the bridge as calls to borrowBridgeAccessor will revert if a BridgeRouter is not saved with a valid BridgeAccessor Capability. To fully integrate, the bridge contracts will need to be deployed, a BridgeAccessor Capability published for the EVM contract account, and the published Capability will need to be saved in a BridgeRouter stored at the hardcoded storage path of /storage/evmBridgeRouter. Between the EVM update and the storage of the BridgeRouter, any bridge-related COA calls will simply fail.

@sisyphusSmiling sisyphusSmiling changed the title Add bridging interface to COA [EVM] Add bridging interface to COA Apr 12, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.53%. Comparing base (39f59b1) to head (76b072d).
Report is 551 commits behind head on feature/stable-cadence.

Additional details and impacted files
@@                    Coverage Diff                     @@
##           feature/stable-cadence    #5664      +/-   ##
==========================================================
+ Coverage                   55.78%   58.53%   +2.74%     
==========================================================
  Files                         997       96     -901     
  Lines                       98760     8129   -90631     
==========================================================
- Hits                        55094     4758   -50336     
+ Misses                      39400     2869   -36531     
+ Partials                     4266      502    -3764     
Flag Coverage Δ
unittests 58.53% <ø> (+2.74%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sisyphusSmiling sisyphusSmiling force-pushed the giovanni/add-bridge-interface branch from 3f56e23 to e73a0a1 Compare April 15, 2024 22:28
@sisyphusSmiling sisyphusSmiling self-assigned this Apr 15, 2024
@sisyphusSmiling sisyphusSmiling added enhancement New feature or request SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board Feature Flow EVM labels Apr 15, 2024
@sisyphusSmiling sisyphusSmiling marked this pull request as ready for review April 15, 2024 22:38
@turbolent
Copy link
Member

Nice! Should this also be on master?

@sisyphusSmiling
Copy link
Contributor Author

@turbolent I suppose it should, though I'm not sure how we're deciding what features live on master vs feature/stable-cadence

@turbolent
Copy link
Member

turbolent commented Apr 15, 2024

@sisyphusSmiling AFAIK, so far all EVM features are on master. feature/stable-cadence only differs in terms of Cadence 1.0 support (for example, the EVM contract has entitlements, which are not supported pre-Cadence 1.0).

Maybe rebase on master (without entitlements), and then once we merge/sync master into feature/stable-cadence (fairly frequently), we can make Cadence 1.0-adjustments, like adding entitlements.

@sisyphusSmiling
Copy link
Contributor Author

@turbolent had some issues trying to rebase on master with this branch. Thinking I'll close this PR and open a new one with a branch upstream from master.

@sisyphusSmiling sisyphusSmiling changed the title [EVM] Add bridging interface to COA [EVM] Add 1.0 bridging interface to COA Apr 15, 2024
@turbolent
Copy link
Member

@sisyphusSmiling Sounds good, either way works! Also happy to help with rebasing this PR if you want

@sisyphusSmiling
Copy link
Contributor Author

Closing in favor of #5716

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Feature Flow EVM SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board
Projects
Status: Done
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants