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.

Sign up for GitHub

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

Jump to bottom

feat: ERC20Custody and ZetaConnector uups upgradable #385

Merged
merged 6 commits into from
Oct 14, 2024
Merged

feat: ERC20Custody and ZetaConnector uups upgradable #385

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
579a1ac
erc20 upgradable
skosito Oct 10, 2024
758ea1c
connectors upgradable
skosito Oct 10, 2024
4e1eae7
fmt
skosito Oct 10, 2024
0638977
missing initializers
skosito Oct 11, 2024
ec2fd2a
add upgrade unit test for every upgradable contract
skosito Oct 11, 2024
8084b72
generate
skosito Oct 14, 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
34 changes: 27 additions & 7 deletions v2/contracts/evm/ERC20Custody.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,30 @@ import { IGatewayEVM } from "./interfaces/IGatewayEVM.sol";

import { RevertContext } from "../../contracts/Revert.sol";

import "@openzeppelin/contracts/access/AccessControl.sol";
import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "@openzeppelin/contracts/utils/Pausable.sol";
import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";

/// @title ERC20Custody
/// @notice Holds the ERC20 tokens deposited on ZetaChain and includes functionality to call a contract.
/// @dev This contract does not call smart contracts directly, it passes through the Gateway contract.
contract ERC20Custody is IERC20Custody, ReentrancyGuard, AccessControl, Pausable {
contract ERC20Custody is
Initializable,
UUPSUpgradeable,
IERC20Custody,
ReentrancyGuardUpgradeable,
AccessControlUpgradeable,
PausableUpgradeable
{
using SafeERC20 for IERC20;

/// @notice Gateway contract.
IGatewayEVM public immutable gateway;
IGatewayEVM public gateway;
/// @notice Mapping of whitelisted tokens => true/false.
mapping(address => bool) public whitelisted;
/// @notice The address of the TSS (Threshold Signature Scheme) contract.
Expand All @@ -33,12 +43,18 @@ contract ERC20Custody is IERC20Custody, ReentrancyGuard, AccessControl, Pausable
/// @notice New role identifier for whitelister role.
bytes32 public constant WHITELISTER_ROLE = keccak256("WHITELISTER_ROLE");

/// @notice Constructor for ERC20Custody.
/// @notice Initializer for ERC20Custody.
/// @dev Set admin as default admin and pauser, and tssAddress as tss role.
constructor(address gateway_, address tssAddress_, address admin_) {
function initialize(address gateway_, address tssAddress_, address admin_) public initializer {
if (gateway_ == address(0) || tssAddress_ == address(0) || admin_ == address(0)) {
revert ZeroAddress();
}

__UUPSUpgradeable_init();
__ReentrancyGuard_init();
__AccessControl_init();
__Pausable_init();

gateway = IGatewayEVM(gateway_);
tssAddress = tssAddress_;
_grantRole(DEFAULT_ADMIN_ROLE, admin_);
Expand All @@ -48,6 +64,10 @@ contract ERC20Custody is IERC20Custody, ReentrancyGuard, AccessControl, Pausable
_grantRole(WHITELISTER_ROLE, tssAddress_);
}

/// @dev Authorizes the upgrade of the contract, sender must be owner.
/// @param newImplementation Address of the new implementation.
function _authorizeUpgrade(address newImplementation) internal override onlyRole(DEFAULT_ADMIN_ROLE) { }

/// @notice Pause contract.
function pause() external onlyRole(PAUSER_ROLE) {
_pause();
Expand Down
44 changes: 36 additions & 8 deletions v2/contracts/evm/ZetaConnectorBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import "@openzeppelin/contracts/access/AccessControl.sol";
import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "@openzeppelin/contracts/utils/Pausable.sol";
import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";

import { RevertContext } from "../../contracts/Revert.sol";
import { IGatewayEVM, IGatewayEVMErrors, IGatewayEVMEvents } from "../../contracts/evm/interfaces/IGatewayEVM.sol";
Expand All @@ -14,16 +16,23 @@ import "../../contracts/evm/interfaces/IZetaConnector.sol";
/// @title ZetaConnectorBase
/// @notice Abstract base contract for ZetaConnector.
/// @dev This contract implements basic functionality for handling tokens and interacting with the Gateway contract.
abstract contract ZetaConnectorBase is IZetaConnectorEvents, ReentrancyGuard, Pausable, AccessControl {
abstract contract ZetaConnectorBase is
Initializable,
UUPSUpgradeable,
IZetaConnectorEvents,
ReentrancyGuardUpgradeable,
PausableUpgradeable,
AccessControlUpgradeable
{
using SafeERC20 for IERC20;

/// @notice Error indicating that a zero address was provided.
error ZeroAddress();

/// @notice The Gateway contract used for executing cross-chain calls.
IGatewayEVM public immutable gateway;
IGatewayEVM public gateway;
/// @notice The address of the Zeta token.
address public immutable zetaToken;
address public zetaToken;
/// @notice The address of the TSS (Threshold Signature Scheme) contract.
address public tssAddress;

Expand All @@ -34,12 +43,27 @@ abstract contract ZetaConnectorBase is IZetaConnectorEvents, ReentrancyGuard, Pa
/// @notice New role identifier for tss role.
bytes32 public constant TSS_ROLE = keccak256("TSS_ROLE");

/// @notice Constructor for ZetaConnectors.
/// @notice Initializer for ZetaConnectors.
/// @dev Set admin as default admin and pauser, and tssAddress as tss role.
constructor(address gateway_, address zetaToken_, address tssAddress_, address admin_) {
function initialize(
address gateway_,
address zetaToken_,
address tssAddress_,
address admin_
)
public
virtual
initializer
{
if (gateway_ == address(0) || zetaToken_ == address(0) || tssAddress_ == address(0) || admin_ == address(0)) {
revert ZeroAddress();
}

__UUPSUpgradeable_init();
__ReentrancyGuard_init();
__AccessControl_init();
__Pausable_init();

gateway = IGatewayEVM(gateway_);
zetaToken = zetaToken_;
tssAddress = tssAddress_;
Expand All @@ -50,6 +74,10 @@ abstract contract ZetaConnectorBase is IZetaConnectorEvents, ReentrancyGuard, Pa
_grantRole(PAUSER_ROLE, admin_);
}

/// @dev Authorizes the upgrade of the contract, sender must be owner.
/// @param newImplementation Address of the new implementation.
function _authorizeUpgrade(address newImplementation) internal override onlyRole(DEFAULT_ADMIN_ROLE) { }

/// @notice Update tss address
/// @param newTSSAddress new tss address
function updateTSSAddress(address newTSSAddress) external onlyRole(DEFAULT_ADMIN_ROLE) {
Expand Down
10 changes: 7 additions & 3 deletions v2/contracts/evm/ZetaConnectorNative.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,18 @@ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
contract ZetaConnectorNative is ZetaConnectorBase {
using SafeERC20 for IERC20;

constructor(
function initialize(
address gateway_,
address zetaToken_,
address tssAddress_,
address admin_
)
ZetaConnectorBase(gateway_, zetaToken_, tssAddress_, admin_)
{ }
public
override
initializer
{
super.initialize(gateway_, zetaToken_, tssAddress_, admin_);
}

/// @notice Withdraw tokens to a specified address.
/// @param to The address to withdraw tokens to.
Expand Down
18 changes: 12 additions & 6 deletions v2/contracts/evm/ZetaConnectorNonNative.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,29 @@ import "./interfaces/IZetaNonEthNew.sol";
/// @notice Implementation of ZetaConnectorBase for non-native token handling.
/// @dev This contract mints and burns Zeta tokens and interacts with the Gateway contract.
contract ZetaConnectorNonNative is ZetaConnectorBase {
/// @notice Max supply for minting.
uint256 public maxSupply = type(uint256).max;

/// @notice Event triggered when max supply is updated.
/// @param maxSupply New max supply.
event MaxSupplyUpdated(uint256 maxSupply);

error ExceedsMaxSupply();

constructor(
/// @notice Max supply for minting.
uint256 public maxSupply;

function initialize(
address gateway_,
address zetaToken_,
address tssAddress_,
address admin_
)
ZetaConnectorBase(gateway_, zetaToken_, tssAddress_, admin_)
{ }
public
override
initializer
{
super.initialize(gateway_, zetaToken_, tssAddress_, admin_);

maxSupply = type(uint256).max;
}

/// @notice Set max supply for minting.
/// @param maxSupply_ New max supply.
Expand Down
44 changes: 36 additions & 8 deletions v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,55 @@
pragma solidity 0.8.26;

import "forge-std/Script.sol";
import "src/evm/ERC20Custody.sol";
import "contracts/evm/ERC20Custody.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

contract DeployERC20Custody is Script {
function run() external {
address payable tss = payable(vm.envAddress("TSS_ADDRESS"));
address admin = vm.envAddress("ERC20_CUSTODY_ADMIN_ADDRESS_EVM");
address gateway = vm.envAddress("GATEWAY_PROXY_EVM");

bytes32 salt = keccak256("ERC20Custody");
address expectedImplAddress;
address expectedProxyAddress;

bytes32 implSalt = keccak256("ERC20Custody");
bytes32 proxySalt = keccak256("ERC20CustodyProxy");

vm.startBroadcast();

ERC20Custody custody = new ERC20Custody{salt: salt}(gateway, tss, admin);
require(address(custody) != address(0), "deployment failed");
expectedImplAddress = vm.computeCreate2Address(
implSalt,
hashInitCode(type(ERC20Custody).creationCode)
);

ERC20Custody erc20CustodyImpl = new ERC20Custody{salt: implSalt}();
require(address(erc20CustodyImpl) != address(0), "erc20CustodyImpl deployment failed");

require(expectedImplAddress == address(erc20CustodyImpl), "impl address doesn't match expected address");
Comment on lines +17 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor repetitive deployment logic into helper functions

The code for computing expected addresses and deploying both the implementation and proxy contracts is repetitive. To enhance maintainability and reduce duplication, consider abstracting this logic into reusable helper functions.

For example, you could create helper functions like computeExpectedAddress and deployContract:

function computeExpectedAddress(bytes32 salt, bytes memory bytecode) internal view returns (address) {
    return vm.computeCreate2Address(salt, hashInitCode(bytecode));
}

function deployContract(bytes32 salt, bytes memory bytecode) internal returns (address deployedAddress) {
    assembly {
        deployedAddress := create2(0, add(bytecode, 0x20), mload(bytecode), salt)
    }
    require(deployedAddress != address(0), "Deployment failed");
}

Refactored implementation deployment:

-expectedImplAddress = vm.computeCreate2Address(
-    implSalt,
-    hashInitCode(type(ERC20Custody).creationCode)
-);
-
-ERC20Custody erc20CustodyImpl = new ERC20Custody{salt: implSalt}();
-require(address(erc20CustodyImpl) != address(0), "erc20CustodyImpl deployment failed");
-
-require(expectedImplAddress == address(erc20CustodyImpl), "impl address doesn't match expected address");
+bytes memory implBytecode = type(ERC20Custody).creationCode;
+expectedImplAddress = computeExpectedAddress(implSalt, implBytecode);
+address implAddress = deployContract(implSalt, implBytecode);
+require(expectedImplAddress == implAddress, "Implementation address mismatch");
+ERC20Custody erc20CustodyImpl = ERC20Custody(implAddress);

Refactored proxy deployment:

-expectedProxyAddress = vm.computeCreate2Address(
-    proxySalt,
-    hashInitCode(
-        type(ERC1967Proxy).creationCode,
-        abi.encode(
-            address(erc20CustodyImpl),
-            abi.encodeWithSelector(ERC20Custody.initialize.selector, gateway, tss, admin)
-        )
-    )
-);
-
-ERC1967Proxy erc20CustodyProxy = new ERC1967Proxy{salt: proxySalt}(
-    address(erc20CustodyImpl),
-    abi.encodeWithSelector(ERC20Custody.initialize.selector, gateway, tss, admin)
-);
-require(address(erc20CustodyProxy) != address(0), "erc20CustodyProxy deployment failed");
-
-require(expectedProxyAddress == address(erc20CustodyProxy), "Proxy address mismatch");
+bytes memory proxyInitCode = abi.encodePacked(
+    type(ERC1967Proxy).creationCode,
+    abi.encode(
+        address(erc20CustodyImpl),
+        abi.encodeWithSelector(ERC20Custody.initialize.selector, gateway, tss, admin)
+    )
+);
+expectedProxyAddress = computeExpectedAddress(proxySalt, proxyInitCode);
+address proxyAddress = deployContract(proxySalt, proxyInitCode);
+require(expectedProxyAddress == proxyAddress, "Proxy address mismatch");
+ERC1967Proxy erc20CustodyProxy = ERC1967Proxy(payable(proxyAddress));

Also applies to: 32-49


address expectedAddr = vm.computeCreate2Address(
salt,
hashInitCode(type(ERC20Custody).creationCode, abi.encode(gateway, tss, admin))
expectedProxyAddress = vm.computeCreate2Address(
proxySalt,
hashInitCode(
type(ERC1967Proxy).creationCode,
abi.encode(
address(erc20CustodyImpl),
abi.encodeWithSelector(ERC20Custody.initialize.selector, gateway, tss, admin)
)
)
);

require(expectedAddr == address(custody), "erc20 custody address doesn't match expected address");
ERC1967Proxy erc20CustodyProxy = new ERC1967Proxy{salt: proxySalt}(
address(erc20CustodyImpl),
abi.encodeWithSelector(ERC20Custody.initialize.selector, gateway, tss, admin)
);
require(address(erc20CustodyProxy) != address(0), "erc20CustodyProxy deployment failed");

require(expectedProxyAddress == address(erc20CustodyProxy), "proxy address doesn't match expected address");

ERC20Custody erc20Custody = ERC20Custody(address(erc20CustodyProxy));
require(erc20Custody.tssAddress() == tss, "tss not set");
require(address(erc20Custody.gateway()) == gateway, "gateway not set");

vm.stopBroadcast();
}
Expand Down
3 changes: 1 addition & 2 deletions v2/scripts/deploy/deterministic/DeployGatewayEVM.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity 0.8.26;

import "forge-std/Script.sol";
import "src/evm/GatewayEVM.sol";
import "contracts/evm/GatewayEVM.sol";
import "test/utils/TestERC20.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

Expand Down Expand Up @@ -47,7 +47,6 @@ contract DeployGatewayEVM is Script {
);
require(address(gatewayProxy) != address(0), "gatewayProxy deployment failed");


require(expectedProxyAddress == address(gatewayProxy), "proxy address doesn't match expected address");

GatewayEVM gateway = GatewayEVM(address(gatewayProxy));
Expand Down
45 changes: 37 additions & 8 deletions v2/scripts/deploy/deterministic/DeployZetaConnectorNonNative.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
pragma solidity 0.8.26;

import "forge-std/Script.sol";
import "src/evm/ZetaConnectorNonNative.sol";
import "contracts/evm/ZetaConnectorNonNative.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

contract DeployZetaConnectorNonNative is Script {
function run() external {
Expand All @@ -11,19 +12,47 @@ contract DeployZetaConnectorNonNative is Script {
address gateway = vm.envAddress("GATEWAY_PROXY_EVM");
address zeta = vm.envAddress("ZETA_ERC20_EVM");

bytes32 salt = keccak256("ZetaConnectorNonNative");
address expectedImplAddress;
address expectedProxyAddress;

bytes32 implSalt = keccak256("ZetaConnectorNonNative");
bytes32 proxySalt = keccak256("ZetaConnectorNonNativeProxy");
Comment on lines +18 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Define salt strings as constants for clarity

Consider defining the salt strings used in keccak256 as constants to enhance readability and maintainability.

Apply this diff to define the salt strings as constants:

+    string constant IMPL_SALT_STRING = "ZetaConnectorNonNative";
+    string constant PROXY_SALT_STRING = "ZetaConnectorNonNativeProxy";

-    bytes32 implSalt = keccak256("ZetaConnectorNonNative");
-    bytes32 proxySalt = keccak256("ZetaConnectorNonNativeProxy");
+    bytes32 implSalt = keccak256(abi.encodePacked(IMPL_SALT_STRING));
+    bytes32 proxySalt = keccak256(abi.encodePacked(PROXY_SALT_STRING));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bytes32 implSalt = keccak256("ZetaConnectorNonNative");
bytes32 proxySalt = keccak256("ZetaConnectorNonNativeProxy");
string constant IMPL_SALT_STRING = "ZetaConnectorNonNative";
string constant PROXY_SALT_STRING = "ZetaConnectorNonNativeProxy";
bytes32 implSalt = keccak256(abi.encodePacked(IMPL_SALT_STRING));
bytes32 proxySalt = keccak256(abi.encodePacked(PROXY_SALT_STRING));


vm.startBroadcast();

ZetaConnectorNonNative connector = new ZetaConnectorNonNative{salt: salt}(gateway, zeta, tss, admin);
require(address(connector) != address(0), "deployment failed");
expectedImplAddress = vm.computeCreate2Address(
implSalt,
hashInitCode(type(ZetaConnectorNonNative).creationCode)
);

ZetaConnectorNonNative connectorImpl = new ZetaConnectorNonNative{salt: implSalt}();
require(address(connectorImpl) != address(0), "connectorImpl deployment failed");

require(expectedImplAddress == address(connectorImpl), "impl address doesn't match expected address");

address expectedAddr = vm.computeCreate2Address(
salt,
hashInitCode(type(ZetaConnectorNonNative).creationCode, abi.encode(gateway, zeta, tss, admin))
expectedProxyAddress = vm.computeCreate2Address(
proxySalt,
hashInitCode(
type(ERC1967Proxy).creationCode,
abi.encode(
address(connectorImpl),
abi.encodeWithSelector(ZetaConnectorNonNative.initialize.selector, gateway, zeta, tss, admin)
)
)
);

require(expectedAddr == address(connector), "zeta connector non native address doesn't match expected address");
ERC1967Proxy connectorProxy = new ERC1967Proxy{salt: proxySalt}(
address(connectorImpl),
abi.encodeWithSelector(ZetaConnectorNonNative.initialize.selector, gateway, zeta, tss, admin)
);
require(address(connectorProxy) != address(0), "connectorProxy deployment failed");

require(expectedProxyAddress == address(connectorProxy), "proxy address doesn't match expected address");

ZetaConnectorNonNative connector = ZetaConnectorNonNative(address(connectorProxy));
require(connector.tssAddress() == tss, "tss not set");
require(address(connector.gateway()) == gateway, "gateway not set");
require(address(connector.zetaToken()) == zeta, "zeta not set");

vm.stopBroadcast();
}
Expand Down
2 changes: 1 addition & 1 deletion v2/scripts/deploy/deterministic/UpgradeGatewayEVM.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity 0.8.26;

import "forge-std/Script.sol";
import "src/evm/GatewayEVM.sol";
import "contracts/evm/GatewayEVM.sol";
import "test/utils/GatewayEVMUpgradeTest.sol";
import "test/utils/TestERC20.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
Expand Down
Loading
Loading