diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot index 41cf370200..7a241e9cb0 100644 --- a/contracts/gas-snapshots/ccip.gas-snapshot +++ b/contracts/gas-snapshots/ccip.gas-snapshot @@ -973,10 +973,10 @@ TokenPoolAndProxy:test_setPreviousPool_Success() (gas: 3070731) TokenPoolAndProxyMigration:test_tokenPoolMigration_Success_1_2() (gas: 6434801) TokenPoolAndProxyMigration:test_tokenPoolMigration_Success_1_4() (gas: 6634934) TokenPoolFactoryTests:test_TokenPoolFactory_Constructor_Revert() (gas: 4141230) -TokenPoolFactoryTests:test_createTokenPool_ExistingRemoteToken_AndPredictPool_Success() (gas: 15409035) -TokenPoolFactoryTests:test_createTokenPool_WithNoExistingRemoteContracts_predict_Success() (gas: 15678921) -TokenPoolFactoryTests:test_createTokenPool_WithNoExistingTokenOnRemoteChain_Success() (gas: 5732918) -TokenPoolFactoryTests:test_createTokenPool_WithRemoteTokenAndRemotePool_Success() (gas: 5887058) +TokenPoolFactoryTests:test_createTokenPool_ExistingRemoteToken_AndPredictPool_Success() (gas: 15422302) +TokenPoolFactoryTests:test_createTokenPool_WithNoExistingRemoteContracts_predict_Success() (gas: 15692550) +TokenPoolFactoryTests:test_createTokenPool_WithNoExistingTokenOnRemoteChain_Success() (gas: 5740699) +TokenPoolFactoryTests:test_createTokenPool_WithRemoteTokenAndRemotePool_Success() (gas: 5894838) TokenPoolFactoryTests:test_updateRemoteChainConfig_Success() (gas: 86259) TokenPoolWithAllowList_applyAllowListUpdates:test_AllowListNotEnabled_Revert() (gas: 1979943) TokenPoolWithAllowList_applyAllowListUpdates:test_OnlyOwner_Revert() (gas: 12113) diff --git a/contracts/gas-snapshots/shared.gas-snapshot b/contracts/gas-snapshots/shared.gas-snapshot index f4b071612b..70b9ccc982 100644 --- a/contracts/gas-snapshots/shared.gas-snapshot +++ b/contracts/gas-snapshots/shared.gas-snapshot @@ -21,7 +21,7 @@ BurnMintERC20_burnFromAlias:testBurnFromSuccess() (gas: 57932) BurnMintERC20_burnFromAlias:testExceedsBalanceReverts() (gas: 35880) BurnMintERC20_burnFromAlias:testInsufficientAllowanceReverts() (gas: 21869) BurnMintERC20_burnFromAlias:testSenderNotBurnerReverts() (gas: 32137) -BurnMintERC20_constructor:testConstructorSuccess() (gas: 1722070) +BurnMintERC20_constructor:testConstructorSuccess() (gas: 1737298) BurnMintERC20_decreaseApproval:testDecreaseApprovalSuccess() (gas: 31123) BurnMintERC20_grantMintAndBurnRoles:testGrantMintAndBurnRolesSuccess() (gas: 121170) BurnMintERC20_grantRole:testGrantBurnAccessSuccess() (gas: 53407) @@ -31,7 +31,7 @@ BurnMintERC20_increaseApproval:testIncreaseApprovalSuccess() (gas: 44121) BurnMintERC20_mint:testBasicMintSuccess() (gas: 52689) BurnMintERC20_mint:testMaxSupplyExceededReverts() (gas: 50429) BurnMintERC20_mint:testSenderNotMinterReverts() (gas: 30039) -BurnMintERC20_supportsInterface:testConstructorSuccess() (gas: 11072) +BurnMintERC20_supportsInterface:testConstructorSuccess() (gas: 11123) BurnMintERC20_transfer:testInvalidAddressReverts() (gas: 10661) BurnMintERC20_transfer:testTransferSuccess() (gas: 42277) BurnMintERC677_approve:testApproveSuccess() (gas: 55512) diff --git a/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory.sol b/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory.sol index fc671c31d0..8b6612a1a1 100644 --- a/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory.sol +++ b/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory.sol @@ -56,7 +56,7 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { address private immutable i_rmnProxy; address private immutable i_ccipRouter; - mapping(uint64 remoteChainSelector => RemoteChainConfig) internal s_remoteChainConfigs; + mapping(uint64 remoteChainSelector => RemoteChainConfig remoteConfig) internal s_remoteChainConfigs; constructor(address tokenAdminRegistry, address tokenAdminModule, address rmnProxy, address ccipRouter) { if ( @@ -92,8 +92,7 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { // Set the token pool for token in the token admin registry since this contract is the token and pool owner _setTokenPoolInTokenAdminRegistry(token, pool); - // Transfer the ownership of the token to the msg.sender. - // This is a 2 step process and must be accepted in a separate tx. + // Begin the 2 step ownership transfer of the newly deployed token to the msg.sender IOwnable(token).transferOwnership(msg.sender); return (token, pool); @@ -153,7 +152,7 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { // If the user provides the empty parameter flag, then the address of the token needs to be predicted // otherwise the address provided is used. if (bytes4(remoteTokenPool.remoteTokenAddress) == EMPTY_PARAMETER_FLAG) { - // The user must provide the initCode for the remote token, so we can predict its address correctly. It's + // The user must provide the initCode for the remote token, so its address can be predicted correctly. It's // provided in the remoteTokenInitCode field for the remoteTokenPool remoteTokenPool.remoteTokenAddress = abi.encode( salt.computeAddress(keccak256(remoteTokenPool.remoteTokenInitCode), remoteChainConfig.remotePoolFactory) @@ -165,7 +164,7 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { // Generate the initCode that will be used on the remote chain. It is assumed that tokenInitCode // will be the same on all chains, so it can be reused here. - // Combine the initCode with the initArgs to create the full initCode + // Combine the initCode with the initArgs to create the full deployment code bytes32 remotePoolInitcode = keccak256( bytes.concat( type(BurnMintTokenPool).creationCode, @@ -179,7 +178,7 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { ) ); - // Predict the address of the undeployed contract on the destination chain + // Abi encode the computed remote address so it can be used as bytes in the chain update remoteTokenPool.remotePoolAddress = abi.encode(salt.computeAddress(remotePoolInitcode, remoteChainConfig.remotePoolFactory)); } @@ -195,13 +194,13 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { } // If the user doesn't want to provide any special parameters which may be needed for a custom the token pool then - /// use the standard burn/mint token pool params. Since the user can provide custom token pool init code, + // use the standard burn/mint token pool params. Since the user can provide custom token pool init code, // they must also provide custom constructor args. if (bytes4(tokenPoolInitArgs) == EMPTY_PARAMETER_FLAG) { tokenPoolInitArgs = abi.encode(token, new address[](0), i_rmnProxy, i_ccipRouter); } - // Construct the code that will be deployed from the initCode and the initArgs + // Construct the deployment code from the initCode and the initArgs and then deploy address poolAddress = Create2.deploy(0, salt, abi.encodePacked(tokenPoolInitCode, tokenPoolInitArgs)); // Apply the chain updates to the token pool @@ -238,6 +237,7 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { for (uint256 i = 0; i < remoteChainConfigs.length; ++i) { RemoteChainConfig memory remoteConfig = remoteChainConfigs[i].remoteChainConfig; + // Zero address validation check if ( remoteChainConfigs[i].remoteChainSelector == 0 || remoteConfig.remotePoolFactory == address(0) || remoteConfig.remoteRouter == address(0) || remoteConfig.remoteRMNProxy == address(0) diff --git a/contracts/src/v0.8/shared/token/ERC20/BurnMintERC20.sol b/contracts/src/v0.8/shared/token/ERC20/BurnMintERC20.sol index 585372af4c..82f1e3cfe1 100644 --- a/contracts/src/v0.8/shared/token/ERC20/BurnMintERC20.sol +++ b/contracts/src/v0.8/shared/token/ERC20/BurnMintERC20.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.0; import {IBurnMintERC20} from "../ERC20/IBurnMintERC20.sol"; +import {IOwnable} from "../../interfaces/IOwnable.sol"; import {OwnerIsCreator} from "../../access/OwnerIsCreator.sol"; @@ -69,7 +70,8 @@ contract BurnMintERC20 is IBurnMintERC20, IERC165, ERC20Burnable, OwnerIsCreator return interfaceId == type(IERC20).interfaceId || interfaceId == type(IBurnMintERC20).interfaceId || - interfaceId == type(IERC165).interfaceId; + interfaceId == type(IERC165).interfaceId || + interfaceId == type(IOwnable).interfaceId; } // ================================================================ @@ -99,11 +101,16 @@ contract BurnMintERC20 is IBurnMintERC20, IERC165, ERC20Burnable, OwnerIsCreator } /// @dev Exists to be backwards compatible with the older naming convention. + /// @param spender the account being approved to spend on the users' behalf. + /// @param subtractedValue the amount being removed from the approval. + /// @return success Bool to return if the approval was successfully decreased. function decreaseApproval(address spender, uint256 subtractedValue) external returns (bool success) { return decreaseAllowance(spender, subtractedValue); } /// @dev Exists to be backwards compatible with the older naming convention. + /// @param spender the account being approved to spend on the users' behalf. + /// @param addedValue the amount being added to the approval. function increaseApproval(address spender, uint256 addedValue) external { increaseAllowance(spender, addedValue); } @@ -132,6 +139,7 @@ contract BurnMintERC20 is IBurnMintERC20, IERC165, ERC20Burnable, OwnerIsCreator /// @inheritdoc IBurnMintERC20 /// @dev Alias for BurnFrom for compatibility with the older naming convention. /// @dev Uses burnFrom for all validation & logic. + function burn(address account, uint256 amount) public virtual override { burnFrom(account, amount); } @@ -175,6 +183,7 @@ contract BurnMintERC20 is IBurnMintERC20, IERC165, ERC20Burnable, OwnerIsCreator /// @notice Grants burn role to the given address. /// @dev only the owner can call this function. + /// @param burner the address to grant the burner role to function grantBurnRole(address burner) public onlyOwner { if (s_burners.add(burner)) { emit BurnAccessGranted(burner); @@ -183,6 +192,7 @@ contract BurnMintERC20 is IBurnMintERC20, IERC165, ERC20Burnable, OwnerIsCreator /// @notice Revokes mint role for the given address. /// @dev only the owner can call this function. + /// @param minter the address to revoke the mint role from. function revokeMintRole(address minter) public onlyOwner { if (s_minters.remove(minter)) { emit MintAccessRevoked(minter); @@ -191,6 +201,7 @@ contract BurnMintERC20 is IBurnMintERC20, IERC165, ERC20Burnable, OwnerIsCreator /// @notice Revokes burn role from the given address. /// @dev only the owner can call this function + /// @param burner the address to revoke the burner role from function revokeBurnRole(address burner) public onlyOwner { if (s_burners.remove(burner)) { emit BurnAccessRevoked(burner); @@ -214,7 +225,8 @@ contract BurnMintERC20 is IBurnMintERC20, IERC165, ERC20Burnable, OwnerIsCreator /// @notice Transfers the CCIPAdmin role to a new address /// @dev only the owner can call this function, NOT the current ccipAdmin, and 1-step ownership transfer is used. - /// @param newAdmin The address to transfer the CCIPAdmin role to. Setting to address(0) is a valid way to revoke the role + /// @param newAdmin The address to transfer the CCIPAdmin role to. Setting to address(0) is a valid way to revoke + /// the role function setCCIPAdmin(address newAdmin) public onlyOwner { address currentAdmin = s_ccipAdmin;