Skip to content

Commit

Permalink
fix: Apply fixes based on code review (#11)
Browse files Browse the repository at this point in the history
* fix: Use modern version of Initializable

* fix: Update diffs

* fix: Add event emission on bridge limit admin update

* fix: Rebuild GHO diffs

* fix: Fix certora and test config files

* fix: Remove unneeded revision getter
  • Loading branch information
miguelmtzinf authored Jun 11, 2024
1 parent f72c0f7 commit 7c777f9
Show file tree
Hide file tree
Showing 17 changed files with 155 additions and 245 deletions.
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@
[submodule "contracts/foundry-lib/solidity-utils"]
path = contracts/foundry-lib/solidity-utils
url = https://github.com/bgd-labs/solidity-utils
[submodule "contracts/foundry-lib/gho-core"]
path = contracts/foundry-lib/gho-core
url = https://github.com/aave/gho-core
3 changes: 3 additions & 0 deletions certora/confs/ccip.conf
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
"contracts/src/v0.8/ccip/pools/GHO/UpgradeableLockReleaseTokenPool.sol",
"certora/harness/SimpleERC20.sol"
],
"packages": [
"solidity-utils/=contracts/foundry-lib/solidity-utils/src/"
],
"link": [
"UpgradeableLockReleaseTokenPool:i_token=SimpleERC20"
],
Expand Down
1 change: 1 addition & 0 deletions contracts/foundry-lib/gho-core
Submodule gho-core added at a8d05e
2 changes: 1 addition & 1 deletion contracts/remappings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ forge-std/=foundry-lib/forge-std/src/
hardhat/=node_modules/hardhat/
@eth-optimism/=node_modules/@eth-optimism/
@scroll-tech/=node_modules/@scroll-tech/
@aave/gho-core/=node_modules/@aave/gho/src/contracts/
@aave/gho-core/=foundry-lib/gho-core/src/contracts/
solidity-utils/=foundry-lib/solidity-utils/src/
18 changes: 4 additions & 14 deletions contracts/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
// SPDX-License-Identifier: BUSL-1.1
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 {IRouter} from "../../interfaces/IRouter.sol";
import {VersionedInitializable} from "./VersionedInitializable.sol";

/// @title UpgradeableBurnMintTokenPool
/// @author Aave Labs
/// @notice Upgradeable version of Chainlink's CCIP BurnMintTokenPool
/// @dev Contract adaptations:
/// - Implementation of VersionedInitializable to allow upgrades
/// - Implementation of Initializable to allow upgrades
/// - Move of allowlist and router definition to initialization stage
contract UpgradeableBurnMintTokenPool is VersionedInitializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
string public constant override typeAndVersion = "BurnMintTokenPool 1.4.0";

/// @dev Constructor
Expand Down Expand Up @@ -52,15 +53,4 @@ contract UpgradeableBurnMintTokenPool is VersionedInitializable, UpgradeableBurn
function _burn(uint256 amount) internal virtual override {
IBurnMintERC20(address(i_token)).burn(amount);
}

/// @notice Returns the revision number
/// @return The revision number
function REVISION() public pure virtual returns (uint256) {
return 1;
}

/// @inheritdoc VersionedInitializable
function getRevision() internal pure virtual override returns (uint256) {
return REVISION();
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: BUSL-1.1
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,21 +13,15 @@ import {IERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/tok
import {SafeERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/utils/SafeERC20.sol";

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

/// @title UpgradeableLockReleaseTokenPool
/// @author Aave Labs
/// @notice Upgradeable version of Chainlink's CCIP LockReleaseTokenPool
/// @dev Contract adaptations:
/// - Implementation of VersionedInitializable to allow upgrades
/// - 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)
contract UpgradeableLockReleaseTokenPool is
VersionedInitializable,
UpgradeableTokenPool,
ILiquidityContainer,
ITypeAndVersion
{
contract UpgradeableLockReleaseTokenPool is Initializable, UpgradeableTokenPool, ILiquidityContainer, ITypeAndVersion {
using SafeERC20 for IERC20;

error InsufficientLiquidity();
Expand All @@ -34,7 +30,9 @@ contract UpgradeableLockReleaseTokenPool is

error BridgeLimitExceeded(uint256 bridgeLimit);
error NotEnoughBridgedAmount();

event BridgeLimitUpdated(uint256 oldBridgeLimit, uint256 newBridgeLimit);
event BridgeLimitAdminUpdated(address indexed oldAdmin, address indexed newAdmin);

string public constant override typeAndVersion = "LockReleaseTokenPool 1.4.0";

Expand Down Expand Up @@ -197,7 +195,9 @@ contract UpgradeableLockReleaseTokenPool is
/// @dev Only callable by the owner.
/// @param bridgeLimitAdmin The new bridge limit admin address.
function setBridgeLimitAdmin(address bridgeLimitAdmin) external onlyOwner {
address oldAdmin = s_bridgeLimitAdmin;
s_bridgeLimitAdmin = bridgeLimitAdmin;
emit BridgeLimitAdminUpdated(oldAdmin, bridgeLimitAdmin);
}

/// @notice Gets the bridge limit
Expand Down Expand Up @@ -263,15 +263,4 @@ contract UpgradeableLockReleaseTokenPool is

_setRateLimitConfig(remoteChainSelector, outboundConfig, inboundConfig);
}

/// @notice Returns the revision number
/// @return The revision number
function REVISION() public pure virtual returns (uint256) {
return 1;
}

/// @inheritdoc VersionedInitializable
function getRevision() internal pure virtual override returns (uint256) {
return REVISION();
}
}
77 changes: 0 additions & 77 deletions contracts/src/v0.8/ccip/pools/GHO/VersionedInitializable.sol

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
```diff
diff --git a/src/v0.8/ccip/pools/BurnMintTokenPoolAbstract.sol b/src/v0.8/ccip/pools/UpgradeableBurnMintTokenPoolAbstract.sol
index f5eb135186..651965e40b 100644
diff --git a/src/v0.8/ccip/pools/BurnMintTokenPoolAbstract.sol b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPoolAbstract.sol
index f5eb135186..e228732855 100644
--- a/src/v0.8/ccip/pools/BurnMintTokenPoolAbstract.sol
+++ b/src/v0.8/ccip/pools/UpgradeableBurnMintTokenPoolAbstract.sol
+++ b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPoolAbstract.sol
@@ -1,11 +1,11 @@
// 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 {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,39 +1,42 @@
```diff
diff --git a/src/v0.8/ccip/pools/BurnMintTokenPool.sol b/src/v0.8/ccip/pools/UpgradeableBurnMintTokenPool.sol
index 9af0f22f4c..f07f8c3a28 100644
diff --git a/src/v0.8/ccip/pools/BurnMintTokenPool.sol b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol
index 9af0f22f4c..58be87812f 100644
--- a/src/v0.8/ccip/pools/BurnMintTokenPool.sol
+++ b/src/v0.8/ccip/pools/UpgradeableBurnMintTokenPool.sol
@@ -1,29 +1,66 @@
+++ b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol
@@ -1,28 +1,55 @@
// 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 {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 {UpgradeableTokenPool} from "./UpgradeableTokenPool.sol";
+import {UpgradeableBurnMintTokenPoolAbstract} from "./UpgradeableBurnMintTokenPoolAbstract.sol";

+import {ITypeAndVersion} from "../../../shared/interfaces/ITypeAndVersion.sol";
+import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol";
-/// @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 {
+import {IRouter} from "../interfaces/IRouter.sol";
+import {VersionedInitializable} from "./VersionedInitializable.sol";
+import {UpgradeableTokenPool} from "./UpgradeableTokenPool.sol";
+import {UpgradeableBurnMintTokenPoolAbstract} from "./UpgradeableBurnMintTokenPoolAbstract.sol";
+
+import {IRouter} from "../../interfaces/IRouter.sol";
+
+/// @title UpgradeableBurnMintTokenPool
+/// @author Aave Labs
+/// @notice Upgradeable version of Chainlink's CCIP BurnMintTokenPool
+/// @dev Contract adaptations:
+/// - Implementation of VersionedInitializable to allow upgrades
+/// - Implementation of Initializable to allow upgrades
+/// - Move of allowlist and router definition to initialization stage
+contract UpgradeableBurnMintTokenPool is VersionedInitializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
+contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
string public constant override typeAndVersion = "BurnMintTokenPool 1.4.0";

+ /// @dev Constructor
+ /// @param token The bridgeable token that is managed by this pool.
+ /// @param armProxy The address of the arm proxy
Expand All @@ -47,7 +50,7 @@ index 9af0f22f4c..f07f8c3a28 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 @@ -72,16 +75,4 @@ index 9af0f22f4c..f07f8c3a28 100644
function _burn(uint256 amount) internal virtual override {
IBurnMintERC20(address(i_token)).burn(amount);
}
+
+ /// @notice Returns the revision number
+ /// @return The revision number
+ function REVISION() public pure virtual returns (uint256) {
+ return 1;
+ }
+
+ /// @inheritdoc VersionedInitializable
+ function getRevision() internal pure virtual override returns (uint256) {
+ return REVISION();
+ }
}
```
Loading

0 comments on commit 7c777f9

Please sign in to comment.