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.

Sign up for GitHub

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(protocol): fix bridge token transfer check #15422

Merged
merged 7 commits into from
Dec 30, 2023
Merged

fix(protocol): fix bridge token transfer check #15422

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5525c03
fix Bridge token transfer check
dantaik Dec 27, 2023
cb9854b
Update ERC721Vault.sol
dantaik Dec 27, 2023
1dbecb0
fix(protocol): fix revert when transfer ERC20 due to inconsistent int…
paweenpit Dec 27, 2023
e966d37
override beforeTokenTransfer
dantaik Dec 30, 2023
c1e7a40
Update TaikoL2.sol
dantaik Dec 30, 2023
20d04c2
Merge branch 'alpha-6' into fix_bridged_token_check
dantaik Dec 30, 2023
66108b3
fix lint
dantaik Dec 30, 2023
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
4 changes: 3 additions & 1 deletion packages/protocol/contracts/L1/hooks/AssignmentHook.sol
View file
Open in desktop
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
3 changes: 1 addition & 2 deletions packages/protocol/contracts/L2/TaikoL2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ contract TaikoL2 is CrossChainOwned, TaikoL2Signer, ICrossChainSync {
if (token == address(0)) {
to.sendEther(address(this).balance);
} else {
IERC20 t = IERC20(token);
t.transfer(to, t.balanceOf(address(this)));
IERC20(token).transfer(to, IERC20(token).balanceOf(address(this)));
dantaik marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
45 changes: 18 additions & 27 deletions packages/protocol/contracts/tokenvault/BridgedERC1155.sol
View file
Open in desktop
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,22 @@ 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();

ERC1155Upgradeable._beforeTokenTransfer(operator, from, to, ids, amounts, data);
}
}
49 changes: 7 additions & 42 deletions packages/protocol/contracts/tokenvault/BridgedERC20.sol
View file
Open in desktop
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,11 @@ contract BridgedERC20 is BridgedERC20Base, IERC20MetadataUpgradeable, ERC20Upgra
function _burnToken(address from, uint256 amount) internal override {
_burn(from, amount);
}

function _transfer(address from, address to, uint256 amount) internal virtual override {
dantaik marked this conversation as resolved.
Show resolved Hide resolved
if (to == address(this)) revert BTOKEN_CANNOT_RECEIVE();
if (paused()) revert INVALID_PAUSE_STATUS();

ERC20Upgradeable._transfer(from, to, amount);
}
}
27 changes: 7 additions & 20 deletions packages/protocol/contracts/tokenvault/BridgedERC721.sol
View file
Open in desktop
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,11 @@ contract BridgedERC721 is EssentialContract, ERC721Upgradeable {
function tokenURI(uint256) public pure virtual override returns (string memory) {
return "";
}

function _transfer(address from, address to, uint256 tokenId) internal virtual override {
dantaik marked this conversation as resolved.
Show resolved Hide resolved
if (to == address(this)) revert BTOKEN_CANNOT_RECEIVE();
if (paused()) revert INVALID_PAUSE_STATUS();

ERC721Upgradeable._transfer(from, to, tokenId);
}
}
2 changes: 1 addition & 1 deletion packages/protocol/contracts/tokenvault/ERC20Vault.sol
View file
Open in desktop
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
View file
Open in desktop
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
Loading