Skip to content

FLIP-318: VM Bridge Support for Cross-VM NFTs #188

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
May 19, 2025
Merged

FLIP-318: VM Bridge Support for Cross-VM NFTs #188

merged 13 commits into from
May 19, 2025
+8,064 −1,252

Conversation

sisyphusSmiling
Copy link
Contributor

@sisyphusSmiling sisyphusSmiling commented Apr 30, 2025 •
edited
Loading

Related: #143

Description

  • Implements FLIP-318, enabling support for custom cross-VM NFTs

For contributor use:

  • Targeted PR against main branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

sisyphusSmiling and others added 9 commits March 7, 2025 10:38
@sisyphusSmiling
update BridgePermissions._setPermissions event behavior (#174)
d55dd6f
@sisyphusSmiling
Remove Locker.erc721Address resource field (#175)
ccdd8e1 
* remove NFT escrow Locker.erc721address field

* update FlowEVMBridgeNFTEscrow to use string interpolation

* update cadence_test github action dependency
@sisyphusSmiling @joshuahannan
Add EVM-native cross-vm NFT interfaces & custom cross-vm registration…
0894a3b 
… support (#173)

* update dependencies and implement FlowEVMBridgeCustomAssociations

* update FlowEVMBridgeCustomAssociations.CustomAssociationEstablished event & emit

* move CadenceNativeERC721.sol

* add EVMNativeERC721.sol, update tests & config

* update EVMNativeERC721.sol example to include ICrossVM conformance

* add draft custom cross-vm NFT registration path

* update transaction comments

* add utils for cross-VM EVM contract conformance checks

* utilize new methods in FlowEVMBridgeUtils for cross-VM registration

* add test coverage for net new EVM native NFT

* update test setup

* add FlowEVMBridgeNFTEscrow.isInitialized()

* add comments

* Updates to Go Package (#169)

* update to core-contracts v1.6.0

* add deploy utils transaction and update deps

* refactor go pkg

* update Makefile and require correct package

* add accessor deploy transaction and make sure it is included in templateS

* fix bug in import replacement

* fix accessor replace

* remove NFT escrow Locker.erc721address field

* update FlowEVMBridgeNFTEscrow to use string interpolation

* update cadence_test github action dependency

* update code comments

* add custom association types contract

* implement CustomAssociationTypes contract & update tests

* update messages to use string interpolation

* check consistent native VM on cross-VM NFT registration

* fix failing test case

* tie cross-VM registration into legacy onboarding path

* update comments

* update ExampleEVMNativeNFT path storage patterns

* add tokenURI assignment to example CadenceNativeERC721.sol & update tests

* add initial ExampleCadenceNativeNFT implementation

* update CadenceNativeERC721 constructor args

* add ICrossVM conformance to CadenceNativeERC721

* resolve EVMBridgedMetadata & deploy ERC721 from Cadence contract init args

* initialize escrow on cross-VM NFT registration

* update EVM native cross-VM NFT registration tests

* fix Cadence-native NFT native VM

* fix crossVM interface detection in utils contract

* add Cadence-native cross-VM NFT tests

* add evm-native cross-VM NFT registration test case

* add test cases covering update from bridged to cross-VM NFT

* update test case

* update comments

* add EVM asset conformance check on custom association registration

* update cross-vm registration tests

* update contract comments

* fix NFTFulfillmentMinter pre-condition

* update custom associations event types

* update test_helpers bytecodes

* update utils methods & comments

* rename getEVMPointer() to getEVMPointerView()

* add getter for CustomConfig supported standard types

* Apply suggestions from code review

Co-authored-by: Joshua Hannan <[email protected]>

* removed unused methods from ExampleCadenceNativeNFT

* fix registerCrossVMNFT vulnerability on unchecked minter definition

* remove redundant test assertions

* consolidate test setup

* consolidate test setup in handler tests

* update pre-condition err message based on PR feedback

* add test coverage for new utils methods

* update pre-condition error message

* update dependencies to pull from mainnet sources

* update go package and fix failing template tests

* add Cadence test to Makefile ci command

* update go.mod

* update flow-cli dependency install command in cadence_test workflow

---------

Co-authored-by: Joshua Hannan <[email protected]>
@sisyphusSmiling
Update cross-VM NFT registration native-VM consistency conditions (#178)
fc80346 
* update cross-VM NFT registration native-VM consistency conditions

* fix error message typo
@sisyphusSmiling @joshuahannan
Include custom associations in un/pause by Type functionality (#176)
090b055 
* update pause by type to include custom associations

* update conditional to assertion

* Apply suggestions from code review

Co-authored-by: Joshua Hannan <[email protected]>

* fix failing test case on pausing by type

* update isTypePaused to include CustomConfig status

* fix and reduce isTypePaused logic

---------

Co-authored-by: Joshua Hannan <[email protected]>
@sisyphusSmiling @joshuahannan
Integrate cross-VM NFTs into NFT to EVM path (#179)
ba4e4d1 
* update pause by type to include custom associations

* update conditional to assertion

* Apply suggestions from code review

Co-authored-by: Joshua Hannan <[email protected]>

* fix failing test case on pausing by type

* draft updates to bridgeNFTToEVM()

* add mustBurnERC721 util method

* add path for Cadence-native cross-VM NFT to EVM

* add EVM-native cross-VM NFT to EVM path

* update ExampleCadenceNativeNFT minting process

* fix ExampleCadenceNativeNFT collection data resolution

* begin test coverage for Cadence-native NFT to EVM

* update isTypePaused to include CustomConfig status

* fix and reduce isTypePaused logic

* add FulfilledFromEVM event to NFTFulfillmentMinter

* fix bridge cross-VM NFT to EVM errors

* add coverage for Cadence-native cross-VM NFT to EVM

* add Cadence-native cross-VM NFT test case

* add draft EVM-native cross-VM test case - blocked by from EVM route

* update EVM call transaction interface

* update bridge contract comments

* add test_helper methods to consolidate test code

* restructure update_nft_from_bridged_tests to allow for control over sequence of events

* remove contract comment

---------

Co-authored-by: Joshua Hannan <[email protected]>
@sisyphusSmiling @joshuahannan
Update bridgeNFTFromEVM route for custom cross-VM NFTs (#181)
49e28f6 
* update pause by type to include custom associations

* update conditional to assertion

* Apply suggestions from code review

Co-authored-by: Joshua Hannan <[email protected]>

* fix failing test case on pausing by type

* draft updates to bridgeNFTToEVM()

* add mustBurnERC721 util method

* add path for Cadence-native cross-VM NFT to EVM

* add EVM-native cross-VM NFT to EVM path

* update ExampleCadenceNativeNFT minting process

* fix ExampleCadenceNativeNFT collection data resolution

* begin test coverage for Cadence-native NFT to EVM

* update isTypePaused to include CustomConfig status

* fix and reduce isTypePaused logic

* add FulfilledFromEVM event to NFTFulfillmentMinter

* fix bridge cross-VM NFT to EVM errors

* add coverage for Cadence-native cross-VM NFT to EVM

* add Cadence-native cross-VM NFT test case

* add draft EVM-native cross-VM test case - blocked by from EVM route

* update EVM call transaction interface

* update bridge contract comments

* add test_helper methods to consolidate test code

* restructure update_nft_from_bridged_tests to allow for control over sequence of events

* remove contract comment

* update COA callback in FlowEVMBridgeAccessor.withdrawNFT

* add EVM-native cross-VM bridging from EVM

* add Cadence-native cross-VM NFT from EVM path

* consolidate bridgeNFTFromEVM logic for cross-VM NFTs

* fix EVM-native cross-VM NFT from EVM path

* add internal handler for bridging updated NFTs from EVM

* add EVM-native cross-VM NFT test coverage

* consolidate test code

* add updated cross-VM NFT test case bridging from EVM

* add test coverage for Cadence-native updated cross-VM NFTs

* update code doc comments

* simplify FlowEVMBridgeAccessor.isValidEVMTarget

* move internal bridge helpers

* update contract comments

* add Cadence-native migration transaction & test case

* add Cadence-native ERC721 wrapper example

* fix bug in handling wrapped ERC721 from EVM & add test case

* add balance script

* update EVM dependency hash after latest HCU

* Update cadence/contracts/bridge/FlowEVMBridge.cdc

Co-authored-by: Joshua Hannan <[email protected]>

* remove unused method from FlowEVMBridge

---------

Co-authored-by: Joshua Hannan <[email protected]>
@sisyphusSmiling @joshuahannan
Use dryCall for non-mutating EVM calls (#184)
5197122 
* update pause by type to include custom associations

* update conditional to assertion

* Apply suggestions from code review

Co-authored-by: Joshua Hannan <[email protected]>

* fix failing test case on pausing by type

* draft updates to bridgeNFTToEVM()

* add mustBurnERC721 util method

* add path for Cadence-native cross-VM NFT to EVM

* add EVM-native cross-VM NFT to EVM path

* update ExampleCadenceNativeNFT minting process

* fix ExampleCadenceNativeNFT collection data resolution

* begin test coverage for Cadence-native NFT to EVM

* update isTypePaused to include CustomConfig status

* fix and reduce isTypePaused logic

* add FulfilledFromEVM event to NFTFulfillmentMinter

* fix bridge cross-VM NFT to EVM errors

* add coverage for Cadence-native cross-VM NFT to EVM

* add Cadence-native cross-VM NFT test case

* add draft EVM-native cross-VM test case - blocked by from EVM route

* update EVM call transaction interface

* update bridge contract comments

* add test_helper methods to consolidate test code

* restructure update_nft_from_bridged_tests to allow for control over sequence of events

* remove contract comment

* update COA callback in FlowEVMBridgeAccessor.withdrawNFT

* add EVM-native cross-VM bridging from EVM

* add Cadence-native cross-VM NFT from EVM path

* consolidate bridgeNFTFromEVM logic for cross-VM NFTs

* fix EVM-native cross-VM NFT from EVM path

* add internal handler for bridging updated NFTs from EVM

* add EVM-native cross-VM NFT test coverage

* consolidate test code

* add updated cross-VM NFT test case bridging from EVM

* add test coverage for Cadence-native updated cross-VM NFTs

* update code doc comments

* simplify FlowEVMBridgeAccessor.isValidEVMTarget

* move internal bridge helpers

* update contract comments

* add Cadence-native migration transaction & test case

* add Cadence-native ERC721 wrapper example

* fix bug in handling wrapped ERC721 from EVM & add test case

* add balance script

* update EVM dependency hash after latest HCU

* update view only calls to EVM to use dryCall to reduce computation costs

---------

Co-authored-by: Joshua Hannan <[email protected]>
@sisyphusSmiling
Fix dual-onboarded cross-VM registration edge case (#185)
edbd6c0 
* fix dual-onboarded cross-VM registration edge case

* update legacy association check in cross-VM NFT registration
@sisyphusSmiling sisyphusSmiling added Feature Deployment Requires contract deployment or update labels Apr 30, 2025
@github-project-automation github-project-automation bot moved this to 👀 In Review in 🌊 Flow 4D Apr 30, 2025
@sisyphusSmiling sisyphusSmiling moved this to In Review in FlowEVM Bridge Apr 30, 2025
@sisyphusSmiling sisyphusSmiling self-assigned this Apr 30, 2025
sisyphusSmiling and others added 2 commits April 30, 2025 11:38
@sisyphusSmiling
enable Cadence-native NFTs to update their bridged ERC721 symbol (#187)
1b4003e
@sisyphusSmiling @joshuahannan
Add EVM-native NFT to cross-VM update test coverage (#183)
de2cc3d 
* update pause by type to include custom associations

* update conditional to assertion

* Apply suggestions from code review

Co-authored-by: Joshua Hannan <[email protected]>

* fix failing test case on pausing by type

* draft updates to bridgeNFTToEVM()

* add mustBurnERC721 util method

* add path for Cadence-native cross-VM NFT to EVM

* add EVM-native cross-VM NFT to EVM path

* update ExampleCadenceNativeNFT minting process

* fix ExampleCadenceNativeNFT collection data resolution

* begin test coverage for Cadence-native NFT to EVM

* update isTypePaused to include CustomConfig status

* fix and reduce isTypePaused logic

* add FulfilledFromEVM event to NFTFulfillmentMinter

* fix bridge cross-VM NFT to EVM errors

* add coverage for Cadence-native cross-VM NFT to EVM

* add Cadence-native cross-VM NFT test case

* add draft EVM-native cross-VM test case - blocked by from EVM route

* update EVM call transaction interface

* update bridge contract comments

* add test_helper methods to consolidate test code

* restructure update_nft_from_bridged_tests to allow for control over sequence of events

* remove contract comment

* update COA callback in FlowEVMBridgeAccessor.withdrawNFT

* add EVM-native cross-VM bridging from EVM

* add Cadence-native cross-VM NFT from EVM path

* consolidate bridgeNFTFromEVM logic for cross-VM NFTs

* fix EVM-native cross-VM NFT from EVM path

* add internal handler for bridging updated NFTs from EVM

* add EVM-native cross-VM NFT test coverage

* consolidate test code

* add updated cross-VM NFT test case bridging from EVM

* add test coverage for Cadence-native updated cross-VM NFTs

* update code doc comments

* simplify FlowEVMBridgeAccessor.isValidEVMTarget

* move internal bridge helpers

* update contract comments

* add Cadence-native migration transaction & test case

* add Cadence-native ERC721 wrapper example

* fix bug in handling wrapped ERC721 from EVM & add test case

* add balance script

* update EVM dependency hash after latest HCU

* add openzepplin-contracts-upgradeable dependency & update remappings

* update solidity dependencies

* add upgradeable EVM-native NFT solidity contracts & initial tests

* rename Cadence test suite file

* add initial tests covering EVM-native NFT updating to cross-VM

* add FlowEVMBridge.BridgedNFTBurned event

* add getters for new and old asset type associations

* move Solidity contract

* add util method isBridgeDefined

* add migration test cases covering EVM-native NFTs

* update legacy x cross-vm type getters

* add test coverage for legacy x cross-vm type getters

* add legacy x cross-vm EVM address getters

* include test coverage for new legacy x cross-VM EVM address getters

* add uuid value to BridgedNFTBurned event

---------

Co-authored-by: Joshua Hannan <[email protected]>
@sisyphusSmiling sisyphusSmiling mentioned this pull request Apr 30, 2025
Merged
@sisyphusSmiling sisyphusSmiling marked this pull request as ready for review May 1, 2025 19:02
@sisyphusSmiling sisyphusSmiling requested a review from a team as a code owner May 1, 2025 19:02
@codecov-commenter
Copy link

codecov-commenter commented May 1, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 72.86357% with 181 lines in your changes missing coverage. Please review.

Project coverage is 81.46%. Comparing base (02aab3f) to head (31a091f).
Report is 238 commits behind head on main.

Files with missing lines Patch % Lines
...oss-vm-nfts/ExampleEVMNativeNFTGivenEVMAddress.cdc 43.95% 51 Missing ⚠️
...ample-assets/cross-vm-nfts/ExampleEVMNativeNFT.cdc 50.51% 48 Missing ⚠️
...e-assets/cross-vm-nfts/ExampleCadenceNativeNFT.cdc 72.04% 26 Missing ⚠️
cadence/contracts/bridge/FlowEVMBridge.cdc 89.38% 19 Missing ⚠️
cadence/contracts/bridge/FlowEVMBridgeConfig.cdc 75.00% 12 Missing ⚠️
cadence/contracts/bridge/FlowEVMBridgeUtils.cdc 86.74% 11 Missing ⚠️
...ntracts/bridge/FlowEVMBridgeCustomAssociations.cdc 75.00% 8 Missing ⚠️
...cts/bridge/FlowEVMBridgeCustomAssociationTypes.cdc 93.10% 2 Missing ⚠️
...adence/contracts/bridge/FlowEVMBridgeNFTEscrow.cdc 50.00% 2 Missing ⚠️
.../tests/contracts/MaliciousNFTFulfillmentMinter.cdc 50.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #188      +/-   ##
==========================================
- Coverage   86.19%   81.46%   -4.74%     
==========================================
  Files          18       24       +6     
  Lines         884     1597     +713     
==========================================
+ Hits          762     1301     +539     
- Misses        122      296     +174

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sisyphusSmiling sisyphusSmiling changed the title [WIP - DO NOT MERGE] FLIP-318: VM Bridge Support for Cross-VM NFTs FLIP-318: VM Bridge Support for Cross-VM NFTs May 1, 2025
@github-project-automation github-project-automation bot moved this from In Review to Done in FlowEVM Bridge May 2, 2025
@sisyphusSmiling
Copy link
Contributor Author

Thanks for approving @joshuahannan. Waiting on a new Flow CLI release with fix for onflow/flow-cli#1947 before deploying changes to Testnet.

@sisyphusSmiling sisyphusSmiling removed the status in 🌊 Flow 4D May 2, 2025
@sisyphusSmiling sisyphusSmiling moved this to ✅ Done in 🌊 Flow 4D May 2, 2025
@joshuahannan
Adds full balance script (#189)
31a091f 
* add script to get combined Cadence and COA balance

* change to return zero balance if not successful and return all balances

* use identifier

* add decimals and use UFix64 for Cadence balance
@sisyphusSmiling
Copy link
Contributor Author

Unblocked by latest Flow CLI v2.2.14. However, I'm going to be traveling and unavailable this afternoon and would like to be available should any post-deployment issues come up. The plan then is to deploy to Testnet on Monday, May 12th.

@joshuahannan
Copy link
Member

@sisyphusSmiling Can we merge this now that testnet is deployed?

@sisyphusSmiling sisyphusSmiling merged commit ce3bacc into main May 19, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshuahannan joshuahannan joshuahannan approved these changes

Assignees

@sisyphusSmiling sisyphusSmiling

Labels
Deployment Requires contract deployment or update Feature
Projects
FlowEVM Bridge
Status: Done
🌊 Flow 4D
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sisyphusSmiling @codecov-commenter @joshuahannan