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

feat: enable multiple pausers and unpausers #388

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
forge script scripts/deployments/DeployProxiedICS20Transfer.sol -vvv --broadcast --private-key ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
forge script scripts/deployments/DeploySP1ICS07Tendermint.sol -vvv --broadcast --private-key ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
forge script scripts/deployments/PauseTransfers.sol -vvv --broadcast --private-key ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
forge script scripts/deployments/UnpauseTransfers.sol -vvv --broadcast --private-key ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
forge script scripts/deployments/UnpauseTransfers.sol -vvv --broadcast --private-key 59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d
- name: Show final result
run: |
cat deployments/local/31337.json
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ The IBC contracts use `AccessControl` to manage roles and permissions and allow

| **Role Name** | **Contract** | **Default** | **Description** |
|:---:|:---:|:---:|:---:|
| `PAUSER_ROLE` | `ICS20Transfer.sol` | Set at initialization. | Can pause/unpause the contract. |
| `PAUSER_ROLE` | `ICS20Transfer.sol` | Set at initialization. | Can pause the contract. |
| `UNPAUSER_ROLE` | `ICS20Transfer.sol` | Set at initialization. | Can unpause the contract. |
| `RATE_LIMITER_ROLE` | `Escrow.sol` | `address(0)` | Can set withdrawal rate limits per `ERC20` token. |
| `LIGHT_CLIENT_MIGRATOR_ROLE_{client_id}` | `ICS26Router.sol` | Creator of the light client. | Can migrate the light client identified by `client_id`. |

Expand Down
50 changes: 47 additions & 3 deletions abi/ICS20Transfer.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,19 @@
],
"stateMutability": "view"
},
{
"type": "function",
"name": "UNPAUSER_ROLE",
"inputs": [],
"outputs": [
{
"name": "",
"type": "bytes32",
"internalType": "bytes32"
}
],
"stateMutability": "view"
},
{
"type": "function",
"name": "UPGRADE_INTERFACE_VERSION",
Expand Down Expand Up @@ -177,6 +190,19 @@
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "grantUnpauserRole",
"inputs": [
{
"name": "account",
"type": "address",
"internalType": "address"
}
],
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "hasRole",
Expand Down Expand Up @@ -272,9 +298,14 @@
"internalType": "address"
},
{
"name": "pauser",
"type": "address",
"internalType": "address"
"name": "pausers",
"type": "address[]",
"internalType": "address[]"
},
{
"name": "unpausers",
"type": "address[]",
"internalType": "address[]"
},
{
"name": "permit2",
Expand Down Expand Up @@ -611,6 +642,19 @@
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "revokeUnpauserRole",
"inputs": [
{
"name": "account",
"type": "address",
"internalType": "address"
}
],
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "sendTransfer",
Expand Down
99 changes: 86 additions & 13 deletions abigen/ics20transfer/contract.go

Large diffs are not rendered by default.

12 changes: 9 additions & 3 deletions contracts/ICS20Transfer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,23 @@ contract ICS20Transfer is
/// @param ics26Router The ICS26Router contract address
/// @param escrowLogic Is the address of the Escrow logic contract
/// @param ibcERC20Logic Is the address of the IBCERC20 logic contract
/// @param pauser The address that can pause and unpause the contract
/// @param pausers The addresses that can pause the contract
/// @param unpausers The addresses that can unpause the contract
/// @inheritdoc IICS20Transfer
function initialize(
address ics26Router,
address escrowLogic,
address ibcERC20Logic,
address pauser,
address[] memory pausers,
address[] memory unpausers,
address permit2
)
public
initializer
{
__ReentrancyGuardTransient_init();
__Multicall_init();
__IBCPausable_init(pauser);
__IBCPausable_init(pausers, unpausers);

ICS20TransferStorage storage $ = _getICS20TransferStorage();

Expand Down Expand Up @@ -484,6 +486,10 @@ contract ICS20Transfer is
function _authorizeSetPauser(address) internal view override onlyAdmin { }
// solhint-disable-previous-line no-empty-blocks

/// @inheritdoc IBCPausableUpgradeable
function _authorizeSetUnpauser(address) internal view override onlyAdmin { }
// solhint-disable-previous-line no-empty-blocks

/// @inheritdoc IICS20Transfer
function upgradeEscrowTo(address newEscrowLogic) external onlyAdmin {
_getICS20TransferStorage()._escrowBeacon.upgradeTo(newEscrowLogic);
Expand Down
15 changes: 14 additions & 1 deletion contracts/interfaces/IIBCPausable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ interface IIBCPausable {
/// @notice The role identifier for the pauser role
function PAUSER_ROLE() external view returns (bytes32);

/// @notice The role identifier for the unpauser role
function UNPAUSER_ROLE() external view returns (bytes32);

/// @notice Pauses the contract
/// @dev The caller must have the pauser role
function pause() external;

/// @notice Unpauses the contract
/// @dev The caller must have the pauser role
/// @dev The caller must have the unpauser role
function unpause() external;

/// @notice Grants the pauser role to an account
Expand All @@ -22,4 +25,14 @@ interface IIBCPausable {
/// @dev The caller must be authorized by the derived contract
/// @param account The account to revoke the role from
function revokePauserRole(address account) external;

/// @notice Grants the unpauser role to an account
/// @dev The caller must be authorized by the derived contract
/// @param account The account to grant the role to
function grantUnpauserRole(address account) external;

/// @notice Revokes the unpauser role from an account
/// @dev The caller must be authorized by the derived contract
/// @param account The account to revoke the role from
function revokeUnpauserRole(address account) external;
}
6 changes: 4 additions & 2 deletions contracts/interfaces/IICS20Transfer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,15 @@ interface IICS20Transfer {
/// @param ics26Router The ICS26Router contract address
/// @param escrowLogic The address of the Escrow logic contract
/// @param ibcERC20Logic The address of the IBCERC20 logic contract
/// @param pauser The address that can pause and unpause the contract
/// @param pausers The addresses that can pause the contract
/// @param unpausers The addresses that can unpause the contract
/// @param permit2 The address of the permit2 contract
function initialize(
address ics26Router,
address escrowLogic,
address ibcERC20Logic,
address pauser,
address[] memory pausers,
address[] memory unpausers,
address permit2
)
external;
Expand Down
36 changes: 30 additions & 6 deletions contracts/utils/IBCPausableUpgradeable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,22 @@ abstract contract IBCPausableUpgradeable is
{
/// @inheritdoc IIBCPausable
bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
/// @inheritdoc IIBCPausable
bytes32 public constant UNPAUSER_ROLE = keccak256("UNPAUSER_ROLE");

/// @dev Initializes the contract in unpaused state.
/// @param pauser The address that can pause and unpause the contract
function __IBCPausable_init(address pauser) internal onlyInitializing {
/// @param pausers The addresses that are granted the `PAUSER_ROLE`
/// @param unpausers The addresses that are granted the `UNPAUSER_ROLE`
function __IBCPausable_init(address[] memory pausers, address[] memory unpausers) internal onlyInitializing {
__Pausable_init();
__AccessControl_init();

if (pauser != address(0)) {
_grantRole(PAUSER_ROLE, pauser);
for (uint256 i = 0; i < pausers.length; i++) {
_grantRole(PAUSER_ROLE, pausers[i]);
}

for (uint256 i = 0; i < unpausers.length; i++) {
_grantRole(UNPAUSER_ROLE, unpausers[i]);
}
}

Expand All @@ -34,7 +41,7 @@ abstract contract IBCPausableUpgradeable is
}

/// @inheritdoc IIBCPausable
function unpause() external onlyRole(PAUSER_ROLE) {
function unpause() external onlyRole(UNPAUSER_ROLE) {
_unpause();
}

Expand All @@ -50,8 +57,25 @@ abstract contract IBCPausableUpgradeable is
_revokeRole(PAUSER_ROLE, account);
}

/// @inheritdoc IIBCPausable
function grantUnpauserRole(address account) external {
_authorizeSetUnpauser(account);
_grantRole(UNPAUSER_ROLE, account);
}

/// @inheritdoc IIBCPausable
function revokeUnpauserRole(address account) external {
_authorizeSetUnpauser(account);
_revokeRole(UNPAUSER_ROLE, account);
}

/// @notice Authorizes the setting of a new pauser
/// @param pauser The new address that can pause and unpause the contract
/// @param pauser The new address that can pause the contract
/// @dev This function must be overridden to add authorization logic
function _authorizeSetPauser(address pauser) internal virtual;

/// @notice Authorizes the setting of a new unpauser
/// @param unpauser The new address that can unpause the contract
/// @dev This function must be overridden to add authorization logic
function _authorizeSetUnpauser(address unpauser) internal virtual;
}
4 changes: 2 additions & 2 deletions deployments/devnet/11155111.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
"ibcERC20Implementation": "0x50B244F8F3cAC4444f44cfD8B4ddc1805823E017",
"ics26Router": "0x15cB0fC94d072B367a1A2D7f0c8fF9792aB9f546",
"implementation": "0x23A45521Dda9ed9C47F4C8cf44282544FdAdF6D9",
"pauser": "0x0000000000000000000000000000000000000000",
"unpauser": "0x0000000000000000000000000000000000000000",
"pausers": [],
"unpausers": [],
"permit2": "0x000000000022D473030F116dDEE9F6B43aC78BA3",
"proxy": "0xbb87C1ACc6306ad2233a4c7BBE75a1230409b358"
},
Expand Down
3 changes: 2 additions & 1 deletion deployments/local/31337.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"ibcERC20Implementation": "0x0000000000000000000000000000000000000000",
"ics26Router": "0x0000000000000000000000000000000000000000",
"implementation": "0x0000000000000000000000000000000000000000",
"pauser": "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266",
"pausers": ["0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266"],
"unpausers": ["0x70997970C51812dc3A010C7d01b50e0d17dc79C8"],
"permit2": "0x0000000000000000000000000000000000000000",
"proxy": "0x0000000000000000000000000000000000000000"
},
Expand Down
4 changes: 2 additions & 2 deletions deployments/testnet-staging/11155111.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
"ibcERC20Implementation": "0xbAfa1093C913d92992634a6c9b148a3229191927",
"ics26Router": "0x718AbdD2f29A6aC1a34A3e20Dae378B5d3d2B0E9",
"implementation": "0xa453828f345b85520444C6B4b23c8bA9F6A46fe6",
"pauser": "0xAe3E5CCaF3216de61090E68Cf5a191f3b75CaAd3",
"unpauser": "0x0000000000000000000000000000000000000000",
"pausers": ["0xAe3E5CCaF3216de61090E68Cf5a191f3b75CaAd3"],
"unpausers": [],
"permit2": "0x000000000022D473030F116dDEE9F6B43aC78BA3",
"proxy": "0xE80DC519EE86146057B9dBEfBa900Edd7a2385e4"
},
Expand Down
3 changes: 2 additions & 1 deletion scripts/E2ETestDeploy.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ contract E2ETestDeploy is Script, IICS07TendermintMsgs, DeploySP1ICS07Tendermint
ics26Router: address(routerProxy),
escrowImplementation: escrowLogic,
ibcERC20Implementation: ibcERC20Logic,
pauser: address(0),
pausers: new address[](0),
unpausers: new address[](0),
permit2: address(0)
});

Expand Down
37 changes: 28 additions & 9 deletions scripts/deployments/DeployProxiedICS20Transfer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ abstract contract DeployProxiedICS20Transfer is Deployments {
deployment.ics26Router,
deployment.escrowImplementation,
deployment.ibcERC20Implementation,
deployment.pauser,
deployment.pausers,
deployment.unpausers,
deployment.permit2
)
);
Expand Down Expand Up @@ -85,13 +86,30 @@ contract DeployProxiedICS20TransferScript is DeployProxiedICS20Transfer, Script
"transfer app address doesn't match with the one in ics26Router"
);

if (deployment.pauser != address(0)) {
IBCPausableUpgradeable ipu = IBCPausableUpgradeable(address(transferProxy));
if (deployment.pausers.length != 0) {
for (uint32 i = 0; i < deployment.pausers.length; i++) {
address pauser = deployment.pausers[i];

vm.assertTrue(
ipu.hasRole(ipu.PAUSER_ROLE(), deployment.pauser),
"pauser address doesn't have pauser role"
);
IBCPausableUpgradeable ipu = IBCPausableUpgradeable(address(transferProxy));

vm.assertTrue(
ipu.hasRole(ipu.PAUSER_ROLE(), pauser),
string.concat("pauser address (", Strings.toHexString(pauser), ") doesn't have pauser role")
);
}
}

if (deployment.unpausers.length != 0) {
for (uint32 i = 0; i < deployment.unpausers.length; i++) {
address unpauser = deployment.unpausers[i];

IBCPausableUpgradeable ipu = IBCPausableUpgradeable(address(transferProxy));

vm.assertTrue(
ipu.hasRole(ipu.UNPAUSER_ROLE(), unpauser),
string.concat("unpauser address (", Strings.toHexString(unpauser), ") doesn't have unpauser role")
);
}
}
}

Expand Down Expand Up @@ -142,12 +160,13 @@ contract DeployProxiedICS20TransferScript is DeployProxiedICS20Transfer, Script
vm.serializeAddress("ics20Transfer", "implementation", deployment.implementation);
vm.serializeAddress("ics20Transfer", "escrowImplementation", deployment.escrowImplementation);
vm.serializeAddress("ics20Transfer", "ibcERC20Implementation", deployment.ibcERC20Implementation);
vm.serializeAddress("ics20Transfer", "pauser", deployment.pauser);
vm.serializeAddress("ics20Transfer", "pausers", deployment.pausers);
vm.serializeAddress("ics20Transfer", "unpausers", deployment.unpausers);
vm.serializeAddress("ics20Transfer", "ics26Router", deployment.ics26Router);
string memory output = vm.serializeAddress("ics20Transfer", "permit2", deployment.permit2);

vm.writeJson(output, path, ".ics20Transfer");

return address(transferProxy);
}
}
}
6 changes: 4 additions & 2 deletions scripts/helpers/Deployments.sol
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ abstract contract Deployments {
address ibcERC20Implementation;

// admin control
address pauser;
address[] pausers;
address[] unpausers;
address permit2;
address proxy;
}
Expand All @@ -121,7 +122,8 @@ abstract contract Deployments {
ibcERC20Implementation: vm.parseJsonAddress(json, ".ics20Transfer.ibcERC20Implementation"),
ics26Router: vm.parseJsonAddress(json, ".ics20Transfer.ics26Router"),
implementation: vm.parseJsonAddress(json, ".ics20Transfer.implementation"),
pauser: vm.parseJsonAddress(json, ".ics20Transfer.pauser"),
pausers: vm.parseJsonAddressArray(json, ".ics20Transfer.pausers"),
unpausers: vm.parseJsonAddressArray(json, ".ics20Transfer.unpausers"),
permit2: vm.parseJsonAddress(json, ".ics20Transfer.permit2"),
proxy: vm.parseJsonAddress(json, ".ics20Transfer.proxy")
});
Expand Down
3 changes: 2 additions & 1 deletion test/solidity-ibc/FixtureTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ abstract contract FixtureTest is Test, IICS07TendermintMsgs {
ERC1967Proxy transferProxy = new ERC1967Proxy(
address(ics20TransferLogic),
abi.encodeCall(
ICS20Transfer.initialize, (address(routerProxy), escrowLogic, ibcERC20Logic, address(0), address(0))
ICS20Transfer.initialize,
(address(routerProxy), escrowLogic, ibcERC20Logic, new address[](0), new address[](0), address(0))
)
);

Expand Down
Loading
Loading