Skip to content

Commit

Permalink
feat: Add compatibility for CCIP 1.5 Migration (#18)
Browse files Browse the repository at this point in the history
* feat: enable remote rateLimitAdmin

* chore: prettier

* fix: remove unnecessary modifer

* chore: reorder state variables

* feat: upgrade remote pool

* test: add tests after upgrading

* feat: reserve storage space for future upgrades

* diff: Update diff for UpgradeableBurnMintTokenPool

* ci: Fix ci

* ci: Fix ci

* test: show init reverting after upgrade

* fix: remove gap

* fix: return to original test suite

* test: simple tests showing upgrade

* fix: add and correct comments

* fix: comments to match parent contract

* chore: prettier

* fix: allow calls 1.2 on ramp during 1.5 migration by introducing legacyOnRamp

* chore: update certora & compiler used to match forge

* test: add fork base

* wip: fork upgrade test

* fix: onlyOnRamp condition

* test: add testnet legacy pools

* test: successful send with upgrade

* test: add releaseOrMint tests

* fix: legacyOnRamp -> proxyPool

* feat: set proxy pool only once for dest chain

* feat(tokenPools): disableInitializer on constructor, upd docs

* fix: revert UpgradeableBurnMintTokenPoolOld from #16 (was accidentally merged)

* chore: update diffs

* chore: use negative conditional code style

* docs: move base contract modification doc

* Update contracts/src/v0.8/ccip/test/pools/GHO/fork/GhoTokenPoolMigrate1_4To1_5/ForkBase.t.sol

Co-authored-by: miguelmtz <[email protected]>

* chore: update diffs

* doc: for SendViaLegacyPool test

* chore: reorder imports for consistency with code style

* fix: rm immutability of pool proxy

* test: rename for consistency

* fix: use chain agnostic proxypool

* fix: import on token pools to match chainlink style convention, upd diff

* fix: test renaming to match style convention

* chore: fix import order

* chore: rm internal inline, typo

* test: for 1_2OnRamp, refactor _messageToEvent

* chore: rm unsued

* doc: rm incorrect comment on proxy pool

* test: fork test pre migration setup

* test: expect events fork before migration

* chore: revert name change in test

* chore: upd comment

* fix: rm disableInitializer as its handled by Initializable

* chore: fix diff using diff algorithm patience

* fix: add proxyPool whitelist to onlyOffRamp modifier

* test: improve off ramp tests

* chore: cleanup imports

* test: dynamically fetch dest gas amt

* fix: docs

---------

Co-authored-by: CheyenneAtapour <[email protected]>
Co-authored-by: miguelmtzinf <[email protected]>
Co-authored-by: miguelmtz <[email protected]>
  • Loading branch information
4 people authored Nov 7, 2024
1 parent d27d468 commit 46a4bdc
Show file tree
Hide file tree
Showing 18 changed files with 1,372 additions and 72 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/certora.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ jobs:
with: { java-version: "11", java-package: jre }

- name: Install certora cli
run: pip install certora-cli==7.6.3
run: pip install certora-cli==7.17.2

- name: Install solc
run: |
wget https://github.com/ethereum/solidity/releases/download/v0.8.10/solc-static-linux
wget https://github.com/ethereum/solidity/releases/download/v0.8.19/solc-static-linux
chmod +x solc-static-linux
sudo mv solc-static-linux /usr/local/bin/solc8.10
sudo mv solc-static-linux /usr/local/bin/solc8.19
- name: Verify rule ${{ matrix.rule }}
run: |
Expand Down
2 changes: 1 addition & 1 deletion certora/confs/ccip.conf
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"process": "emv",
"prover_args": ["-depth 10","-mediumTimeout 700"],
"smt_timeout": "600",
"solc": "solc8.10",
"solc": "solc8.19",
"verify": "UpgradeableLockReleaseTokenPool:certora/specs/ccip.spec",
"rule_sanity": "basic",
"msg": "CCIP"
Expand Down
4 changes: 4 additions & 0 deletions contracts/foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ gas_price = 1
block_timestamp = 1234567890
block_number = 12345

[rpc_endpoints]
sepolia = "https://sepolia.gateway.tenderly.co"
arb_sepolia = "https://arbitrum-sepolia.gateway.tenderly.co"

[profile.ccip]
solc_version = '0.8.19'
src = 'src/v0.8/ccip'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@
pragma solidity ^0.8.0;

import {Initializable} from "solidity-utils/contracts/transparent-proxy/Initializable.sol";

import {ITypeAndVersion} from "../../../shared/interfaces/ITypeAndVersion.sol";
import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol";

import {UpgradeableTokenPool} from "./UpgradeableTokenPool.sol";
import {UpgradeableBurnMintTokenPoolAbstract} from "./UpgradeableBurnMintTokenPoolAbstract.sol";
import {RateLimiter} from "../../libraries/RateLimiter.sol";

import {IRouter} from "../../interfaces/IRouter.sol";

/// @title UpgradeableBurnMintTokenPool
Expand All @@ -19,6 +17,8 @@ import {IRouter} from "../../interfaces/IRouter.sol";
/// - Implementation of Initializable to allow upgrades
/// - Move of allowlist and router definition to initialization stage
/// - Inclusion of rate limit admin who may configure rate limits in addition to owner
/// - Modifications from inherited contract (see contract for more details):
/// - UpgradeableTokenPool: Modify `onlyOnRamp` & `onlyOffRamp` modifier to accept transactions from ProxyPool
contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
error Unauthorized(address caller);

Expand All @@ -45,8 +45,7 @@ contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintToken
/// @param allowlist A set of addresses allowed to trigger lockOrBurn as original senders
/// @param router The address of the router
function initialize(address owner, address[] memory allowlist, address router) public virtual initializer {
if (owner == address(0)) revert ZeroAddressNotAllowed();
if (router == address(0)) revert ZeroAddressNotAllowed();
if (owner == address(0) || router == address(0)) revert ZeroAddressNotAllowed();
_transferOwnership(owner);

s_router = IRouter(router);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity ^0.8.0;

import {Initializable} from "solidity-utils/contracts/transparent-proxy/Initializable.sol";

import {ITypeAndVersion} from "../../../shared/interfaces/ITypeAndVersion.sol";
import {ILiquidityContainer} from "../../../rebalancer/interfaces/ILiquidityContainer.sol";

Expand All @@ -11,7 +10,6 @@ import {RateLimiter} from "../../libraries/RateLimiter.sol";

import {IERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/utils/SafeERC20.sol";

import {IRouter} from "../../interfaces/IRouter.sol";

/// @title UpgradeableLockReleaseTokenPool
Expand All @@ -21,6 +19,8 @@ import {IRouter} from "../../interfaces/IRouter.sol";
/// - Implementation of Initializable to allow upgrades
/// - Move of allowlist and router definition to initialization stage
/// - Addition of a bridge limit to regulate the maximum amount of tokens that can be transferred out (burned/locked)
/// - Modifications from inherited contract (see contract for more details):
/// - UpgradeableTokenPool: Modify `onlyOnRamp` & `onlyOffRamp` modifier to accept transactions from ProxyPool
contract UpgradeableLockReleaseTokenPool is Initializable, UpgradeableTokenPool, ILiquidityContainer, ITypeAndVersion {
using SafeERC20 for IERC20;

Expand Down Expand Up @@ -86,8 +86,7 @@ contract UpgradeableLockReleaseTokenPool is Initializable, UpgradeableTokenPool,
address router,
uint256 bridgeLimit
) public virtual initializer {
if (owner == address(0)) revert ZeroAddressNotAllowed();
if (router == address(0)) revert ZeroAddressNotAllowed();
if (owner == address(0) || router == address(0)) revert ZeroAddressNotAllowed();
_transferOwnership(owner);

s_router = IRouter(router);
Expand Down
40 changes: 35 additions & 5 deletions contracts/src/v0.8/ccip/pools/GHO/UpgradeableTokenPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ import {IERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/tok
import {IERC165} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/IERC165.sol";
import {EnumerableSet} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableSet.sol";

/// @notice Base abstract class with common functions for all token pools.
/// A token pool serves as isolated place for holding tokens and token specific logic
/// that may execute as tokens move across the bridge.
/// @title UpgradeableTokenPool
/// @author Aave Labs
/// @notice Upgradeable version of Chainlink's CCIP TokenPool
/// @dev Contract adaptations:
/// - Setters & Getters for new ProxyPool (to support 1.5 CCIP migration on the existing 1.4 Pool)
/// - Modify `onlyOnRamp` & `onlyOffRamp` modifier to accept transactions from ProxyPool
abstract contract UpgradeableTokenPool is IPool, OwnerIsCreator, IERC165 {
using EnumerableSet for EnumerableSet.AddressSet;
using EnumerableSet for EnumerableSet.UintSet;
Expand Down Expand Up @@ -55,6 +58,12 @@ abstract contract UpgradeableTokenPool is IPool, OwnerIsCreator, IERC165 {
RateLimiter.Config inboundRateLimiterConfig; // Inbound rate limited config, meaning the rate limits for all of the offRamps for the given chain
}

/// @dev The storage slot for Proxy Pool address, act as an on ramp "wrapper" post ccip 1.5 migration.
/// @dev This was added to continue support for 1.2 onRamp during 1.5 migration, and is stored
/// this way to avoid storage collision.
// bytes32(uint256(keccak256("ccip.pools.GHO.UpgradeableTokenPool.proxyPool")) - 1)
bytes32 internal constant PROXY_POOL_SLOT = 0x75bb68f1b335d4dab6963140ecff58281174ef4362bb85a8593ab9379f24fae2;

/// @dev The bridgeable token that is managed by this pool.
IERC20 internal immutable i_token;
/// @dev The address of the arm proxy
Expand Down Expand Up @@ -250,15 +259,19 @@ abstract contract UpgradeableTokenPool is IPool, OwnerIsCreator, IERC165 {
/// is a permissioned onRamp for the given chain on the Router.
modifier onlyOnRamp(uint64 remoteChainSelector) {
if (!isSupportedChain(remoteChainSelector)) revert ChainNotAllowed(remoteChainSelector);
if (!(msg.sender == s_router.getOnRamp(remoteChainSelector))) revert CallerIsNotARampOnRouter(msg.sender);
if (!(msg.sender == getProxyPool() || msg.sender == s_router.getOnRamp(remoteChainSelector))) {
revert CallerIsNotARampOnRouter(msg.sender);
}
_;
}

/// @notice Checks whether remote chain selector is configured on this contract, and if the msg.sender
/// is a permissioned offRamp for the given chain on the Router.
modifier onlyOffRamp(uint64 remoteChainSelector) {
if (!isSupportedChain(remoteChainSelector)) revert ChainNotAllowed(remoteChainSelector);
if (!s_router.isOffRamp(remoteChainSelector, msg.sender)) revert CallerIsNotARampOnRouter(msg.sender);
if (!(msg.sender == getProxyPool() || s_router.isOffRamp(remoteChainSelector, msg.sender))) {
revert CallerIsNotARampOnRouter(msg.sender);
}
_;
}

Expand Down Expand Up @@ -317,4 +330,21 @@ abstract contract UpgradeableTokenPool is IPool, OwnerIsCreator, IERC165 {
if (IARM(i_armProxy).isCursed()) revert BadARMSignal();
_;
}

/// @notice Getter for proxy pool address.
/// @return proxyPool The proxy pool address.
function getProxyPool() public view returns (address proxyPool) {
assembly ("memory-safe") {
proxyPool := shr(96, shl(96, sload(PROXY_POOL_SLOT)))
}
}

/// @notice Setter for proxy pool address, only callable by the DAO.
/// @param proxyPool The address of the proxy pool.
function setProxyPool(address proxyPool) external onlyOwner {
if (proxyPool == address(0)) revert ZeroAddressNotAllowed();
assembly ("memory-safe") {
sstore(PROXY_POOL_SLOT, proxyPool)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ index f5eb135186..e228732855 100644
// SPDX-License-Identifier: BUSL-1.1
-pragma solidity 0.8.19;
+pragma solidity ^0.8.0;

-import {IBurnMintERC20} from "../../shared/token/ERC20/IBurnMintERC20.sol";
+import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol";

-import {TokenPool} from "./TokenPool.sol";
+import {UpgradeableTokenPool} from "./UpgradeableTokenPool.sol";

-abstract contract BurnMintTokenPoolAbstract is TokenPool {
+abstract contract UpgradeableBurnMintTokenPoolAbstract is UpgradeableTokenPool {
/// @notice Contains the specific burn call for a pool.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,24 @@
```diff
diff --git a/src/v0.8/ccip/pools/BurnMintTokenPool.sol b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol
index 9af0f22f4c..a46ff915e5 100644
index 9af0f22f4c..a5cecc0430 100644
--- a/src/v0.8/ccip/pools/BurnMintTokenPool.sol
+++ b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol
@@ -1,28 +1,90 @@
// SPDX-License-Identifier: BUSL-1.1
-pragma solidity 0.8.19;
+pragma solidity ^0.8.0;

-import {ITypeAndVersion} from "../../shared/interfaces/ITypeAndVersion.sol";
-import {IBurnMintERC20} from "../../shared/token/ERC20/IBurnMintERC20.sol";
+import {Initializable} from "solidity-utils/contracts/transparent-proxy/Initializable.sol";

-import {TokenPool} from "./TokenPool.sol";
-import {BurnMintTokenPoolAbstract} from "./BurnMintTokenPoolAbstract.sol";
+import {ITypeAndVersion} from "../../../shared/interfaces/ITypeAndVersion.sol";
+import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol";
+

-import {TokenPool} from "./TokenPool.sol";
-import {BurnMintTokenPoolAbstract} from "./BurnMintTokenPoolAbstract.sol";
+import {UpgradeableTokenPool} from "./UpgradeableTokenPool.sol";
+import {UpgradeableBurnMintTokenPoolAbstract} from "./UpgradeableBurnMintTokenPoolAbstract.sol";
+import {RateLimiter} from "../../libraries/RateLimiter.sol";
+
+import {IRouter} from "../../interfaces/IRouter.sol";
+
+/// @title UpgradeableBurnMintTokenPool
Expand All @@ -29,17 +27,20 @@ index 9af0f22f4c..a46ff915e5 100644
+/// @dev Contract adaptations:
+/// - Implementation of Initializable to allow upgrades
+/// - Move of allowlist and router definition to initialization stage
+/// - Inclusion of rate limit admin who may configure rate limits in addition to owner
+/// - Modifications from inherited contract (see contract for more details):
+/// - UpgradeableTokenPool: Modify `onlyOnRamp` & `onlyOffRamp` modifier to accept transactions from ProxyPool
+contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
+ error Unauthorized(address caller);

-/// @notice This pool mints and burns a 3rd-party token.
-/// @dev Pool whitelisting mode is set in the constructor and cannot be modified later.
-/// It either accepts any address as originalSender, or only accepts whitelisted originalSender.
-/// The only way to change whitelisting mode is to deploy a new pool.
-/// If that is expected, please make sure the token's burner/minter roles are adjustable.
-contract BurnMintTokenPool is BurnMintTokenPoolAbstract, ITypeAndVersion {
string public constant override typeAndVersion = "BurnMintTokenPool 1.4.0";

+ /// @notice The address of the rate limiter admin.
+ /// @dev Can be address(0) if none is configured.
+ address internal s_rateLimitAdmin;
Expand All @@ -57,7 +58,7 @@ index 9af0f22f4c..a46ff915e5 100644
- ) TokenPool(token, allowlist, armProxy, router) {}
+ bool allowlistEnabled
+ ) UpgradeableTokenPool(IBurnMintERC20(token), armProxy, allowlistEnabled) {}

- /// @inheritdoc BurnMintTokenPoolAbstract
+ /// @dev Initializer
+ /// @dev The address passed as `owner` must accept ownership after initialization.
Expand All @@ -66,8 +67,7 @@ index 9af0f22f4c..a46ff915e5 100644
+ /// @param allowlist A set of addresses allowed to trigger lockOrBurn as original senders
+ /// @param router The address of the router
+ function initialize(address owner, address[] memory allowlist, address router) public virtual initializer {
+ if (owner == address(0)) revert ZeroAddressNotAllowed();
+ if (router == address(0)) revert ZeroAddressNotAllowed();
+ if (owner == address(0) || router == address(0)) revert ZeroAddressNotAllowed();
+ _transferOwnership(owner);
+
+ s_router = IRouter(router);
Expand All @@ -90,7 +90,7 @@ index 9af0f22f4c..a46ff915e5 100644
+ return s_rateLimitAdmin;
+ }
+
+ /// @notice Sets the rate limiter admin address.
+ /// @notice Sets the chain rate limiter config.
+ /// @dev Only callable by the owner or the rate limiter admin. NOTE: overwrites the normal
+ /// onlyAdmin check in the base implementation to also allow the rate limiter admin.
+ /// @param remoteChainSelector The remote chain selector for which the rate limits apply.
Expand Down
Loading

0 comments on commit 46a4bdc

Please sign in to comment.