Skip to content

Commit

Permalink
quality cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
jhweintraub committed Sep 23, 2024
1 parent 3301bae commit ddef2db
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 16 deletions.
8 changes: 4 additions & 4 deletions contracts/gas-snapshots/ccip.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions contracts/gas-snapshots/shared.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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));
}
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 14 additions & 2 deletions contracts/src/v0.8/shared/token/ERC20/BurnMintERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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;
}

// ================================================================
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;

Expand Down

0 comments on commit ddef2db

Please sign in to comment.