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

Make Gateway entry points nonreantrant #1358

Merged
merged 11 commits into from
Jan 10, 2025
6 changes: 5 additions & 1 deletion contracts/foundry.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[profile.default]
solc_version = "0.8.25"
solc_version = "0.8.28"
optimizer = true
optimizer_runs = 20000
via_ir = false
Expand All @@ -13,8 +13,12 @@ fs_permissions = [
ignored_error_codes = [
# DeployLocal.sol is never deployed
5574,
# tstore
2394,
]

evm_version = 'Cancun'

[profile.production]
via_ir = true

Expand Down
2 changes: 1 addition & 1 deletion contracts/scripts/Deploy.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {Script} from "forge-std/Script.sol";
import {GatewayProxy} from "../src/GatewayProxy.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/scripts/DeployBeefyClient.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {Script} from "forge-std/Script.sol";
import {BeefyClient} from "../src/BeefyClient.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/scripts/DeployLocal.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {WETH9} from "canonical-weth/WETH9.sol";
import {Script} from "forge-std/Script.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/scripts/DeployLocalGatewayLogic.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {AgentExecutor} from "../src/AgentExecutor.sol";
import {Gateway} from "../src//Gateway.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/scripts/FundAgent.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {WETH9} from "canonical-weth/WETH9.sol";
import {Script} from "forge-std/Script.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/scripts/UpgradeShell.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {WETH9} from "canonical-weth/WETH9.sol";
import {Script} from "forge-std/Script.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/scripts/westend/UpgradeShell.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {WETH9} from "canonical-weth/WETH9.sol";
import {Script} from "forge-std/Script.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/Agent.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

/// @title An agent contract that acts on behalf of a consensus system on Polkadot
/// @dev Instances of this contract act as an agents for arbitrary consensus systems on Polkadot. These consensus systems
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/AgentExecutor.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {AgentExecuteCommand, ParaID} from "./Types.sol";
import {SubstrateTypes} from "./SubstrateTypes.sol";
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/Assets.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {IERC20} from "./interfaces/IERC20.sol";
import {IGateway} from "./interfaces/IGateway.sol";
Expand Down Expand Up @@ -254,7 +254,7 @@ library Assets {
// It means that registration can be retried.
// But register a PNA here is not allowed
TokenInfo storage info = $.tokenRegistry[token];
if(info.foreignID != bytes32(0)) {
if (info.foreignID != bytes32(0)) {
revert TokenAlreadyRegistered();
}
info.isRegistered = true;
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/BeefyClient.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {ECDSA} from "openzeppelin/utils/cryptography/ECDSA.sol";
import {SubstrateMerkleProof} from "./utils/SubstrateMerkleProof.sol";
Expand Down
28 changes: 23 additions & 5 deletions contracts/src/Gateway.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {MerkleProof} from "openzeppelin/utils/cryptography/MerkleProof.sol";
import {Verification} from "./Verification.sol";
Expand Down Expand Up @@ -74,7 +74,7 @@ contract Gateway is IGateway, IInitializable, IUpgradable {
// Gas used for:
// 1. Mapping a command id to an implementation function
// 2. Calling implementation function
uint256 DISPATCH_OVERHEAD_GAS = 10_000;
uint256 constant DISPATCH_OVERHEAD_GAS = 10_000;

// The maximum fee that can be sent to a destination parachain to pay for execution (DOT).
// Has two functions:
Expand Down Expand Up @@ -108,6 +108,24 @@ contract Gateway is IGateway, IInitializable, IUpgradable {
_;
}

modifier nonreentrant() {
assembly {
// Check if flag is set and if true revert because it means the function is currently executing.
if tload(0) { revert(0, 0) }
Copy link
Contributor

@yrong yrong Jan 1, 2025

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use that pattern. But in the V2 code we use slot 0? Is this an issue?

Copy link
Contributor Author

@alistair-singh alistair-singh Jan 7, 2025

Choose a reason for hiding this comment

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

It should be fine because all our variables except for DISPATCH_OVERHEAD_GAS are constant or immutable. DISPATCH_OVERHEAD_GAS is neither constant or immutable but we never write to it, so storage slot 0 is never used in Gateway.
Just to be safe I made DISPATCH_OVERHEAD_GAS constant in:
6148116
And added a storage slot for the reentrancy guard so that we do not mistakenly re-use that slot 0 in future by adding variables to the mutable Gateway.
266a350

Nice catch!

Copy link
Collaborator

@vgeddes vgeddes Jan 8, 2025

Choose a reason for hiding this comment

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

Transient storage lives in a separate data location from normal storage, at least that's what the internet says. So transient slot 0 is separate from normal storage slot 0. Can check this with a test.

I generally don't like the way OpenZeppelin code is implemented, its verbose.


// Set the flag to mark the the function is currently executing.
tstore(0, 1)
}

// Execute the function here.
_;

assembly {
// Clear the flag as the function has completed execution.
tstore(0, 0)
}
}

constructor(
address beefyClient,
address agentExecutor,
Expand Down Expand Up @@ -137,7 +155,7 @@ contract Gateway is IGateway, IInitializable, IUpgradable {
InboundMessage calldata message,
bytes32[] calldata leafProof,
Verification.Proof calldata headerProof
) external {
) external nonreentrant {
uint256 startGas = gasleft();

Channel storage channel = _ensureChannel(message.channelID);
Expand Down Expand Up @@ -437,7 +455,7 @@ contract Gateway is IGateway, IInitializable, IUpgradable {
}

// Register an Ethereum-native token in the gateway and on AssetHub
function registerToken(address token) external payable {
function registerToken(address token) external payable nonreentrant {
_submitOutbound(Assets.registerToken(token));
}

Expand All @@ -457,7 +475,7 @@ contract Gateway is IGateway, IInitializable, IUpgradable {
MultiAddress calldata destinationAddress,
uint128 destinationFee,
uint128 amount
) external payable {
) external payable nonreentrant {
Ticket memory ticket = Assets.sendToken(
token, msg.sender, destinationChain, destinationAddress, destinationFee, MAX_DESTINATION_FEE, amount
);
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/GatewayProxy.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {ERC1967} from "./utils/ERC1967.sol";
import {Call} from "./utils/Call.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/MultiAddress.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

using {isIndex, asIndex, isAddress32, asAddress32, isAddress20, asAddress20} for MultiAddress global;

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/Params.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {ChannelID, OperatingMode} from "./Types.sol";
import {UD60x18} from "prb/math/src/UD60x18.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/Shell.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {Upgrade} from "./Upgrade.sol";
import {IInitializable} from "./interfaces/IInitializable.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/SubstrateTypes.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {ScaleCodec} from "./utils/ScaleCodec.sol";
import {ParaID} from "./Types.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/Token.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-FileCopyrightText: 2023 Axelar Network
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>

pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {IERC20} from "./interfaces/IERC20.sol";
import {IERC20Permit} from "./interfaces/IERC20Permit.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/TokenLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-FileCopyrightText: 2023 Axelar Network
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>

pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {IERC20} from "./interfaces/IERC20.sol";
import {IERC20Permit} from "./interfaces/IERC20Permit.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/Types.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {
MultiAddress, multiAddressFromUint32, multiAddressFromBytes32, multiAddressFromBytes20
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/Upgrade.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {ERC1967} from "./utils/ERC1967.sol";
import {Call} from "./utils/Call.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/Verification.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {SubstrateMerkleProof} from "./utils/SubstrateMerkleProof.sol";
import {BeefyClient} from "./BeefyClient.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/interfaces/IERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-FileCopyrightText: 2023 Axelar Network
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>

pragma solidity 0.8.25;
pragma solidity 0.8.28;

/**
* @dev Interface of the ERC20 standard as defined in the EIP.
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/interfaces/IERC20Permit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-FileCopyrightText: 2023 Axelar Network
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>

pragma solidity 0.8.25;
pragma solidity 0.8.28;

interface IERC20Permit {
error PermitExpired();
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/interfaces/IGateway.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {OperatingMode, InboundMessage, ParaID, ChannelID, MultiAddress} from "../Types.sol";
import {Verification} from "../Verification.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/interfaces/IInitializable.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

/**
* @title Initialization of gateway logic contracts
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/interfaces/IShell.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

interface IShell {
error Unauthorized();
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/interfaces/IUpgradable.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

interface IUpgradable {
// The new implementation address is a not a contract
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/interfaces/extensions/IERC20Metadata.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-FileCopyrightText: 2024 Snowfork <[email protected]>
// OpenZeppelin Contracts v4.4.1 (token/ERC20/extensions/IERC20Metadata.sol)

pragma solidity 0.8.25;
pragma solidity 0.8.28;

import "../IERC20.sol";

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/storage/AssetsStorage.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {TokenInfo, ParaID} from "../Types.sol";

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/storage/CoreStorage.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {Channel, OperatingMode, ChannelID, ParaID} from "../Types.sol";

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/storage/OperatorStorage.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

library OperatorStorage {
struct Layout {
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/storage/PricingStorage.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {UD60x18} from "prb/math/src/UD60x18.sol";

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/upgrades/Gateway202410.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import "../Gateway.sol";

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/utils/Address.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-FileCopyrightText: 2023 Axelar Network
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>

pragma solidity 0.8.25;
pragma solidity 0.8.28;

library Address {
// Checks whether `account` is a contract
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/utils/Bitfield.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
pragma solidity 0.8.25;
pragma solidity 0.8.28;

import {Bits} from "./Bits.sol";

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/utils/Bits.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
// Code from https://github.com/ethereum/solidity-examples
pragma solidity 0.8.25;
pragma solidity 0.8.28;

library Bits {
uint256 internal constant ONE = uint256(1);
Expand Down
Loading
Loading