From fe99818cd180c4498cdd833efd2d9d83ecba784a Mon Sep 17 00:00:00 2001 From: excaliborr Date: Wed, 3 Jul 2024 10:07:38 -0400 Subject: [PATCH 1/3] fix: potential second minter blocks burnLockedUSDC --- script/sepolia/DeployBase.s.sol | 2 +- script/sepolia/DeployOptimism.s.sol | 2 +- src/contracts/L1OpUSDCBridgeAdapter.sol | 10 +++++++++- src/contracts/L2OpUSDCFactory.sol | 12 ++++++------ src/interfaces/IL1OpUSDCFactory.sol | 2 +- test/unit/L1OpUSDCBridgeAdapter.t.sol | 25 +++++++++++++++++++++++++ 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/script/sepolia/DeployBase.s.sol b/script/sepolia/DeployBase.s.sol index a73279e5..ea283d84 100644 --- a/script/sepolia/DeployBase.s.sol +++ b/script/sepolia/DeployBase.s.sol @@ -9,7 +9,7 @@ import {USDCInitTxs} from 'src/contracts/utils/USDCInitTxs.sol'; contract DeployBase is Script { address public constant L1_MESSENGER = 0xC34855F4De64F1840e5686e64278da901e261f20; - uint32 public constant MIN_GAS_LIMIT_DEPLOY = 8_000_000; + uint32 public constant MIN_GAS_LIMIT_DEPLOY = 1_500_000; IL1OpUSDCFactory public immutable L1_FACTORY = IL1OpUSDCFactory(vm.envAddress('L1_FACTORY_SEPOLIA')); address public deployer = vm.rememberKey(vm.envUint('SEPOLIA_DEPLOYER_PK')); diff --git a/script/sepolia/DeployOptimism.s.sol b/script/sepolia/DeployOptimism.s.sol index ed888ee0..7cd15845 100644 --- a/script/sepolia/DeployOptimism.s.sol +++ b/script/sepolia/DeployOptimism.s.sol @@ -9,7 +9,7 @@ import {USDCInitTxs} from 'src/contracts/utils/USDCInitTxs.sol'; contract DeployOptimism is Script { address public constant L1_MESSENGER = 0x58Cc85b8D04EA49cC6DBd3CbFFd00B4B8D6cb3ef; - uint32 public constant MIN_GAS_LIMIT_DEPLOY = 8_000_000; + uint32 public constant MIN_GAS_LIMIT_DEPLOY = 13_000_000; IL1OpUSDCFactory public immutable L1_FACTORY = IL1OpUSDCFactory(vm.envAddress('L1_FACTORY_SEPOLIA')); address public deployer = vm.rememberKey(vm.envUint('SEPOLIA_DEPLOYER_PK')); diff --git a/src/contracts/L1OpUSDCBridgeAdapter.sol b/src/contracts/L1OpUSDCBridgeAdapter.sol index a337ecb5..557b84c4 100644 --- a/src/contracts/L1OpUSDCBridgeAdapter.sol +++ b/src/contracts/L1OpUSDCBridgeAdapter.sol @@ -114,7 +114,15 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { // Burn the USDC tokens if (burnAmount != 0) { - IUSDC(USDC).burn(burnAmount); + // NOTE: This is a very edge case and will only happen if the chain operator adds a second minter on L2 + // So now this adapter doesnt have the full backing supply locked in this contract + // Incase the bridged usdc token has other minters and the supply sent is greater then what we have + // We need to burn the full amount stored in this contract + // This could also cause in-flight messages to fail because of the multiple supply sources + uint256 _balanceOf = IUSDC(USDC).balanceOf(address(this)); + uint256 _burnAmount = burnAmount > _balanceOf ? _balanceOf : burnAmount; + + IUSDC(USDC).burn(_burnAmount); // Set the burn amount to 0 burnAmount = 0; diff --git a/src/contracts/L2OpUSDCFactory.sol b/src/contracts/L2OpUSDCFactory.sol index 56896594..fec6d75c 100644 --- a/src/contracts/L2OpUSDCFactory.sol +++ b/src/contracts/L2OpUSDCFactory.sol @@ -37,16 +37,16 @@ contract L2OpUSDCFactory is IL2OpUSDCFactory { emit USDCImplementationDeployed(_usdcImplementation); // Deploy USDC proxy - bytes memory _usdcProxyCArgs = abi.encode(_usdcImplementation); - bytes memory _usdcProxyInitCode = bytes.concat(USDC_PROXY_CREATION_CODE, _usdcProxyCArgs); - (address _usdcProxy) = _deployCreate(_usdcProxyInitCode); + bytes memory _cachedBytes = abi.encode(_usdcImplementation); + _cachedBytes = bytes.concat(USDC_PROXY_CREATION_CODE, _cachedBytes); + (address _usdcProxy) = _deployCreate(_cachedBytes); emit USDCProxyDeployed(_usdcProxy); // Deploy L2 Adapter address _l2Messenger = 0x4200000000000000000000000000000000000007; - bytes memory _l2AdapterCArgs = abi.encode(_usdcProxy, _l2Messenger, _l1Adapter, _l2AdapterOwner); - bytes memory _l2AdapterInitCode = bytes.concat(type(L2OpUSDCBridgeAdapter).creationCode, _l2AdapterCArgs); - (address _l2Adapter) = _deployCreate(_l2AdapterInitCode); + _cachedBytes = abi.encode(_usdcProxy, _l2Messenger, _l1Adapter, _l2AdapterOwner); + _cachedBytes = bytes.concat(type(L2OpUSDCBridgeAdapter).creationCode, _cachedBytes); + (address _l2Adapter) = _deployCreate(_cachedBytes); emit L2AdapterDeployed(_l2Adapter); // Deploy the FallbackProxyAdmin internally in the L2 Adapter to keep it unique diff --git a/src/interfaces/IL1OpUSDCFactory.sol b/src/interfaces/IL1OpUSDCFactory.sol index f4f602bb..4658f279 100644 --- a/src/interfaces/IL1OpUSDCFactory.sol +++ b/src/interfaces/IL1OpUSDCFactory.sol @@ -57,7 +57,7 @@ interface IL1OpUSDCFactory { function deploy( address _l1Messenger, address _l1AdapterOwner, - L2Deployments memory _l2Deployments + L2Deployments calldata _l2Deployments ) external returns (address _l1Adapter, address _l2Factory, address _l2Adapter); /*/////////////////////////////////////////////////////////////// diff --git a/test/unit/L1OpUSDCBridgeAdapter.t.sol b/test/unit/L1OpUSDCBridgeAdapter.t.sol index 5b112d1e..c3149160 100644 --- a/test/unit/L1OpUSDCBridgeAdapter.t.sol +++ b/test/unit/L1OpUSDCBridgeAdapter.t.sol @@ -397,6 +397,7 @@ contract L1OpUSDCBridgeAdapter_Unit_BurnLockedUSDC is Base { vm.assume(_burnAmount > 0); _mockAndExpect(_usdc, abi.encodeWithSignature('burn(uint256)', _burnAmount), abi.encode(true)); + _mockAndExpect(_usdc, abi.encodeWithSignature('balanceOf(address)', address(adapter)), abi.encode(_burnAmount)); adapter.forTest_setBurnAmount(_burnAmount); @@ -416,6 +417,29 @@ contract L1OpUSDCBridgeAdapter_Unit_BurnLockedUSDC is Base { vm.mockCall( _usdc, abi.encodeWithSignature('burn(address,uint256)', address(adapter), _burnAmount), abi.encode(true) ); + vm.mockCall(_usdc, abi.encodeWithSignature('balanceOf(address)', address(adapter)), abi.encode(_burnAmount)); + + adapter.forTest_setBurnAmount(_burnAmount); + + // Execute + vm.prank(_circle); + adapter.burnLockedUSDC(); + + assertEq(adapter.burnAmount(), 0, 'Burn amount should be set to 0'); + assertEq(adapter.burnCaller(), address(0), 'Circle should be set to 0'); + } + + /** + * @notice Check that balance of gets used if burn amount is greater then the balance + */ + function test_balanceOfUsedIfBurnAmountGt(uint256 _burnAmount, uint256 _balanceOf, address _circle) external { + vm.assume(_burnAmount > 0); + vm.assume(_burnAmount > _balanceOf); + adapter.forTest_setBurnCaller(_circle); + adapter.forTest_setMessengerStatus(IL1OpUSDCBridgeAdapter.Status.Deprecated); + + vm.mockCall(_usdc, abi.encodeWithSignature('burn(address,uint256)', address(adapter), _balanceOf), abi.encode(true)); + vm.mockCall(_usdc, abi.encodeWithSignature('balanceOf(address)', address(adapter)), abi.encode(_balanceOf)); adapter.forTest_setBurnAmount(_burnAmount); @@ -436,6 +460,7 @@ contract L1OpUSDCBridgeAdapter_Unit_BurnLockedUSDC is Base { adapter.forTest_setMessengerStatus(IL1OpUSDCBridgeAdapter.Status.Deprecated); vm.mockCall(_usdc, abi.encodeWithSignature('burn(address)', address(adapter)), abi.encode(true)); + vm.mockCall(_usdc, abi.encodeWithSignature('balanceOf(address)', address(adapter)), abi.encode(_burnAmount)); adapter.forTest_setBurnAmount(_burnAmount); From e087662f690d5f96326653b281c5c5bb14f4dfcb Mon Sep 17 00:00:00 2001 From: excaliborr Date: Wed, 3 Jul 2024 10:10:12 -0400 Subject: [PATCH 2/3] fix: revert script --- script/sepolia/DeployBase.s.sol | 2 +- script/sepolia/DeployOptimism.s.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/script/sepolia/DeployBase.s.sol b/script/sepolia/DeployBase.s.sol index ea283d84..a73279e5 100644 --- a/script/sepolia/DeployBase.s.sol +++ b/script/sepolia/DeployBase.s.sol @@ -9,7 +9,7 @@ import {USDCInitTxs} from 'src/contracts/utils/USDCInitTxs.sol'; contract DeployBase is Script { address public constant L1_MESSENGER = 0xC34855F4De64F1840e5686e64278da901e261f20; - uint32 public constant MIN_GAS_LIMIT_DEPLOY = 1_500_000; + uint32 public constant MIN_GAS_LIMIT_DEPLOY = 8_000_000; IL1OpUSDCFactory public immutable L1_FACTORY = IL1OpUSDCFactory(vm.envAddress('L1_FACTORY_SEPOLIA')); address public deployer = vm.rememberKey(vm.envUint('SEPOLIA_DEPLOYER_PK')); diff --git a/script/sepolia/DeployOptimism.s.sol b/script/sepolia/DeployOptimism.s.sol index 7cd15845..ed888ee0 100644 --- a/script/sepolia/DeployOptimism.s.sol +++ b/script/sepolia/DeployOptimism.s.sol @@ -9,7 +9,7 @@ import {USDCInitTxs} from 'src/contracts/utils/USDCInitTxs.sol'; contract DeployOptimism is Script { address public constant L1_MESSENGER = 0x58Cc85b8D04EA49cC6DBd3CbFFd00B4B8D6cb3ef; - uint32 public constant MIN_GAS_LIMIT_DEPLOY = 13_000_000; + uint32 public constant MIN_GAS_LIMIT_DEPLOY = 8_000_000; IL1OpUSDCFactory public immutable L1_FACTORY = IL1OpUSDCFactory(vm.envAddress('L1_FACTORY_SEPOLIA')); address public deployer = vm.rememberKey(vm.envUint('SEPOLIA_DEPLOYER_PK')); From 33e7289b22eb3f7478dde86597e4d4c5fd2709f9 Mon Sep 17 00:00:00 2001 From: excaliborr Date: Wed, 3 Jul 2024 10:45:48 -0400 Subject: [PATCH 3/3] fix: lint --- src/contracts/L2OpUSDCFactory.sol | 12 ++++++------ test/unit/L1OpUSDCBridgeAdapter.t.sol | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/contracts/L2OpUSDCFactory.sol b/src/contracts/L2OpUSDCFactory.sol index fec6d75c..56896594 100644 --- a/src/contracts/L2OpUSDCFactory.sol +++ b/src/contracts/L2OpUSDCFactory.sol @@ -37,16 +37,16 @@ contract L2OpUSDCFactory is IL2OpUSDCFactory { emit USDCImplementationDeployed(_usdcImplementation); // Deploy USDC proxy - bytes memory _cachedBytes = abi.encode(_usdcImplementation); - _cachedBytes = bytes.concat(USDC_PROXY_CREATION_CODE, _cachedBytes); - (address _usdcProxy) = _deployCreate(_cachedBytes); + bytes memory _usdcProxyCArgs = abi.encode(_usdcImplementation); + bytes memory _usdcProxyInitCode = bytes.concat(USDC_PROXY_CREATION_CODE, _usdcProxyCArgs); + (address _usdcProxy) = _deployCreate(_usdcProxyInitCode); emit USDCProxyDeployed(_usdcProxy); // Deploy L2 Adapter address _l2Messenger = 0x4200000000000000000000000000000000000007; - _cachedBytes = abi.encode(_usdcProxy, _l2Messenger, _l1Adapter, _l2AdapterOwner); - _cachedBytes = bytes.concat(type(L2OpUSDCBridgeAdapter).creationCode, _cachedBytes); - (address _l2Adapter) = _deployCreate(_cachedBytes); + bytes memory _l2AdapterCArgs = abi.encode(_usdcProxy, _l2Messenger, _l1Adapter, _l2AdapterOwner); + bytes memory _l2AdapterInitCode = bytes.concat(type(L2OpUSDCBridgeAdapter).creationCode, _l2AdapterCArgs); + (address _l2Adapter) = _deployCreate(_l2AdapterInitCode); emit L2AdapterDeployed(_l2Adapter); // Deploy the FallbackProxyAdmin internally in the L2 Adapter to keep it unique diff --git a/test/unit/L1OpUSDCBridgeAdapter.t.sol b/test/unit/L1OpUSDCBridgeAdapter.t.sol index c3149160..a7aaf972 100644 --- a/test/unit/L1OpUSDCBridgeAdapter.t.sol +++ b/test/unit/L1OpUSDCBridgeAdapter.t.sol @@ -438,6 +438,7 @@ contract L1OpUSDCBridgeAdapter_Unit_BurnLockedUSDC is Base { adapter.forTest_setBurnCaller(_circle); adapter.forTest_setMessengerStatus(IL1OpUSDCBridgeAdapter.Status.Deprecated); + // solhint-disable-next-line max-line-length vm.mockCall(_usdc, abi.encodeWithSignature('burn(address,uint256)', address(adapter), _balanceOf), abi.encode(true)); vm.mockCall(_usdc, abi.encodeWithSignature('balanceOf(address)', address(adapter)), abi.encode(_balanceOf));