From 721e881814ae5f98a978f7b136bd98d606b907af Mon Sep 17 00:00:00 2001 From: excaliborr Date: Mon, 8 Jul 2024 08:22:55 -0400 Subject: [PATCH 1/2] fix: deadline in signature checK --- script/sepolia/DeployOptimism.s.sol | 2 +- src/contracts/L1OpUSDCBridgeAdapter.sol | 9 +++++---- src/contracts/L2OpUSDCBridgeAdapter.sol | 13 +++++++------ src/contracts/L2OpUSDCFactory.sol | 5 +++-- test/integration/L1OpUSDCBridgeAdapter.t.sol | 11 ++++++----- test/integration/L2OpUSDCBridgeAdapter.t.sol | 12 ++++++------ test/unit/L1OpUSDCBridgeAdapter.t.sol | 12 ++++++++---- test/unit/L2OpUSDCBridgeAdapter.t.sol | 12 ++++++++---- test/utils/Helpers.sol | 4 +++- 9 files changed, 47 insertions(+), 33 deletions(-) diff --git a/script/sepolia/DeployOptimism.s.sol b/script/sepolia/DeployOptimism.s.sol index ed888ee0..cb5e561c 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 = 9_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 557b84c4..fb7366f2 100644 --- a/src/contracts/L1OpUSDCBridgeAdapter.sol +++ b/src/contracts/L1OpUSDCBridgeAdapter.sol @@ -28,7 +28,7 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { /** * @notice Modifier to check if the sender is the linked adapter through the messenger */ - modifier checkSender() { + modifier onlyLinkedAdapter() { if (msg.sender != MESSENGER || ICrossDomainMessenger(MESSENGER).xDomainMessageSender() != LINKED_ADAPTER) { revert IOpUSDCBridgeAdapter_InvalidSender(); } @@ -93,7 +93,7 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { * @param _amount The amount of USDC tokens that will be burned * @dev Only callable by a whitelisted messenger during its migration process */ - function setBurnAmount(uint256 _amount) external checkSender { + function setBurnAmount(uint256 _amount) external onlyLinkedAdapter { if (messengerStatus != Status.Upgrading) revert IOpUSDCBridgeAdapter_NotUpgrading(); burnAmount = _amount; @@ -230,7 +230,8 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { if (block.timestamp > _deadline) revert IOpUSDCBridgeAdapter_MessageExpired(); // Hash the message - bytes32 _messageHash = keccak256(abi.encode(address(this), block.chainid, _to, _amount, userNonce[_signer]++)); + bytes32 _messageHash = + keccak256(abi.encode(address(this), block.chainid, _to, _amount, _deadline, userNonce[_signer]++)); _checkSignature(_signer, _messageHash, _signature); @@ -251,7 +252,7 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { * @param _user The user to mint the bridged representation for * @param _amount The amount of tokens to mint */ - function receiveMessage(address _user, uint256 _amount) external override checkSender { + function receiveMessage(address _user, uint256 _amount) external override onlyLinkedAdapter { // Transfer the tokens to the user IUSDC(USDC).safeTransfer(_user, _amount); emit MessageReceived(_user, _amount, MESSENGER); diff --git a/src/contracts/L2OpUSDCBridgeAdapter.sol b/src/contracts/L2OpUSDCBridgeAdapter.sol index 696cc53d..63280a9f 100644 --- a/src/contracts/L2OpUSDCBridgeAdapter.sol +++ b/src/contracts/L2OpUSDCBridgeAdapter.sol @@ -39,7 +39,7 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { /** * @notice Modifier to check if the sender is the linked adapter through the messenger */ - modifier checkSender() { + modifier onlyLinkedAdapter() { if (msg.sender != MESSENGER || ICrossDomainMessenger(MESSENGER).xDomainMessageSender() != LINKED_ADAPTER) { revert IOpUSDCBridgeAdapter_InvalidSender(); } @@ -74,7 +74,7 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { * @param _roleCaller The address that will be allowed to transfer the USDC roles * @param _setBurnAmountMinGasLimit Minimum gas limit that the setBurnAmount message can be executed on L1 */ - function receiveMigrateToNative(address _roleCaller, uint32 _setBurnAmountMinGasLimit) external checkSender { + function receiveMigrateToNative(address _roleCaller, uint32 _setBurnAmountMinGasLimit) external onlyLinkedAdapter { isMessagingDisabled = true; roleCaller = _roleCaller; @@ -109,7 +109,7 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { /** * @notice Receive the stop messaging message from the linked adapter and stop outgoing messages */ - function receiveStopMessaging() external checkSender { + function receiveStopMessaging() external onlyLinkedAdapter { isMessagingDisabled = true; emit MessagingStopped(MESSENGER); @@ -118,7 +118,7 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { /** * @notice Resume messaging after it was stopped */ - function receiveResumeMessaging() external checkSender { + function receiveResumeMessaging() external onlyLinkedAdapter { // NOTE: This is safe because this message can only be received when messaging is not deprecated on the L1 messenger isMessagingDisabled = false; @@ -176,7 +176,8 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { if (block.timestamp > _deadline) revert IOpUSDCBridgeAdapter_MessageExpired(); // Hash the message - bytes32 _messageHash = keccak256(abi.encode(address(this), block.chainid, _to, _amount, userNonce[_signer]++)); + bytes32 _messageHash = + keccak256(abi.encode(address(this), block.chainid, _to, _amount, _deadline, userNonce[_signer]++)); _checkSignature(_signer, _messageHash, _signature); @@ -199,7 +200,7 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { * @param _user The user to mint the bridged representation for * @param _amount The amount of tokens to mint */ - function receiveMessage(address _user, uint256 _amount) external override checkSender { + function receiveMessage(address _user, uint256 _amount) external override onlyLinkedAdapter { // Mint the tokens to the user IUSDC(USDC).mint(_user, _amount); emit MessageReceived(_user, _amount, MESSENGER); diff --git a/src/contracts/L2OpUSDCFactory.sol b/src/contracts/L2OpUSDCFactory.sol index ff016210..81b240e5 100644 --- a/src/contracts/L2OpUSDCFactory.sol +++ b/src/contracts/L2OpUSDCFactory.sol @@ -15,6 +15,8 @@ import {IUSDC} from 'interfaces/external/IUSDC.sol'; * L2 contracts have the same address on different L2s when triggered by different owners. */ contract L2OpUSDCFactory is IL2OpUSDCFactory { + address constant L2_MESSENGER = 0x4200000000000000000000000000000000000007; + /** * @notice Deploys the USDC implementation, proxy, and L2 adapter contracts all at once, and then initializes the USDC * @param _l1Adapter The address of the L1 adapter contract @@ -43,8 +45,7 @@ contract L2OpUSDCFactory is IL2OpUSDCFactory { emit USDCProxyDeployed(_usdcProxy); // Deploy L2 Adapter - address _l2Messenger = 0x4200000000000000000000000000000000000007; - bytes memory _l2AdapterCArgs = abi.encode(_usdcProxy, _l2Messenger, _l1Adapter, _l2AdapterOwner); + bytes memory _l2AdapterCArgs = abi.encode(_usdcProxy, L2_MESSENGER, _l1Adapter, _l2AdapterOwner); bytes memory _l2AdapterInitCode = bytes.concat(type(L2OpUSDCBridgeAdapter).creationCode, _l2AdapterCArgs); (address _l2Adapter) = _deployCreate(_l2AdapterInitCode); emit L2AdapterDeployed(_l2Adapter); diff --git a/test/integration/L1OpUSDCBridgeAdapter.t.sol b/test/integration/L1OpUSDCBridgeAdapter.t.sol index 3aa45cfb..10fc1b23 100644 --- a/test/integration/L1OpUSDCBridgeAdapter.t.sol +++ b/test/integration/L1OpUSDCBridgeAdapter.t.sol @@ -91,10 +91,10 @@ contract Integration_Bridging is IntegrationBase { vm.prank(_signerAd); MAINNET_USDC.approve(address(l1Adapter), _amount); - - uint256 _nonce = vm.getNonce(_signerAd); - bytes memory _signature = _generateSignature(_signerAd, _amount, _nonce, _signerAd, _signerPk, address(l1Adapter)); uint256 _deadline = block.timestamp + 1 days; + uint256 _nonce = vm.getNonce(_signerAd); + bytes memory _signature = + _generateSignature(_signerAd, _amount, _deadline, _nonce, _signerAd, _signerPk, address(l1Adapter)); // Different address can execute the message vm.prank(_user); @@ -132,12 +132,13 @@ contract Integration_Bridging is IntegrationBase { vm.prank(_signerAd); MAINNET_USDC.approve(address(l1Adapter), _amount); + uint256 _deadline = block.timestamp + 1 days; uint256 _nonce = vm.getNonce(_signerAd); // Changing to `to` param to _user but we call it with _signerAd - bytes memory _signature = _generateSignature(_user, _amount, _nonce, _signerAd, _signerPk, address(l1Adapter)); - uint256 _deadline = block.timestamp + 1 days; + bytes memory _signature = + _generateSignature(_user, _amount, _deadline, _nonce, _signerAd, _signerPk, address(l1Adapter)); // Different address can execute the message vm.startPrank(_user); diff --git a/test/integration/L2OpUSDCBridgeAdapter.t.sol b/test/integration/L2OpUSDCBridgeAdapter.t.sol index 491ba2b6..8b95bd23 100644 --- a/test/integration/L2OpUSDCBridgeAdapter.t.sol +++ b/test/integration/L2OpUSDCBridgeAdapter.t.sol @@ -101,10 +101,10 @@ contract Integration_Bridging is IntegrationBase { vm.prank(_signerAd); bridgedUSDC.approve(address(l2Adapter), _amount); - - uint256 _nonce = vm.getNonce(_signerAd); - bytes memory _signature = _generateSignature(_signerAd, _amount, _nonce, _signerAd, _signerPk, address(l2Adapter)); uint256 _deadline = block.timestamp + 1 days; + uint256 _nonce = vm.getNonce(_signerAd); + bytes memory _signature = + _generateSignature(_signerAd, _amount, _deadline, _nonce, _signerAd, _signerPk, address(l2Adapter)); // Different address can execute the message vm.prank(_user); @@ -144,12 +144,12 @@ contract Integration_Bridging is IntegrationBase { vm.prank(_signerAd); bridgedUSDC.approve(address(l2Adapter), _amount); - + uint256 _deadline = block.timestamp + 1 days; uint256 _nonce = vm.getNonce(_signerAd); // Changing to `to` param to _user but we call it with _signerAd - bytes memory _signature = _generateSignature(_user, _amount, _nonce, _signerAd, _signerPk, address(l2Adapter)); - uint256 _deadline = block.timestamp + 1 days; + bytes memory _signature = + _generateSignature(_user, _amount, _deadline, _nonce, _signerAd, _signerPk, address(l2Adapter)); // Different address can execute the message vm.startPrank(_user); diff --git a/test/unit/L1OpUSDCBridgeAdapter.t.sol b/test/unit/L1OpUSDCBridgeAdapter.t.sol index a7aaf972..54797396 100644 --- a/test/unit/L1OpUSDCBridgeAdapter.t.sol +++ b/test/unit/L1OpUSDCBridgeAdapter.t.sol @@ -788,7 +788,8 @@ contract L1OpUSDCBridgeAdapter_Unit_SendMessageWithSignature is Base { vm.warp(_deadline - 1); uint256 _nonce = adapter.userNonce(_signerAd); (address _notSignerAd, uint256 _notSignerPk) = makeAddrAndKey('notSigner'); - bytes memory _signature = _generateSignature(_to, _amount, _nonce, _notSignerAd, _notSignerPk, address(adapter)); + bytes memory _signature = + _generateSignature(_to, _amount, _deadline, _nonce, _notSignerAd, _notSignerPk, address(adapter)); // Execute vm.prank(_user); @@ -803,7 +804,8 @@ contract L1OpUSDCBridgeAdapter_Unit_SendMessageWithSignature is Base { vm.assume(_deadline > 0); vm.warp(_deadline - 1); uint256 _nonce = adapter.userNonce(_signerAd); - bytes memory _signature = _generateSignature(_to, _amount, _nonce, _signerAd, _signerPk, address(adapter)); + bytes memory _signature = + _generateSignature(_to, _amount, _deadline, _nonce, _signerAd, _signerPk, address(adapter)); vm.mockCall( _usdc, @@ -834,7 +836,8 @@ contract L1OpUSDCBridgeAdapter_Unit_SendMessageWithSignature is Base { vm.assume(_deadline > 0); vm.warp(_deadline - 1); uint256 _nonce = adapter.userNonce(_signerAd); - bytes memory _signature = _generateSignature(_to, _amount, _nonce, _signerAd, _signerPk, address(adapter)); + bytes memory _signature = + _generateSignature(_to, _amount, _deadline, _nonce, _signerAd, _signerPk, address(adapter)); _mockAndExpect( _usdc, @@ -864,7 +867,8 @@ contract L1OpUSDCBridgeAdapter_Unit_SendMessageWithSignature is Base { vm.assume(_deadline > 0); vm.warp(_deadline - 1); uint256 _nonce = adapter.userNonce(_signerAd); - bytes memory _signature = _generateSignature(_to, _amount, _nonce, _signerAd, _signerPk, address(adapter)); + bytes memory _signature = + _generateSignature(_to, _amount, _deadline, _nonce, _signerAd, _signerPk, address(adapter)); vm.mockCall( _usdc, diff --git a/test/unit/L2OpUSDCBridgeAdapter.t.sol b/test/unit/L2OpUSDCBridgeAdapter.t.sol index 3b1773d3..c1cd4671 100644 --- a/test/unit/L2OpUSDCBridgeAdapter.t.sol +++ b/test/unit/L2OpUSDCBridgeAdapter.t.sol @@ -433,7 +433,8 @@ contract L2OpUSDCBridgeAdapter_Unit_SendMessageWithSignature is Base { vm.warp(_deadline - 1); uint256 _nonce = adapter.userNonce(_signerAd); (address _notSignerAd, uint256 _notSignerPk) = makeAddrAndKey('notSigner'); - bytes memory _signature = _generateSignature(_to, _amount, _nonce, _notSignerAd, _notSignerPk, address(adapter)); + bytes memory _signature = + _generateSignature(_to, _amount, _deadline, _nonce, _notSignerAd, _notSignerPk, address(adapter)); // Execute vm.prank(_user); @@ -448,7 +449,8 @@ contract L2OpUSDCBridgeAdapter_Unit_SendMessageWithSignature is Base { vm.assume(_deadline > 0); vm.warp(_deadline - 1); uint256 _nonce = adapter.userNonce(_signerAd); - bytes memory _signature = _generateSignature(_to, _amount, _nonce, _signerAd, _signerPk, address(adapter)); + bytes memory _signature = + _generateSignature(_to, _amount, _deadline, _nonce, _signerAd, _signerPk, address(adapter)); vm.mockCall( _usdc, abi.encodeWithSignature('transferFrom(address,address,uint256)', _user, address(adapter), _amount), @@ -479,7 +481,8 @@ contract L2OpUSDCBridgeAdapter_Unit_SendMessageWithSignature is Base { vm.assume(_deadline > 0); vm.warp(_deadline - 1); uint256 _nonce = adapter.userNonce(_signerAd); - bytes memory _signature = _generateSignature(_to, _amount, _nonce, _signerAd, _signerPk, address(adapter)); + bytes memory _signature = + _generateSignature(_to, _amount, _deadline, _nonce, _signerAd, _signerPk, address(adapter)); _mockAndExpect( _usdc, abi.encodeWithSignature('transferFrom(address,address,uint256)', _signerAd, address(adapter), _amount), @@ -509,7 +512,8 @@ contract L2OpUSDCBridgeAdapter_Unit_SendMessageWithSignature is Base { vm.assume(_deadline > 0); vm.warp(_deadline - 1); uint256 _nonce = adapter.userNonce(_signerAd); - bytes memory _signature = _generateSignature(_to, _amount, _nonce, _signerAd, _signerPk, address(adapter)); + bytes memory _signature = + _generateSignature(_to, _amount, _deadline, _nonce, _signerAd, _signerPk, address(adapter)); vm.mockCall( _usdc, abi.encodeWithSignature('transferFrom(address,address,uint256)', _user, address(adapter), _amount), diff --git a/test/utils/Helpers.sol b/test/utils/Helpers.sol index 1a73c5f8..72e0a8d1 100644 --- a/test/utils/Helpers.sol +++ b/test/utils/Helpers.sol @@ -24,13 +24,15 @@ contract Helpers is Test { function _generateSignature( address _to, uint256 _amount, + uint256 _deadline, uint256 _nonce, address _signerAd, uint256 _signerPk, address _adapter ) internal returns (bytes memory _signature) { vm.startPrank(_signerAd); - bytes32 _digest = keccak256(abi.encode(_adapter, block.chainid, _to, _amount, _nonce)).toEthSignedMessageHash(); + bytes32 _digest = + keccak256(abi.encode(_adapter, block.chainid, _to, _amount, _deadline, _nonce)).toEthSignedMessageHash(); (uint8 v, bytes32 r, bytes32 s) = vm.sign(_signerPk, _digest); _signature = abi.encodePacked(r, s, v); vm.stopPrank(); From 33923078cf9cc96ac76c9d98e968c726b40f8e94 Mon Sep 17 00:00:00 2001 From: excaliborr Date: Mon, 8 Jul 2024 10:54:38 -0400 Subject: [PATCH 2/2] fix: lint --- src/contracts/L2OpUSDCFactory.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contracts/L2OpUSDCFactory.sol b/src/contracts/L2OpUSDCFactory.sol index 81b240e5..ca01ed23 100644 --- a/src/contracts/L2OpUSDCFactory.sol +++ b/src/contracts/L2OpUSDCFactory.sol @@ -15,7 +15,7 @@ import {IUSDC} from 'interfaces/external/IUSDC.sol'; * L2 contracts have the same address on different L2s when triggered by different owners. */ contract L2OpUSDCFactory is IL2OpUSDCFactory { - address constant L2_MESSENGER = 0x4200000000000000000000000000000000000007; + address internal constant _L2_MESSENGER = 0x4200000000000000000000000000000000000007; /** * @notice Deploys the USDC implementation, proxy, and L2 adapter contracts all at once, and then initializes the USDC @@ -45,7 +45,7 @@ contract L2OpUSDCFactory is IL2OpUSDCFactory { emit USDCProxyDeployed(_usdcProxy); // Deploy L2 Adapter - bytes memory _l2AdapterCArgs = abi.encode(_usdcProxy, L2_MESSENGER, _l1Adapter, _l2AdapterOwner); + bytes memory _l2AdapterCArgs = abi.encode(_usdcProxy, _L2_MESSENGER, _l1Adapter, _l2AdapterOwner); bytes memory _l2AdapterInitCode = bytes.concat(type(L2OpUSDCBridgeAdapter).creationCode, _l2AdapterCArgs); (address _l2Adapter) = _deployCreate(_l2AdapterInitCode); emit L2AdapterDeployed(_l2Adapter);