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] DRAFT - Replace EVM.BridgeRouter interface with implementation #5821

Closed
wants to merge 2 commits into from

Conversation

sisyphusSmiling
Copy link
Contributor

@sisyphusSmiling sisyphusSmiling commented Apr 30, 2024

Related: onflow/flow-evm-bridge#60

While redeploying bridge contracts to Previewnet, I realized that the Accessor-Router pattern originally introduced is subverted by the Router being defined in a bridge contract. The idea originally was to define a neutral party router which would store a bridge-defined accessor through which the EVM contract would pass bridge calls to from COAs. However, currently, the BridgeRouter implementation is defined in a bridge contract which, in the event of a redeployment, must necessarily be replaced thus defeating the original purpose of the design.

This draft PR seeks to address this issue by replacing the interface with a resource definition within the EVM contract. I'm leaving this as a draft to serve as a point of discussion for a couple of points I could use clarity on:

  1. Removing an interface declaration is a breaking change - is this allowable on PreviewNet?
    a. If not, we could leave the EVM contract as-is and simply deploy a simple BridgeRouter defining contract to the EVM host account
  2. Normally, we cannot re-initialize a contract but we would need a way to initialize the BridgeRouter resource in the EVM account.
    a. If we proceed with the alternative in 1.a, this wouldn't be a problem as the resource could be initialized on the new contract's deployment
    b. If we move forward with the EVM contract as proposed in this PR, is there a way we could execute an initialization block to remove the need for the public initBridgeRouter method and just include setup in the contract's init block?

You can see changes in context in onflow/flow-evm-bridge#61

@sisyphusSmiling sisyphusSmiling self-assigned this Apr 30, 2024
@sisyphusSmiling sisyphusSmiling added SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board Flow EVM Smart Contract labels Apr 30, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.47%. Comparing base (cb89118) to head (2c7c1c2).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5821      +/-   ##
==========================================
+ Coverage   55.69%   60.47%   +4.77%     
==========================================
  Files        1094      924     -170     
  Lines       85646    68547   -17099     
==========================================
- Hits        47704    41452    -6252     
+ Misses      33328    23393    -9935     
+ Partials     4614     3702     -912     
Flag Coverage Δ
unittests 60.47% <ø> (+4.77%) ⬆️

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.

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

Successfully merging this pull request may close these issues.

2 participants