Add EVM bridge contracts to SystemContracts#8358
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
📝 WalkthroughWalkthroughThis change expands the SystemContracts struct by introducing 22 new public system contract fields for EVM bridge-related contracts and utilities, adds an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@fvm/systemcontracts/system_contracts.go`:
- Around line 355-380: All() currently lists bridge contracts (CrossVMNFT,
CrossVMToken, FlowEVMBridge*, FlowEVMBridgeUtils, etc.) even though
setupVMBridge() only deploys them on transient networks; update the registry so
All() only includes those bridge-related contracts for transient chains (or when
setupVMBridge() indicates they were deployed) — locate the All() function in
system_contracts.go and either filter out the bridge symbols for non-transient
chains (e.g., check the chain type or call the same transient check used by
setupVMBridge()) or move the bridge entries into the transient-only registration
path so that contract existence checks reflect actual deployments.
🧹 Nitpick comments (1)
fvm/systemcontracts/system_contracts.go (1)
294-321: Prefer deriving bridge env from the concrete SystemContracts fields.Using the service account address for every entry can drift if any bridge contracts move to different accounts. Pulling from the actual
SystemContractsfields keeps this consistent withcontractAddressFunc.♻️ Proposed refactor
func (c SystemContracts) AsBridgeEnv() bridge.Environment { - serviceAccountAddress := c.FlowServiceAccount.Address.Hex() return bridge.Environment{ - CrossVMNFTAddress: serviceAccountAddress, - CrossVMTokenAddress: serviceAccountAddress, - FlowEVMBridgeHandlerInterfacesAddress: serviceAccountAddress, - IBridgePermissionsAddress: serviceAccountAddress, - ICrossVMAddress: serviceAccountAddress, - ICrossVMAssetAddress: serviceAccountAddress, - IEVMBridgeNFTMinterAddress: serviceAccountAddress, - IEVMBridgeTokenMinterAddress: serviceAccountAddress, - IFlowEVMNFTBridgeAddress: serviceAccountAddress, - IFlowEVMTokenBridgeAddress: serviceAccountAddress, - FlowEVMBridgeAddress: serviceAccountAddress, - FlowEVMBridgeAccessorAddress: serviceAccountAddress, - FlowEVMBridgeConfigAddress: serviceAccountAddress, - FlowEVMBridgeHandlersAddress: serviceAccountAddress, - FlowEVMBridgeNFTEscrowAddress: serviceAccountAddress, - FlowEVMBridgeResolverAddress: serviceAccountAddress, - FlowEVMBridgeTemplatesAddress: serviceAccountAddress, - FlowEVMBridgeTokenEscrowAddress: serviceAccountAddress, - FlowEVMBridgeUtilsAddress: serviceAccountAddress, - ArrayUtilsAddress: serviceAccountAddress, - ScopedFTProvidersAddress: serviceAccountAddress, - SerializeAddress: serviceAccountAddress, - SerializeMetadataAddress: serviceAccountAddress, - StringUtilsAddress: serviceAccountAddress, + CrossVMNFTAddress: c.CrossVMNFT.Address.Hex(), + CrossVMTokenAddress: c.CrossVMToken.Address.Hex(), + FlowEVMBridgeHandlerInterfacesAddress: c.FlowEVMBridgeHandlerInterfaces.Address.Hex(), + IBridgePermissionsAddress: c.IBridgePermissions.Address.Hex(), + ICrossVMAddress: c.ICrossVM.Address.Hex(), + ICrossVMAssetAddress: c.ICrossVMAsset.Address.Hex(), + IEVMBridgeNFTMinterAddress: c.IEVMBridgeNFTMinter.Address.Hex(), + IEVMBridgeTokenMinterAddress: c.IEVMBridgeTokenMinter.Address.Hex(), + IFlowEVMNFTBridgeAddress: c.IFlowEVMNFTBridge.Address.Hex(), + IFlowEVMTokenBridgeAddress: c.IFlowEVMTokenBridge.Address.Hex(), + FlowEVMBridgeAddress: c.FlowEVMBridge.Address.Hex(), + FlowEVMBridgeAccessorAddress: c.FlowEVMBridgeAccessor.Address.Hex(), + FlowEVMBridgeConfigAddress: c.FlowEVMBridgeConfig.Address.Hex(), + FlowEVMBridgeHandlersAddress: c.FlowEVMBridgeHandlers.Address.Hex(), + FlowEVMBridgeNFTEscrowAddress: c.FlowEVMBridgeNFTEscrow.Address.Hex(), + FlowEVMBridgeResolverAddress: c.FlowEVMBridgeResolver.Address.Hex(), + FlowEVMBridgeTemplatesAddress: c.FlowEVMBridgeTemplates.Address.Hex(), + FlowEVMBridgeTokenEscrowAddress: c.FlowEVMBridgeTokenEscrow.Address.Hex(), + FlowEVMBridgeUtilsAddress: c.FlowEVMBridgeUtils.Address.Hex(), + ArrayUtilsAddress: c.ArrayUtils.Address.Hex(), + ScopedFTProvidersAddress: c.ScopedFTProviders.Address.Hex(), + SerializeAddress: c.Serialize.Address.Hex(), + SerializeMetadataAddress: c.SerializeMetadata.Address.Hex(), + StringUtilsAddress: c.StringUtils.Address.Hex(), } }
| c.Migration, | ||
|
|
||
| c.CrossVMNFT, | ||
| c.CrossVMToken, | ||
| c.FlowEVMBridgeHandlerInterfaces, | ||
| c.IBridgePermissions, | ||
| c.ICrossVM, | ||
| c.ICrossVMAsset, | ||
| c.IEVMBridgeNFTMinter, | ||
| c.IEVMBridgeTokenMinter, | ||
| c.IFlowEVMNFTBridge, | ||
| c.IFlowEVMTokenBridge, | ||
| c.FlowEVMBridge, | ||
| c.FlowEVMBridgeAccessor, | ||
| c.FlowEVMBridgeConfig, | ||
| c.FlowEVMBridgeHandlers, | ||
| c.FlowEVMBridgeNFTEscrow, | ||
| c.FlowEVMBridgeResolver, | ||
| c.FlowEVMBridgeTemplates, | ||
| c.FlowEVMBridgeTokenEscrow, | ||
| c.FlowEVMBridgeUtils, | ||
| c.ArrayUtils, | ||
| c.ScopedFTProviders, | ||
| c.Serialize, | ||
| c.SerializeMetadata, | ||
| c.StringUtils, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the system_contracts.go file to see the context
cd fvm/systemcontracts && wc -l system_contracts.go && head -50 system_contracts.goRepository: onflow/flow-go
Length of output: 2458
🏁 Script executed:
# Find the All() function definition and usage
rg -n "func.*All\(\)|\.All\(\)" --type=go fvm/systemcontracts/Repository: onflow/flow-go
Length of output: 312
🏁 Script executed:
# Search for bootstrap and deployment code
rg -n "bootstrap|deploy" --type=go -i fvm/systemcontracts/Repository: onflow/flow-go
Length of output: 710
🏁 Script executed:
# Check for where these new bridge contracts are defined
rg -n "CrossVMNFT|FlowEVMBridge" --type=go fvm/ | head -30Repository: onflow/flow-go
Length of output: 3155
🏁 Script executed:
# Read the All() function implementation
sed -n '325,380p' fvm/systemcontracts/system_contracts.goRepository: onflow/flow-go
Length of output: 1226
🏁 Script executed:
# Check how All() is tested
cat -n fvm/systemcontracts/system_contracts_test.go | head -80Repository: onflow/flow-go
Length of output: 3060
🏁 Script executed:
# Search for where system contracts are deployed/bootstrapped
rg -n "NewSystemContracts|bootstrap" --type=go fvm/ | grep -v test | head -20Repository: onflow/flow-go
Length of output: 1621
🏁 Script executed:
# Look for init or setup functions that deploy contracts
rg -n "func init|SetupSystemContracts|DeploySystemContracts" --type=go -i | head -20Repository: onflow/flow-go
Length of output: 1436
🏁 Script executed:
# Look at the bootstrap.go file to see how contracts are deployed
sed -n '224,300p' fvm/bootstrap.goRepository: onflow/flow-go
Length of output: 2473
🏁 Script executed:
# Check if there's specific code for deploying bridge contracts
rg -n "WithFlowEVMBridge|DeployBridge" --type=go | head -20Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Look for where SystemContracts are actually deployed in bootstrapping
rg -n "setupSystemContracts|deployContracts" --type=go -i | head -20Repository: onflow/flow-go
Length of output: 1166
🏁 Script executed:
# Search for how chain-specific system contracts are configured
rg -n "SystemContractsForChain|mainnet|testnet" --type=go fvm/systemcontracts/ | head -30Repository: onflow/flow-go
Length of output: 987
🏁 Script executed:
# Look at the SystemContractsForChain function implementation
sed -n '404,500p' fvm/systemcontracts/system_contracts.goRepository: onflow/flow-go
Length of output: 3016
🏁 Script executed:
# Check for chain-specific contract configurations
rg -n "Mainnet|Testnet|Canarynet|Previewnet" --type=go fvm/systemcontracts/system_contracts.go | head -30Repository: onflow/flow-go
Length of output: 2165
🏁 Script executed:
# Look for where bridge contracts are conditionally included or excluded
rg -n "CrossVMNFT|FlowEVMBridge" --type=go fvm/systemcontracts/system_contracts.go -B 2 -A 2 | head -60Repository: onflow/flow-go
Length of output: 3390
🏁 Script executed:
# Check if there's any migration or rollout status for these contracts
rg -n "Migration|rollout|enabled|disabled" --type=go fvm/systemcontracts/system_contracts.go -i | head -20Repository: onflow/flow-go
Length of output: 304
🏁 Script executed:
# Look at the full init() function to see which contracts are registered
sed -n '428,700p' fvm/systemcontracts/system_contracts.goRepository: onflow/flow-go
Length of output: 11580
🏁 Script executed:
# Search for where contractAddressFunc is populated for the bridge contracts
rg -n "contractAddressFunc\[" --type=go fvm/systemcontracts/system_contracts.go | tail -50Repository: onflow/flow-go
Length of output: 221
🏁 Script executed:
# Check the complete list of what's registered in contractAddressFunc
rg -n "registerContractAddress|contractAddressFunc\[ContractName" --type=go fvm/systemcontracts/system_contracts.go | wc -lRepository: onflow/flow-go
Length of output: 59
🏁 Script executed:
# Count how many contracts are in All()
sed -n '325,390p' fvm/systemcontracts/system_contracts.go | grep "c\." | wc -lRepository: onflow/flow-go
Length of output: 60
🏁 Script executed:
# Look for the registerContractAddress or similar registration pattern
rg -n "registerContractAddress" --type=go fvm/systemcontracts/system_contracts.go -A 3 | head -30Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Search for bootstrap code that actually deploys contracts
rg -n "setupVM|DeployContracts|BridgeContract" --type=go fvm/ | grep -i bootstrap | head -20Repository: onflow/flow-go
Length of output: 547
🏁 Script executed:
# Look for Cadence contract deployment in bootstrap
rg -n "setupVMBridge|bridge" --type=go fvm/bootstrap.go -i -A 3 | head -40Repository: onflow/flow-go
Length of output: 1584
🏁 Script executed:
# Check if there's a deployment manifest or list
rg -n "manifest|deployment.*list|contract.*list" --type=go fvm/ -i | head -20Repository: onflow/flow-go
Length of output: 1119
🏁 Script executed:
# Look at what happens in transient network setup vs mainnet/testnet
rg -n "transient|mainnet|testnet" --type=go fvm/systemcontracts/system_contracts.go -B 1 -A 1 | head -40Repository: onflow/flow-go
Length of output: 383
🏁 Script executed:
# Look at the test that was mentioned - transient network contract existence checks
cat -n fvm/systemcontracts/system_contracts_test.go | head -100Repository: onflow/flow-go
Length of output: 4043
🏁 Script executed:
# Search for tests that check contract existence
rg -n "All\(\)" --type=go fvm/systemcontracts/ -A 5 -B 5Repository: onflow/flow-go
Length of output: 2394
🏁 Script executed:
# Check if there's documentation about deployment on mainnet/testnet
rg -n "mainnet|testnet" --type=go fvm/bootstrap.go -B 2 -A 2 | grep -i "bridge\|deploy"Repository: onflow/flow-go
Length of output: 258
🏁 Script executed:
# Look for where bridge contracts are actually deployed on mainnet/testnet
rg -n "CrossVMNFT|FlowEVMBridge" --type=go --type=go -l | xargs grep -l "mainnet\|testnet" | head -10Repository: onflow/flow-go
Length of output: 114
🏁 Script executed:
# Check the blueprints to see what contracts are deployed
rg -n "BridgeContracts" --type=go fvm/blueprints/ -A 10 -B 5Repository: onflow/flow-go
Length of output: 1321
🏁 Script executed:
# Check all chain types
rg -n "type ChainID|func AllChainIDs" --type=go model/flow/Repository: onflow/flow-go
Length of output: 206
🏁 Script executed:
# See what AllChainIDs returns
rg -n "func AllChainIDs" --type=go -A 10 model/flow/Repository: onflow/flow-go
Length of output: 485
🏁 Script executed:
# Check if there's chain-specific logic for bridge deployment on mainnet/testnet
sed -n '1042,1200p' fvm/bootstrap.goRepository: onflow/flow-go
Length of output: 7193
🏁 Script executed:
# Look for where bridge contracts might be deployed on mainnet/testnet separately
rg -n "migration|deploy.*bridge|bridge.*deploy" --type=go -i fvm/ | grep -v test | head -20Repository: onflow/flow-go
Length of output: 1827
🏁 Script executed:
# Double-check: does SystemContractsForChain return different contracts for different chains?
# Look for any filtering or chain-specific logic in the SystemContracts construction
sed -n '575,700p' fvm/systemcontracts/system_contracts.go | grep -A 100 "getSystemContractsForChain"Repository: onflow/flow-go
Length of output: 5179
🏁 Script executed:
# Check if Mainnet and Testnet are transient
rg -n "func.*Transient|func.*IsTransient" --type=go model/flow/chain.go -A 5Repository: onflow/flow-go
Length of output: 349
🏁 Script executed:
# Look for any documented notes about bridge contract availability on mainnet/testnet
rg -n "bridge.*mainnet|bridge.*testnet|bridge.*availability" --type=go -i | head -20Repository: onflow/flow-go
Length of output: 378
🏁 Script executed:
# Final confirmation: check if there's any migration plan or deployment roadmap for bridge contracts on mainnet/testnet
rg -n "bridge.*migration|rollout.*bridge|deploy.*bridge" --type=go -i | grep -v "test" | head -15Repository: onflow/flow-go
Length of output: 2084
🏁 Script executed:
# Check if the test actually validates this concern
rg -n "TestSystemContracts|checkSystemContracts" --type=go -A 20 fvm/systemcontracts/system_contracts_test.go | head -50Repository: onflow/flow-go
Length of output: 1955
Bridge contracts are only deployed on transient networks, but All() returns them for all chains including Mainnet and Testnet.
The bridge contracts (CrossVMNFT, CrossVMToken, FlowEVMBridge*, BridgeUtils, etc.) are only deployed during bootstrap on transient networks (Emulator, Previewnet, etc.) via the setupVMBridge() function which returns early for non-transient chains. However, All() includes these contracts for all chains (Mainnet, Testnet, Sandboxnet, etc.), which means tests checking contract existence will report valid addresses but the contracts won't actually be deployed on long-lived networks. Verify whether this is intentional (e.g., planned for future rollout) or if All() and the system contracts registry should only include contracts that are actually deployed on each target chain.
🤖 Prompt for AI Agents
In `@fvm/systemcontracts/system_contracts.go` around lines 355 - 380, All()
currently lists bridge contracts (CrossVMNFT, CrossVMToken, FlowEVMBridge*,
FlowEVMBridgeUtils, etc.) even though setupVMBridge() only deploys them on
transient networks; update the registry so All() only includes those
bridge-related contracts for transient chains (or when setupVMBridge() indicates
they were deployed) — locate the All() function in system_contracts.go and
either filter out the bridge symbols for non-transient chains (e.g., check the
chain type or call the same transient check used by setupVMBridge()) or move the
bridge entries into the transient-only registration path so that contract
existence checks reflect actual deployments.
|
This adds a lot of contracts to the default bootstrapping, is this something we need for flow-go or do we just need it downstream? |
3 participants
The Emulator lists all system contracts on startup. Include the EVM bridge contracts in that list for informational purposes. Also add a helper to return
bridge.Environmentand don't duplicate it in tests.Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.