Skip to content

Commit

Permalink
fix(protocol): fix bridge token transfer check (#15422)
Browse files Browse the repository at this point in the history
Co-authored-by: Paween Pitimanaaree <paween.pit@gmail.com>
  • Loading branch information
dantaik and paweenpit authored Dec 30, 2023
1 parent 045cc3c commit a31b91a
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 95 deletions.
4 changes: 3 additions & 1 deletion packages/protocol/contracts/L1/hooks/AssignmentHook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
pragma solidity 0.8.20;

import "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
import "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
import "../../common/EssentialContract.sol";
import "../../libs/LibAddress.sol";
import "../TaikoData.sol";
Expand All @@ -24,6 +25,7 @@ import "./IHook.sol";
/// A hook that handles prover assignment varification and fee processing.
contract AssignmentHook is EssentialContract, IHook {
using LibAddress for address;
using SafeERC20 for IERC20;

struct ProverAssignment {
address feeToken;
Expand Down Expand Up @@ -118,7 +120,7 @@ contract AssignmentHook is EssentialContract, IHook {
refund = msg.value - input.tip;
}
// Paying ERC20 tokens
IERC20(assignment.feeToken).transferFrom(msg.sender, blk.assignedProver, proverFee);
IERC20(assignment.feeToken).safeTransferFrom(msg.sender, blk.assignedProver, proverFee);
}

// block.coinbase can be address(0) in tests
Expand Down
8 changes: 5 additions & 3 deletions packages/protocol/contracts/L2/TaikoL2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
pragma solidity 0.8.20;

import "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
import "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";

import "../common/ICrossChainSync.sol";
import "../signal/ISignalService.sol";
Expand All @@ -33,6 +34,7 @@ import "./TaikoL2Signer.sol";
contract TaikoL2 is CrossChainOwned, TaikoL2Signer, ICrossChainSync {
using LibAddress for address;
using LibMath for uint256;
using SafeERC20 for IERC20;

struct Config {
uint32 gasTargetPerL1Block;
Expand Down Expand Up @@ -106,6 +108,7 @@ contract TaikoL2 is CrossChainOwned, TaikoL2Signer, ICrossChainSync {
uint32 parentGasUsed
)
external
nonReentrant
{
if (
l1BlockHash == 0 || l1SignalRoot == 0 || l1Height == 0
Expand Down Expand Up @@ -154,13 +157,12 @@ contract TaikoL2 is CrossChainOwned, TaikoL2Signer, ICrossChainSync {
}

/// @notice Withdraw token or Ether from this address
function withdraw(address token, address to) external onlyOwner {
function withdraw(address token, address to) external onlyOwner nonReentrant whenNotPaused {
if (to == address(0)) revert L2_INVALID_PARAM();
if (token == address(0)) {
to.sendEther(address(this).balance);
} else {
IERC20 t = IERC20(token);
t.transfer(to, t.balanceOf(address(this)));
IERC20(token).safeTransfer(to, IERC20(token).balanceOf(address(this)));
}
}

Expand Down
43 changes: 16 additions & 27 deletions packages/protocol/contracts/tokenvault/BridgedERC1155.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ contract BridgedERC1155 is

uint256[46] private __gap;

// Event triggered upon token transfer.
event Transfer(address indexed from, address indexed to, uint256 tokenId, uint256 amount);

error BTOKEN_CANNOT_RECEIVE();
error BTOKEN_INVALID_PARAMS();

Expand Down Expand Up @@ -104,30 +101,6 @@ contract BridgedERC1155 is
_burn(account, tokenId, amount);
}

/// @dev Safely transfers tokens from one address to another.
/// @param from Address from which tokens are transferred.
/// @param to Address to which tokens are transferred.
/// @param tokenId ID of the token to transfer.
/// @param amount Amount of tokens to transfer.
/// @param data Additional data.
function safeTransferFrom(
address from,
address to,
uint256 tokenId,
uint256 amount,
bytes memory data
)
public
override(ERC1155Upgradeable, IERC1155Upgradeable)
nonReentrant
whenNotPaused
{
if (to == address(this)) {
revert BTOKEN_CANNOT_RECEIVE();
}
return ERC1155Upgradeable.safeTransferFrom(from, to, tokenId, amount, data);
}

/// @notice Gets the name of the bridged token.
/// @return The name.
function name() public view returns (string memory) {
Expand All @@ -139,4 +112,20 @@ contract BridgedERC1155 is
function symbol() public view returns (string memory) {
return LibBridgedToken.buildSymbol(symbol_);
}

function _beforeTokenTransfer(
address, /*operator*/
address, /*from*/
address to,
uint256[] memory, /*ids*/
uint256[] memory, /*amounts*/
bytes memory /*data*/
)
internal
virtual
override
{
if (to == address(this)) revert BTOKEN_CANNOT_RECEIVE();
if (paused()) revert INVALID_PAUSE_STATUS();
}
}
55 changes: 13 additions & 42 deletions packages/protocol/contracts/tokenvault/BridgedERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -72,48 +72,6 @@ contract BridgedERC20 is BridgedERC20Base, IERC20MetadataUpgradeable, ERC20Upgra
srcDecimals = _decimals;
}

/// @notice Transfers tokens from the caller to another account.
/// @dev Any address can call this. Caller must have at least 'amount' to
/// call this.
/// @param to The account to transfer tokens to.
/// @param amount The amount of tokens to transfer.
function transfer(
address to,
uint256 amount
)
public
override(ERC20Upgradeable, IERC20Upgradeable)
nonReentrant
whenNotPaused
returns (bool)
{
if (to == address(this)) revert BTOKEN_CANNOT_RECEIVE();
return ERC20Upgradeable.transfer(to, amount);
}

/// @notice Transfers tokens from one account to another account.
/// @dev Any address can call this. Caller must have allowance of at least
/// 'amount' for 'from's tokens.
/// @param from The account to transfer tokens from.
/// @param to The account to transfer tokens to.
/// @param amount The amount of tokens to transfer.
function transferFrom(
address from,
address to,
uint256 amount
)
public
override(ERC20Upgradeable, IERC20Upgradeable)
nonReentrant
whenNotPaused
returns (bool)
{
if (to == address(this)) {
revert BTOKEN_CANNOT_RECEIVE();
}
return ERC20Upgradeable.transferFrom(from, to, amount);
}

/// @notice Gets the name of the token.
/// @return The name.
function name()
Expand Down Expand Up @@ -160,4 +118,17 @@ contract BridgedERC20 is BridgedERC20Base, IERC20MetadataUpgradeable, ERC20Upgra
function _burnToken(address from, uint256 amount) internal override {
_burn(from, amount);
}

function _beforeTokenTransfer(
address, /*from*/
address to,
uint256 /*amount*/
)
internal
virtual
override
{
if (to == address(this)) revert BTOKEN_CANNOT_RECEIVE();
if (paused()) revert INVALID_PAUSE_STATUS();
}
}
34 changes: 14 additions & 20 deletions packages/protocol/contracts/tokenvault/BridgedERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -93,26 +93,6 @@ contract BridgedERC721 is EssentialContract, ERC721Upgradeable {
_burn(tokenId);
}

/// @dev Safely transfers tokens from one address to another.
/// @param from Address from which the token is transferred.
/// @param to Address to which the token is transferred.
/// @param tokenId ID of the token to transfer.
function transferFrom(
address from,
address to,
uint256 tokenId
)
public
override(ERC721Upgradeable)
nonReentrant
whenNotPaused
{
if (to == address(this)) {
revert BTOKEN_CANNOT_RECEIVE();
}
return ERC721Upgradeable.transferFrom(from, to, tokenId);
}

/// @notice Gets the name of the token.
/// @return The name.
function name() public view override(ERC721Upgradeable) returns (string memory) {
Expand All @@ -135,4 +115,18 @@ contract BridgedERC721 is EssentialContract, ERC721Upgradeable {
function tokenURI(uint256) public pure virtual override returns (string memory) {
return "";
}

function _beforeTokenTransfer(
address, /*from*/
address to,
uint256, /*firstTokenId*/
uint256 /*batchSize*/
)
internal
virtual
override
{
if (to == address(this)) revert BTOKEN_CANNOT_RECEIVE();
if (paused()) revert INVALID_PAUSE_STATUS();
}
}
2 changes: 1 addition & 1 deletion packages/protocol/contracts/tokenvault/ERC20Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ contract ERC20Vault is BaseVault {
// simply using `amount` -- some contract may deduct a fee from the
// transferred amount.
uint256 _balance = t.balanceOf(address(this));
t.transferFrom({ from: msg.sender, to: address(this), amount: amount });
t.safeTransferFrom({ from: msg.sender, to: address(this), value: amount });
_balanceChange = t.balanceOf(address(this)) - _balance;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/tokenvault/ERC721Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable {
if (ctoken.chainId == block.chainid) {
token = ctoken.addr;
for (uint256 i; i < tokenIds.length; ++i) {
ERC721Upgradeable(token).transferFrom({
ERC721Upgradeable(token).safeTransferFrom({
from: address(this),
to: _to,
tokenId: tokenIds[i]
Expand Down

3 comments on commit a31b91a

@vercel
Copy link

@vercel vercel bot commented on a31b91a Dec 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

bridge-ui-v2-internal – ./packages/bridge-ui-v2

bridge-ui-v2-internal.vercel.app
bridge-ui-v2-internal-taikoxyz.vercel.app
bridge-ui-v2-internal-git-alpha-6-taikoxyz.vercel.app

@vercel
Copy link

@vercel vercel bot commented on a31b91a Dec 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

bridge-ui-v2-a6 – ./packages/bridge-ui-v2

bridge-ui-v2-a6.vercel.app
bridge.katla.taiko.xyz
bridge-ui-v2-a6-git-alpha-6-taikoxyz.vercel.app
bridge-ui-v2-a6-taikoxyz.vercel.app

@cyberpoor
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check

Please sign in to comment.