Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: deadline in signature check #96

Merged
merged 2 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion script/sepolia/DeployOptimism.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
9 changes: 5 additions & 4 deletions src/contracts/L1OpUSDCBridgeAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down
13 changes: 7 additions & 6 deletions src/contracts/L2OpUSDCBridgeAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;

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

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

Expand All @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/contracts/L2OpUSDCFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 internal 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
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 6 additions & 5 deletions test/integration/L1OpUSDCBridgeAdapter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions test/integration/L2OpUSDCBridgeAdapter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 8 additions & 4 deletions test/unit/L1OpUSDCBridgeAdapter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 8 additions & 4 deletions test/unit/L2OpUSDCBridgeAdapter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
4 changes: 3 additions & 1 deletion test/utils/Helpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down