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

Sovereign bridge implementation #330

Open
wants to merge 21 commits into
base: feature/ongoingPP
Choose a base branch
from

Conversation

ignasirv
Copy link
Contributor

@ignasirv ignasirv commented Sep 16, 2024

Create Bridge and GlobalExitRootManager contracts for Sovereign chains + tests.
BridgeL2SovereignChain.sol : extends from PolygonZkEVMBridgeV2 with the features of:

  • Set a bridgeManager address
  • Functions to remap and undo remap of token addresses by sovereign token addresses
  • Overrided mint/burn functions from BridgeV2 to handle that new remapping

GlobalExitRootManagerL2SovereignChain.sol : extends from PolygonZkEVMGlobalExitRootL2 with the features of:

  • New function insertGlobalExitRoot only callable by coinbase to update the GER of a Sovereign Chain

@ignasirv ignasirv force-pushed the feature/sovereign-bridge branch 2 times, most recently from c79a5d0 to 45319de Compare September 16, 2024 14:20
- Batch call function implementation to remap multiple tokens
- Allow migrating legacy to updated tokens natively
- Added weth mapped address are initializer
@invocamanman
Copy link
Collaborator

jsut run the coverage:
function setMultipleSovereignTokenAddress remains untested, even tho will be veery easy to do it so

migrateLegacyToken remains untested as wel which i think it's a critical funciton and we should test it!!!

@invocamanman
Copy link
Collaborator

also we shoudl update the 1_create genesis deployment script to be able to put a flag "sovereign" and deploy these contracts

@invocamanman
Copy link
Collaborator

Ger implementation shoould write blockHashes right?¿ probably from AggLayer or L1, to mirror the behaviour of Current Global exit root

Even tho in the zkResidency we though to change that to an index to help us retrieve all the improted GERs, but that depends a lot on what impementaiton we will follow to do that.
For now i wioudl say that you can set block hashes instead of timestamp

@invocamanman
Copy link
Collaborator

overall LGTM ^^ but we should do a proper review ^^

@invocamanman
Copy link
Collaborator

As an added comment i think the docker does not work since the bytecode of hte new bridge contract is too big.

Warning: Contract code size is 31141 bytes and exceeds 24576 bytes (a limit introduced in Spurious Dragon). This contract may not be deployable on Mainnet. Consider enabling the optimizer (with a low "runs" value!), turning off revert strings, or using libraries.
  --> contracts/v2/sovereignChains/BridgeL2SovereignChain.sol:15:1:

We should reduce the bytecode size, the eaisest way is to put it on exceptions on harhdat and lower the optimizer runs by default

Copy link
Contributor

@krlosMata krlosMata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😸

Copy link
Contributor

@krlosMata krlosMata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😸

Copy link

sonarcloud bot commented Oct 17, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants