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

CCIP 1.5 Migration #18

Open
wants to merge 56 commits into
base: ccip-gho
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
acd5d82
feat: enable remote rateLimitAdmin
CheyenneAtapour Jul 11, 2024
a03bad2
chore: prettier
CheyenneAtapour Jul 11, 2024
1268d85
fix: remove unnecessary modifer
CheyenneAtapour Jul 12, 2024
790bcef
chore: reorder state variables
CheyenneAtapour Jul 17, 2024
19829fd
feat: upgrade remote pool
CheyenneAtapour Jul 17, 2024
b075172
test: add tests after upgrading
CheyenneAtapour Jul 17, 2024
37aebc9
feat: reserve storage space for future upgrades
CheyenneAtapour Jul 17, 2024
68e504b
diff: Update diff for UpgradeableBurnMintTokenPool
miguelmtzinf Jul 18, 2024
393649d
ci: Fix ci
miguelmtzinf Jul 18, 2024
3ddfc85
ci: Fix ci
miguelmtzinf Jul 18, 2024
7af804e
test: show init reverting after upgrade
CheyenneAtapour Jul 18, 2024
f879d82
fix: remove gap
CheyenneAtapour Jul 19, 2024
5b0b65b
Merge remote-tracking branch 'origin/feat/remote-rate-limit' into upg…
CheyenneAtapour Jul 19, 2024
b7d9f6d
fix: return to original test suite
CheyenneAtapour Jul 19, 2024
a43c174
test: simple tests showing upgrade
CheyenneAtapour Jul 19, 2024
5bf3723
fix: add and correct comments
CheyenneAtapour Jul 22, 2024
e41fab2
fix: comments to match parent contract
CheyenneAtapour Jul 22, 2024
d2eb138
chore: prettier
CheyenneAtapour Jul 22, 2024
5c5e05a
Merge branch 'feat/remote-rate-limit' into upgrade-remote-pool-fix
CheyenneAtapour Jul 22, 2024
6d35524
fix: allow calls 1.2 on ramp during 1.5 migration by introducing lega…
DhairyaSethi Oct 10, 2024
126311b
chore: update certora & compiler used to match forge
DhairyaSethi Oct 10, 2024
1c75acb
Merge branch 'ccip-gho' into fix/legacy-on-ramp
DhairyaSethi Oct 16, 2024
bf968da
test: add fork base
DhairyaSethi Oct 16, 2024
65d0d11
wip: fork upgrade test
DhairyaSethi Oct 16, 2024
c1ae0c9
fix: onlyOnRamp condition
DhairyaSethi Oct 16, 2024
88184aa
test: add testnet legacy pools
DhairyaSethi Oct 16, 2024
3a74f86
test: successful send with upgrade
DhairyaSethi Oct 16, 2024
679c45f
test: add releaseOrMint tests
DhairyaSethi Oct 16, 2024
b8f6ce0
fix: legacyOnRamp -> proxyPool
DhairyaSethi Oct 16, 2024
6278e91
feat: set proxy pool only once for dest chain
DhairyaSethi Oct 16, 2024
c346fee
feat(tokenPools): disableInitializer on constructor, upd docs
DhairyaSethi Oct 16, 2024
29d6620
fix: revert UpgradeableBurnMintTokenPoolOld from #16 (was accidentall…
DhairyaSethi Oct 16, 2024
4477e39
chore: update diffs
DhairyaSethi Oct 16, 2024
6fd10bb
chore: use negative conditional code style
DhairyaSethi Oct 17, 2024
327fde2
docs: move base contract modification doc
DhairyaSethi Oct 17, 2024
59eda11
Update contracts/src/v0.8/ccip/test/pools/GHO/fork/GhoTokenPoolMigrat…
DhairyaSethi Oct 17, 2024
2ee9703
chore: update diffs
DhairyaSethi Oct 17, 2024
2d357cb
doc: for SendViaLegacyPool test
DhairyaSethi Oct 17, 2024
b82025c
chore: reorder imports for consistency with code style
DhairyaSethi Oct 17, 2024
60e78e8
fix: rm immutability of pool proxy
DhairyaSethi Oct 17, 2024
5153d5d
test: rename for consistency
DhairyaSethi Oct 17, 2024
e398344
fix: use chain agnostic proxypool
DhairyaSethi Oct 17, 2024
c8f7c37
fix: import on token pools to match chainlink style convention, upd diff
DhairyaSethi Oct 17, 2024
284cec3
fix: test renaming to match style convention
DhairyaSethi Oct 17, 2024
2a40f63
chore: fix import order
DhairyaSethi Oct 17, 2024
1de6efb
chore: rm internal inline, typo
DhairyaSethi Oct 17, 2024
6bad8e9
test: for 1_2OnRamp, refactor _messageToEvent
DhairyaSethi Oct 17, 2024
cecb762
chore: rm unsued
DhairyaSethi Oct 17, 2024
623c10c
doc: rm incorrect comment on proxy pool
DhairyaSethi Oct 17, 2024
459ffe8
test: fork test pre migration setup
DhairyaSethi Oct 18, 2024
017391c
test: expect events fork before migration
DhairyaSethi Oct 19, 2024
b56ab36
chore: revert name change in test
DhairyaSethi Oct 21, 2024
9dceff0
chore: upd comment
DhairyaSethi Oct 21, 2024
7b49a40
fix: rm disableInitializer as its handled by Initializable
DhairyaSethi Oct 22, 2024
7ae1477
chore: fix diff using diff algorithm patience
DhairyaSethi Oct 22, 2024
47a535b
fix: add proxyPool whitelist to onlyOffRamp modifier
DhairyaSethi Oct 22, 2024
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
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
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` modifier to accept transactions from ProxyPool
DhairyaSethi marked this conversation as resolved.
Show resolved Hide resolved
contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
error Unauthorized(address caller);

Expand All @@ -36,7 +36,9 @@ contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintToken
address token,
address armProxy,
bool allowlistEnabled
) UpgradeableTokenPool(IBurnMintERC20(token), armProxy, allowlistEnabled) {}
) UpgradeableTokenPool(IBurnMintERC20(token), armProxy, allowlistEnabled) {
_disableInitializers();
}

/// @dev Initializer
/// @dev The address passed as `owner` must accept ownership after initialization.
Expand All @@ -45,8 +47,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` modifier to accept transactions from ProxyPool
contract UpgradeableLockReleaseTokenPool is Initializable, UpgradeableTokenPool, ILiquidityContainer, ITypeAndVersion {
using SafeERC20 for IERC20;

Expand Down Expand Up @@ -71,6 +71,7 @@ contract UpgradeableLockReleaseTokenPool is Initializable, UpgradeableTokenPool,
bool acceptLiquidity
) UpgradeableTokenPool(IERC20(token), armProxy, allowlistEnabled) {
i_acceptLiquidity = acceptLiquidity;
_disableInitializers();
}

/// @dev Initializer
Expand All @@ -86,8 +87,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
38 changes: 34 additions & 4 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` 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;
DhairyaSethi marked this conversation as resolved.
Show resolved Hide resolved

/// @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,7 +259,9 @@ 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);
DhairyaSethi marked this conversation as resolved.
Show resolved Hide resolved
}
_;
}

Expand Down Expand Up @@ -317,4 +328,23 @@ 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 for the given remoteChainSelector
function getProxyPool() public view returns (address proxyPool) {
DhairyaSethi marked this conversation as resolved.
Show resolved Hide resolved
assembly ("memory-safe") {
proxyPool := shr(96, shl(96, sload(PROXY_POOL_SLOT)))
}
}

/// @notice Setter for proxy pool address, only callable by the DAO.
/// @dev This router is currently set for the Eth/Arb lane, and this pool is not expected
/// to support any other lanes in the future - hence can be stored agnostic to chain selector.
DhairyaSethi marked this conversation as resolved.
Show resolved Hide resolved
/// @param proxyPool The address of the proxy pool.
function setProxyPool(address proxyPool) external onlyOwner {
DhairyaSethi marked this conversation as resolved.
Show resolved Hide resolved
if (proxyPool == address(0)) revert ZeroAddressNotAllowed();
assembly ("memory-safe") {
sstore(PROXY_POOL_SLOT, proxyPool)
}
}
}
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..57a3226ef4 100644
--- a/src/v0.8/ccip/pools/BurnMintTokenPool.sol
+++ b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol
@@ -1,28 +1,90 @@
@@ -1,28 +1,92 @@
// 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` 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 @@ -56,18 +57,18 @@ index 9af0f22f4c..a46ff915e5 100644
- address router
- ) TokenPool(token, allowlist, armProxy, router) {}
+ bool allowlistEnabled
+ ) UpgradeableTokenPool(IBurnMintERC20(token), armProxy, allowlistEnabled) {}

- /// @inheritdoc BurnMintTokenPoolAbstract
+ ) UpgradeableTokenPool(IBurnMintERC20(token), armProxy, allowlistEnabled) {
+ _disableInitializers();
+ }
+
+ /// @dev Initializer
+ /// @dev The address passed as `owner` must accept ownership after initialization.
+ /// @dev The `allowlist` is only effective if pool is set to access-controlled mode
+ /// @param owner The address of the owner
+ /// @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 +91,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 All @@ -105,7 +106,8 @@ index 9af0f22f4c..a46ff915e5 100644
+
+ _setRateLimitConfig(remoteChainSelector, outboundConfig, inboundConfig);
+ }
+

- /// @inheritdoc BurnMintTokenPoolAbstract
+ /// @inheritdoc UpgradeableBurnMintTokenPoolAbstract
function _burn(uint256 amount) internal virtual override {
IBurnMintERC20(address(i_token)).burn(amount);
Expand Down
Loading
Loading