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

feat(protocol): Change require to custom err in bridge contracts #13220

Merged
merged 7 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
16 changes: 9 additions & 7 deletions packages/protocol/contracts/bridge/Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pragma solidity ^0.8.18;
import {AddressResolver} from "../common/AddressResolver.sol";
import {EssentialContract} from "../common/EssentialContract.sol";
import {IBridge} from "./IBridge.sol";
import {BridgeCustomErrors} from "./BridgeCustomErrors.sol";
import {LibBridgeData} from "./libs/LibBridgeData.sol";
import {LibBridgeProcess} from "./libs/LibBridgeProcess.sol";
import {LibBridgeRelease} from "./libs/LibBridgeRelease.sol";
Expand All @@ -21,7 +22,7 @@ import {LibBridgeStatus} from "./libs/LibBridgeStatus.sol";
* which calls the library implementations. See _IBridge_ for more details.
* @dev The code hash for the same address on L1 and L2 may be different.
*/
contract Bridge is EssentialContract, IBridge {
contract Bridge is EssentialContract, IBridge, BridgeCustomErrors {
using LibBridgeData for Message;

/*********************
Expand Down Expand Up @@ -49,12 +50,13 @@ contract Bridge is EssentialContract, IBridge {

/// Allow Bridge to receive ETH from the TokenVault or EtherVault.
receive() external payable {
require(
msg.sender == resolve("token_vault", true) ||
msg.sender == resolve("ether_vault", true) ||
msg.sender == owner(),
"B:receive"
);
if (
msg.sender != resolve("token_vault", true) &&
msg.sender != resolve("ether_vault", true) &&
msg.sender != owner()
) {
revert B_CANNOT_RECEIVE();
}
}

/// @dev Initializer to be called after being deployed behind a proxy.
Expand Down
32 changes: 32 additions & 0 deletions packages/protocol/contracts/bridge/BridgeCustomErrors.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// SPDX-License-Identifier: MIT
// _____ _ _ _ _
// |_ _|_ _(_) |_____ | | __ _| |__ ___
// | |/ _` | | / / _ \ | |__/ _` | '_ (_-<
// |_|\__,_|_|_\_\___/ |____\__,_|_.__/__/

pragma solidity ^0.8.18;

abstract contract BridgeCustomErrors {
error B_CANNOT_RECEIVE();
adaki2004 marked this conversation as resolved.
Show resolved Hide resolved
error B_DENIED();
error B_ERC20_CANNOT_RECEIVE();
error B_ETHER_RELEASED_ALREADY();
error B_EV_DO_NOT_BURN();
error B_EV_NOT_AUTHORIZED();
error B_EV_PARAM();
error B_FAILED_TRANSFER();
error B_FORBIDDEN();
error B_GAS_LIMIT();
error B_INCORRECT_VALUE();
error B_INIT_PARAM_ERROR();
error B_MSG_HASH_NULL();
error B_MSG_NON_RETRIABLE();
error B_MSG_NOT_FAILED();
error B_NULL_APP_ADDR();
error B_OWNER_IS_NULL();
error B_SIGNAL_NOT_RECEIVED();
error B_STATUS_MISMTACH();
error B_WRONG_CHAIN_ID();
error B_WRONG_TO_ADDRESS();
error B_ZERO_SIGNAL();
}
29 changes: 18 additions & 11 deletions packages/protocol/contracts/bridge/BridgedERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ import {

import {EssentialContract} from "../common/EssentialContract.sol";
import {ERC20Upgradeable} from "../thirdparty/ERC20Upgradeable.sol";
import {BridgeCustomErrors} from "./BridgeCustomErrors.sol";

contract BridgedERC20 is
EssentialContract,
IERC20Upgradeable,
IERC20MetadataUpgradeable,
ERC20Upgradeable
ERC20Upgradeable,
BridgeCustomErrors
{
address public srcToken;
uint256 public srcChainId;
Expand All @@ -41,14 +43,15 @@ contract BridgedERC20 is
string memory _symbol,
string memory _name
) external initializer {
require(
_srcToken != address(0) &&
_srcChainId != 0 &&
_srcChainId != block.chainid &&
bytes(_symbol).length > 0 &&
bytes(_name).length > 0,
"BE:params"
);
if (
_srcToken == address(0) ||
_srcChainId == 0 ||
_srcChainId == block.chainid ||
bytes(_symbol).length == 0 ||
bytes(_name).length == 0
) {
revert B_INIT_PARAM_ERROR();
}
EssentialContract._init(_addressManager);
ERC20Upgradeable.__ERC20_init({
name_: _name,
Expand Down Expand Up @@ -83,7 +86,9 @@ contract BridgedERC20 is
address to,
uint256 amount
) public override(ERC20Upgradeable, IERC20Upgradeable) returns (bool) {
require(to != address(this), "BE:to");
if (to == address(this)) {
revert B_ERC20_CANNOT_RECEIVE();
}
return ERC20Upgradeable.transfer(to, amount);
}

Expand All @@ -95,7 +100,9 @@ contract BridgedERC20 is
address to,
uint256 amount
) public override(ERC20Upgradeable, IERC20Upgradeable) returns (bool) {
require(to != address(this), "BE:to");
if (to == address(this)) {
revert B_ERC20_CANNOT_RECEIVE();
}
return ERC20Upgradeable.transferFrom(from, to, amount);
}

Expand Down
25 changes: 14 additions & 11 deletions packages/protocol/contracts/bridge/EtherVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ import {

import {EssentialContract} from "../common/EssentialContract.sol";
import {LibAddress} from "../libs/LibAddress.sol";
import {BridgeCustomErrors} from "./BridgeCustomErrors.sol";

/**
* EtherVault is a special vault contract that:
* - Is initialized with 2^128 Ether.
* - Allows the contract owner to authorize addresses.
* - Allows authorized addresses to send/release Ether.
*/
contract EtherVault is EssentialContract {
contract EtherVault is EssentialContract, BridgeCustomErrors {
using LibAddress for address;

/*********************
Expand All @@ -46,7 +47,9 @@ contract EtherVault is EssentialContract {
*********************/

modifier onlyAuthorized() {
require(isAuthorized(msg.sender), "EV:denied");
if (!isAuthorized(msg.sender)) {
revert B_EV_NOT_AUTHORIZED();
}
_;
}

Expand All @@ -56,10 +59,9 @@ contract EtherVault is EssentialContract {

receive() external payable {
// EthVault's balance must == 0 OR the sender isAuthorized.
require(
address(this).balance == 0 || isAuthorized(msg.sender),
"EV:denied"
);
if (address(this).balance != 0 && !isAuthorized(msg.sender)) {
revert B_EV_NOT_AUTHORIZED();
}
}

function init(address addressManager) external initializer {
Expand Down Expand Up @@ -90,7 +92,9 @@ contract EtherVault is EssentialContract {
address recipient,
uint256 amount
) public onlyAuthorized nonReentrant {
require(recipient != address(0), "EV:recipient");
if (recipient == address(0)) {
revert B_EV_DO_NOT_BURN();
}
recipient.sendEther(amount);
emit EtherReleased(recipient, amount);
}
Expand All @@ -101,10 +105,9 @@ contract EtherVault is EssentialContract {
* @param authorized Authorized status to set.
*/
function authorize(address addr, bool authorized) public onlyOwner {
require(
addr != address(0) && _authorizedAddrs[addr] != authorized,
"EV:param"
);
if (addr == address(0) || _authorizedAddrs[addr] == authorized) {
revert B_EV_PARAM();
}
_authorizedAddrs[addr] = authorized;
emit Authorized(addr, authorized);
}
Expand Down
6 changes: 5 additions & 1 deletion packages/protocol/contracts/bridge/libs/LibBridgeInvoke.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ library LibBridgeInvoke {
using LibAddress for address;
using LibBridgeData for IBridge.Message;

error B_GAS_LIMIT();

/*********************
* Internal Functions*
*********************/
Expand All @@ -22,7 +24,9 @@ library LibBridgeInvoke {
bytes32 msgHash,
uint256 gasLimit
) internal returns (bool success) {
require(gasLimit > 0, "B:gasLimit");
if (gasLimit == 0) {
revert B_GAS_LIMIT();
}

state.ctx = IBridge.Context({
msgHash: msgHash,
Expand Down
35 changes: 22 additions & 13 deletions packages/protocol/contracts/bridge/libs/LibBridgeProcess.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ library LibBridgeProcess {
using LibBridgeData for IBridge.Message;
using LibBridgeData for LibBridgeData.State;

error B_FORBIDDEN();
error B_WRONG_CHAIN_ID();
error B_STATUS_MISMTACH();
error B_SIGNAL_NOT_RECEIVED();

/**
* Process the bridge message on the destination chain. It can be called by
* any address, including `message.owner`. It starts by hashing the message,
Expand All @@ -45,37 +50,41 @@ library LibBridgeProcess {
bytes calldata proof
) external {
// If the gas limit is set to zero, only the owner can process the message.
if (message.gasLimit == 0) {
require(msg.sender == message.owner, "B:forbidden");
if (message.gasLimit == 0 && msg.sender != message.owner) {
revert B_FORBIDDEN();
}

require(message.destChainId == block.chainid, "B:destChainId");
if (message.destChainId != block.chainid) {
revert B_WRONG_CHAIN_ID();
}

// The message status must be "NEW"; "RETRIABLE" is handled in
// LibBridgeRetry.sol.
bytes32 msgHash = message.hashMessage();
require(
LibBridgeStatus.getMessageStatus(msgHash) ==
LibBridgeStatus.MessageStatus.NEW,
"B:status"
);
if (
LibBridgeStatus.getMessageStatus(msgHash) !=
LibBridgeStatus.MessageStatus.NEW
) {
revert B_STATUS_MISMTACH();
}
// Message must have been "received" on the destChain (current chain)
address srcBridge = resolver.resolve(
message.srcChainId,
"bridge",
false
);

require(
ISignalService(resolver.resolve("signal_service", false))
if (
!ISignalService(resolver.resolve("signal_service", false))
.isSignalReceived({
srcChainId: message.srcChainId,
app: srcBridge,
signal: msgHash,
proof: proof
}),
"B:notReceived"
);
})
) {
revert B_SIGNAL_NOT_RECEIVED();
}

// We retrieve the necessary ether from EtherVault if receiving on
// Taiko, otherwise it is already available in this Bridge.
Expand Down
36 changes: 27 additions & 9 deletions packages/protocol/contracts/bridge/libs/LibBridgeRelease.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ import {AddressResolver} from "../../common/AddressResolver.sol";
library LibBridgeRelease {
using LibBridgeData for IBridge.Message;

error B_OWNER_IS_NULL();
error B_WRONG_CHAIN_ID();
error B_ETHER_RELEASED_ALREADY();
error B_MSG_NOT_FAILED();
error B_FAILED_TRANSFER();

event EtherReleased(bytes32 indexed msgHash, address to, uint256 amount);

/**
Expand All @@ -28,20 +34,30 @@ library LibBridgeRelease {
IBridge.Message calldata message,
bytes calldata proof
) internal {
require(message.owner != address(0), "B:owner");
require(message.srcChainId == block.chainid, "B:srcChainId");
if (message.owner == address(0)) {
revert B_OWNER_IS_NULL();
}

if (message.srcChainId != block.chainid) {
revert B_WRONG_CHAIN_ID();
}

bytes32 msgHash = message.hashMessage();
require(state.etherReleased[msgHash] == false, "B:etherReleased");
require(
LibBridgeStatus.isMessageFailed(

if (state.etherReleased[msgHash] == true) {
revert B_ETHER_RELEASED_ALREADY();
}

if (
!LibBridgeStatus.isMessageFailed(
resolver,
msgHash,
message.destChainId,
proof
),
"B:notFailed"
);
)
) {
revert B_MSG_NOT_FAILED();
}

state.etherReleased[msgHash] = true;

Expand All @@ -58,7 +74,9 @@ library LibBridgeRelease {
} else {
// if on Ethereum
(bool success, ) = message.owner.call{value: releaseAmount}("");
require(success, "B:transfer");
if (!success) {
revert B_FAILED_TRANSFER();
}
}
}
emit EtherReleased(msgHash, message.owner, releaseAmount);
Expand Down
16 changes: 10 additions & 6 deletions packages/protocol/contracts/bridge/libs/LibBridgeRetry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ library LibBridgeRetry {
using LibBridgeData for IBridge.Message;
using LibBridgeData for LibBridgeData.State;

error B_DENIED();
error B_MSG_NON_RETRIABLE();

/**
* Retries to invoke the messageCall, the owner has already been sent Ether.
* - This function can be called by any address, including `message.owner`.
Expand All @@ -46,15 +49,16 @@ library LibBridgeRetry {
// If the gasLimit is not set to 0 or isLastAttempt is true, the
// address calling this function must be message.owner.
if (message.gasLimit == 0 || isLastAttempt) {
require(msg.sender == message.owner, "B:denied");
if (msg.sender != message.owner) revert B_DENIED();
}

bytes32 msgHash = message.hashMessage();
require(
LibBridgeStatus.getMessageStatus(msgHash) ==
LibBridgeStatus.MessageStatus.RETRIABLE,
"B:notFound"
);
if (
LibBridgeStatus.getMessageStatus(msgHash) !=
LibBridgeStatus.MessageStatus.RETRIABLE
) {
revert B_MSG_NON_RETRIABLE();
}

address ethVault = resolver.resolve("ether_vault", true);
if (ethVault != address(0)) {
Expand Down
Loading