Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for granting burn/mint rights when deploying burn/mint token pools for additional burn/mint token reference types (including ERC677-based burn/mint tokens), and expands regression tests to cover these cases.
Changes:
- Introduces a new
BurnMintTokencontract type constant and extends burn/mint token type detection. - Updates the v2.0.0 token adapter to grant mint/burn roles even when the pool already exists, and selects the correct grant operation for ERC677 tokens.
- Adds a new v1.0.0 operation for
BurnMintERC677.grantMintAndBurnRolesand extends v1.0.0 pool adapter role-grant switching. - Expands adapter tests to cover multiple token ref types/qualifiers and ERC677 tokens.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| deployment/utils/common.go | Adds BurnMintToken contract type used for burn/mint token classification. |
| chains/evm/deployment/v2_0_0/adapters/tokens.go | Grants roles for already-deployed pools and routes role granting to ERC677-specific operation when applicable. |
| chains/evm/deployment/v2_0_0/adapters/tokens_test.go | Expands regression coverage to include multiple burn/mint token ref types and ERC677 deployment. |
| chains/evm/deployment/v1_0_0/operations/burn_mint_erc677/burn_mint_erc677.go | Adds ERC677 role-grant write operation wrapper. |
| chains/evm/deployment/v1_0_0/adapters/pool_adapter.go | Adds ERC677 token type cases to role-grant dispatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -37,6 +37,7 @@ const ( | |||
| // Pools | |||
| TokenPoolFactory cldf.ContractType = "TokenPoolFactory" | |||
| LinkToken cldf.ContractType = "LinkToken" | |||
| require.NoError(t, ds.Addresses().Add(datastore.AddressRef{ | ||
| ChainSelector: tc.chainSel, | ||
| Address: tokenAddr.Hex(), | ||
| Type: tc.tokenRefType, |
| } | ||
| return owner == caller, nil | ||
| }, | ||
| Validate: func(address common.Address) error { return nil }, |
| batchOp, err := grantMintBurnRoles(matches[0]) | ||
| if err != nil { | ||
| return sequences.OnChainOutput{}, err | ||
| } | ||
| if batchOp == nil { | ||
| return sequences.OnChainOutput{}, nil | ||
| } | ||
| return sequences.OnChainOutput{BatchOps: []mcms_types.BatchOperation{*batchOp}}, nil |
| toknRef, lookupErr := datastore_utils.FindAndFormatRef(input.ExistingDataStore, datastore.AddressRef{ | ||
| ChainSelector: input.ChainSelector, | ||
| Address: tokenAddr, | ||
| }, input.ChainSelector, datastore_utils.FullRef) | ||
| if lookupErr != nil || !isBurnMintTokenType(toknRef.Type) { | ||
| return nil, nil | ||
| } |
|
|
|
||
| var ContractType cldf_deployment.ContractType = cciputils.BurnMintToken | ||
|
|
||
| const burnMintERC677ABI = `[{"inputs":[],"name":"BURN_MINT_ADMIN_ROLE","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"DEFAULT_ADMIN_ROLE","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"bytes32","name":"role","type":"bytes32"}],"name":"getRoleAdmin","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"bytes32","name":"role","type":"bytes32"},{"internalType":"address","name":"account","type":"address"}],"name":"grantRole","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"bytes32","name":"role","type":"bytes32"},{"internalType":"address","name":"account","type":"address"}],"name":"hasRole","outputs":[{"internalType":"bool","name":"","type":"bool"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"owner","outputs":[{"internalType":"address","name":"","type":"address"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"burnAndMinter","type":"address"}],"name":"grantMintAndBurnRoles","outputs":[],"stateMutability":"nonpayable","type":"function"}]` |
There was a problem hiding this comment.
@tt-cll quick heads up - looks like chainlink-evm already has this ABI supported:
Also, I noticed a few discrepancies with this ABI and the BurnMintERC677 source code - for example DEFAULT_ADMIN_ROLE, getRoleAdmin, etc. are not present in the BurnMintERC677 contract if I'm not mistaken 👀
Let me know if this is legit and I can help fix this in the PR I have here: #2015
There was a problem hiding this comment.
yeah that is a bug. Let's fix it as part of the other PR
No description provided.