CCIP-4130 CCIP-Compatible Generic BurnMintERC20 #15123
Conversation
Static analysis results are availableHey @jhweintraub, you can view Slither reports in the job summary here or download them as artifact here. |
Solidity Review Jira issueHey! We have taken the liberty to link this PR to a Jira issue for Solidity Review. This is a new feature, that's currently in the pilot phase, so please make sure that the linkage is correct. In a contrary case, please update it manually in JIRA and replace Solidity Review issue key in the changeset file with the correct one. Any changes to the Solidity Review Jira issue should be reflected in the changeset file. If you need to update the issue key, please do so manually in the following changeset file: This PR has been linked to Solidity Review Jira issue: CCIP-3966 |
jhweintraub
force-pushed
the
feat/genericERC20
branch
from
23b7473
to
0744916
Compare
jhweintraub
requested review from
winder,
rstout,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara and
0xAustinWang
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
|
|
||
| /// @notice A basic ERC20 compatible token contract with burn and minting roles. | ||
| /// @dev The total supply can be limited during deployment. | ||
| contract BurnMintERC20 is IBurnMintERC20, IGetCCIPAdmin, IERC165, ERC20Burnable, Ownable2StepMsgSender { |
There was a problem hiding this comment.
I think it makes sense to use OZ AccessControl over Ownable2StepMsgSender. Should reduce the number of lines required in this contract by half?
There was a problem hiding this comment.
remove Ownable2StepMsgSender and replace all the roles with AC? Makes sense to me. Simplifies a lot.
| //go:generate go run ../generation/generate/wrap.go ../../../contracts/solc/v0.8.19/LinkToken/LinkToken.abi ../../../contracts/solc/v0.8.19/LinkToken/LinkToken.bin LinkToken link_token | ||
| //go:generate go run ../generation/generate/wrap.go ../../../contracts/solc/v0.8.19/BurnMintERC20/BurnMintERC20.abi ../../../contracts/solc/v0.8.19/BurnMintERC20/BurnMintERC20.bin BurnMintERC20 burn_mint_erc20 |
There was a problem hiding this comment.
wym? That we don't need it since we have the link_token and erc677? There's different functions which is why its included.
app-token-issuer-infra-releng
bot
requested review from
a team
as code owners
jhweintraub
force-pushed
the
feat/genericERC20
branch
from
c861c21
to
0744916
Compare
Generic BurnMintERC20 that is compatible with CCIPGetAdmin() functionality. Includes tests which are named and organized based on newly established naming conventions for tests. BurnMintERC20 implements OZ AccessControl instead of
Ownable2Stepbut maintains a singular ccip admin