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: edgecase where user is blacklisted with an inflight message #105

Merged
merged 17 commits into from
Jul 23, 2024
38 changes: 36 additions & 2 deletions src/contracts/L1OpUSDCBridgeAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter {
if (messengerStatus != Status.Deprecated) revert IOpUSDCBridgeAdapter_BurnAmountNotSet();

// Burn the USDC tokens
// NOTE: If in flight transactions fail due to user being blacklisted after migration
// The funds will just be trapped in this contract as its deprecated
// If the user is after unblacklisted, they will be able to withdraw their usdc
uint256 _burnAmount = burnAmount;
if (_burnAmount != 0) {
// NOTE: This is a very edge case and will only happen if the chain operator adds a second minter on L2
Expand All @@ -134,7 +137,7 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter {
}

/*///////////////////////////////////////////////////////////////
MESSAGING CONTROL
ADMIN CONTROL
///////////////////////////////////////////////////////////////*/

/**
Expand Down Expand Up @@ -268,7 +271,38 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter {
*/
function receiveMessage(address _user, uint256 _amount) external override onlyLinkedAdapter {
// Transfer the tokens to the user
try this.attemptTransfer(_user, _amount) {
emit MessageReceived(_user, _amount, MESSENGER);
} catch {
userBlacklistedFunds[_user] += _amount;
emit MessageFailed(_user, _amount);
}
}

/**
* @notice Withdraws the blacklisted funds from the contract incase they get unblacklisted
* @param _user The user to withdraw the funds for
*/
function withdrawBlacklistedFunds(address _user) external override {
uint256 _amount = userBlacklistedFunds[_user];
userBlacklistedFunds[_user] = 0;

// The check for if the user is blacklisted happens in USDC's contract
IUSDC(USDC).safeTransfer(_user, _amount);
emit MessageReceived(_user, _amount, MESSENGER);

emit BlacklistedFundsWithdrawn(_user, _amount);
}

/**
* @notice Attempts to transfer the tokens to the user
* @param _to The target address on the destination chain
* @param _amount The amount of tokens to send
* @dev This function should only be called when receiving a message
* And is a workaround for the fact that try/catch
* Only works on external calls and SafeERC20 is an internal library
*/
function attemptTransfer(address _to, uint256 _amount) external {
hexshire marked this conversation as resolved.
Show resolved Hide resolved
if (msg.sender != address(this)) revert IOpUSDCBridgeAdapter_InvalidSender();
IUSDC(USDC).safeTransfer(_to, _amount);
}
}
27 changes: 26 additions & 1 deletion src/contracts/L2OpUSDCBridgeAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter {
isMessagingDisabled = true;
roleCaller = _roleCaller;

// We need to do totalSupply + blacklistedFunds
// Because on `receiveMessage` mint would fail causing the totalSupply to not increase
// But the native token is still locked on L1
uint256 _burnAmount = IUSDC(USDC).totalSupply();

ICrossDomainMessenger(MESSENGER).sendMessage(
Expand Down Expand Up @@ -216,8 +219,30 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter {
*/
function receiveMessage(address _user, uint256 _amount) external override onlyLinkedAdapter {
// Mint the tokens to the user
try IUSDC(USDC).mint(_user, _amount) {
emit MessageReceived(_user, _amount, MESSENGER);
} catch {
userBlacklistedFunds[_user] += _amount;
emit MessageFailed(_user, _amount);
}
}

/**
* @notice Mints the blacklisted funds from the contract incase they get unblacklisted
* @param _user The user to withdraw the funds for
*/
function withdrawBlacklistedFunds(address _user) external override {
uint256 _amount = userBlacklistedFunds[_user];
userBlacklistedFunds[_user] = 0;

// NOTE: This will fail after migration as the adapter will no longer be a minter
// All funds need to be recovered from the contract before migration if applicable
// TODO: If migration has happend instead send a message back to L1 to recover the funds

// The check for if the user is blacklisted happens in USDC's contract
IUSDC(USDC).mint(_user, _amount);
emit MessageReceived(_user, _amount, MESSENGER);

emit BlacklistedFundsWithdrawn(_user, _amount);
}

/*///////////////////////////////////////////////////////////////
Expand Down
9 changes: 9 additions & 0 deletions src/contracts/universal/OpUSDCBridgeAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ abstract contract OpUSDCBridgeAdapter is IOpUSDCBridgeAdapter, Ownable {
/// @inheritdoc IOpUSDCBridgeAdapter
mapping(address _user => mapping(uint256 _nonce => bool _used)) public userNonces;

/// @inheritdoc IOpUSDCBridgeAdapter
mapping(address _user => uint256 _blacklistedAmount) public userBlacklistedFunds;

/**
* @notice Construct the OpUSDCBridgeAdapter contract
* @param _usdc The address of the USDC Contract to be used by the adapter
Expand Down Expand Up @@ -76,6 +79,12 @@ abstract contract OpUSDCBridgeAdapter is IOpUSDCBridgeAdapter, Ownable {
*/
function receiveMessage(address _user, uint256 _amount) external virtual;

/**
* @notice Withdraws the blacklisted funds from the contract if they get unblacklisted
* @param _user The user to withdraw the funds for
*/
function withdrawBlacklistedFunds(address _user) external virtual;

/**
* @notice Cancels a signature by setting the nonce as used
* @param _nonce The nonce of the signature to cancel
Expand Down
27 changes: 27 additions & 0 deletions src/interfaces/IOpUSDCBridgeAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ interface IOpUSDCBridgeAdapter {
*/
event MigratingToNative(address _messenger, address _caller);

/**
* @notice Emitted when a message fails
* @param _user The user that the message failed for
* @param _amount The amount of tokens that were added to the blacklisted funds
*/
event MessageFailed(address _user, uint256 _amount);

/**
* @notice Emitted when the blacklisted funds are withdrawn
* @param _user The user that the funds were withdrawn for
* @param _amountWithdrawn The amount of tokens that were withdrawn
*/
event BlacklistedFundsWithdrawn(address _user, uint256 _amountWithdrawn);

/*///////////////////////////////////////////////////////////////
ERRORS
///////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -152,6 +166,12 @@ interface IOpUSDCBridgeAdapter {
*/
function receiveMessage(address _user, uint256 _amount) external;

/**
* @notice Withdraws the blacklisted funds from the contract if they get unblacklisted
* @param _user The user to withdraw the funds for
*/
function withdrawBlacklistedFunds(address _user) external;

/**
* @notice Cancels a signature by setting the nonce as used
* @param _nonce The nonce of the signature to cancel
Expand Down Expand Up @@ -190,4 +210,11 @@ interface IOpUSDCBridgeAdapter {
* @return _used If the nonce has been used
*/
function userNonces(address _user, uint256 _nonce) external view returns (bool _used);

/**
* @notice Returns the amount of funds locked that got blacklisted for a specific user
* @param _user The user to check for
* @return _amount The amount of funds locked from blacklisted messages
*/
function userBlacklistedFunds(address _user) external view returns (uint256 _amount);
}
12 changes: 12 additions & 0 deletions src/interfaces/external/IUSDC.sol
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,18 @@ interface IUSDC is IERC20 {
*/
function updateMasterMinter(address _newMasterMinter) external;

/**
* @notice Adds account to blacklist
* @param _account The address to blacklist
*/
function blacklist(address _account) external;

/**
* @notice Removes account from blacklist.
* @param _account The address to remove from the blacklist.
*/
function unBlacklist(address _account) external;

/**
* @notice Function to upgrade the usdc proxy to a new implementation
* @param _newImplementation Address of the new implementation
Expand Down
38 changes: 38 additions & 0 deletions test/integration/L1OpUSDCBridgeAdapter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -296,4 +296,42 @@ contract Integration_Integration_PermissionedFlows is IntegrationBase {

assertEq(l2Adapter.isMessagingDisabled(), false);
}

/**
* @notice Test that the user can withdraw the blacklisted funds if they get unblacklisted
*/
function test_userCanWithdrawBlacklistedFunds() public {
vm.selectFork(mainnet);
_mintSupplyOnL2(optimism, OP_ALIASED_L1_MESSENGER, _amount);

vm.selectFork(optimism);
vm.startPrank(_user);
bridgedUSDC.approve(address(l2Adapter), _amount);
l2Adapter.sendMessage(_user, _amount, _MIN_GAS_LIMIT);
vm.stopPrank();
assertEq(bridgedUSDC.balanceOf(_user), 0);

vm.selectFork(mainnet);

vm.prank(MAINNET_USDC.blacklister());
MAINNET_USDC.blacklist(_user);
_relayL2ToL1Message(
address(l2Adapter),
address(l1Adapter),
_ZERO_VALUE,
_MIN_GAS_LIMIT,
abi.encodeWithSignature('receiveMessage(address,uint256)', _user, _amount)
);

assertEq(MAINNET_USDC.balanceOf(_user), 0);

vm.prank(MAINNET_USDC.blacklister());
MAINNET_USDC.unBlacklist(_user);

vm.prank(_user);
l1Adapter.withdrawBlacklistedFunds(_user);

assertEq(MAINNET_USDC.balanceOf(_user), _amount);
assertEq(l1Adapter.userBlacklistedFunds(_user), 0);
}
}
35 changes: 35 additions & 0 deletions test/integration/L2OpUSDCBridgeAdapter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,41 @@ contract Integration_Bridging is IntegrationBase {
assertEq(MAINNET_USDC.balanceOf(address(l1Adapter)), 0);
}

/**
* @notice Test bridging with a user who is blacklisted on L1
*/
function test_bridgingWithBlacklistedUser() public {
vm.selectFork(mainnet);
vm.prank(MAINNET_USDC.blacklister());
MAINNET_USDC.blacklist(_user);

vm.selectFork(optimism);
// Mint to increment total supply of bridgedUSDC and balance of _user
vm.prank(address(l2Adapter));
bridgedUSDC.mint(_user, _amount);

vm.startPrank(_user);
bridgedUSDC.approve(address(l2Adapter), _amount);
l2Adapter.sendMessage(_user, _amount, _MIN_GAS_LIMIT);
vm.stopPrank();

assertEq(bridgedUSDC.balanceOf(_user), 0);
assertEq(bridgedUSDC.balanceOf(address(l2Adapter)), 0);

vm.selectFork(mainnet);
_relayL2ToL1Message(
address(l2Adapter),
address(l1Adapter),
_ZERO_VALUE,
_MIN_GAS_LIMIT,
abi.encodeWithSignature('receiveMessage(address,uint256)', _user, _amount)
);

assertEq(MAINNET_USDC.balanceOf(_user), 0);
assertEq(MAINNET_USDC.balanceOf(address(l1Adapter)), _amount);
assertEq(l1Adapter.userBlacklistedFunds(_user), _amount);
}

/**
* @notice Test bridging with signature
*/
Expand Down
Loading