Skip to content

Commit 223a896

Browse files
committed
feat(core): batch set/add/remove on DomainRoutingIsm
Adds setBatch, add, addBatch and removeBatch external functions plus ModuleSet and ModuleRemoved events to DomainRoutingIsm. add/addBatch revert if a domain is already enrolled, enabling distinct role-based access control by function selector. setBatch/removeBatch let owners enroll or remove many domains in a single transaction. Re-opens the idea from #5219 against main.
1 parent f6d797b commit 223a896

4 files changed

Lines changed: 252 additions & 1 deletion

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@hyperlane-xyz/core': minor
3+
---
4+
5+
Added `setBatch`, `add`, `addBatch`, and `removeBatch` external functions to `DomainRoutingIsm`, plus `ModuleSet` and `ModuleRemoved` events. `add`/`addBatch` revert if a domain is already enrolled, enabling distinct role-based access control via function selectors. `setBatch`/`removeBatch` let owners enroll or remove many domains in a single transaction, which avoids the need to enroll one domain per transaction after routing ISM initialization.

solidity/contracts/isms/routing/DomainRoutingIsm.sol

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,19 @@ contract DomainRoutingIsm is
2929
using Address for address;
3030
using Strings for uint32;
3131

32+
// ============ Events ============
33+
event ModuleSet(uint32 indexed domain, address module);
34+
event ModuleRemoved(uint32 indexed domain);
35+
3236
// ============ Mutable Storage ============
3337
EnumerableMapExtended.UintToBytes32Map internal _modules;
3438

39+
// ============ Structs ============
40+
struct DomainModule {
41+
uint32 domain;
42+
IInterchainSecurityModule module;
43+
}
44+
3545
// ============ External Functions ============
3646

3747
/**
@@ -74,6 +84,43 @@ contract DomainRoutingIsm is
7484
_set(_domain, address(_module));
7585
}
7686

87+
/**
88+
* @notice Sets the ISMs to be used for the specified origin domains
89+
* @param _domainModules The origin domains and ISMs to enroll
90+
*/
91+
function setBatch(
92+
DomainModule[] calldata _domainModules
93+
) external onlyOwner {
94+
_set(_domainModules);
95+
}
96+
97+
/**
98+
* @notice Adds the specified origin domain
99+
* @dev Reverts if the domain already exists
100+
* @dev Behavior similar to set but useful for distinguishing by selector
101+
* @param _domain The origin domain
102+
* @param _module The ISM to use to verify messages
103+
*/
104+
function add(
105+
uint32 _domain,
106+
IInterchainSecurityModule _module
107+
) external onlyOwner {
108+
_add(_domain, address(_module));
109+
}
110+
111+
/**
112+
* @notice Adds the specified origin domains
113+
* @dev Reverts if any domain already exists
114+
* @param _domainModules The origin domains and ISMs to enroll
115+
*/
116+
function addBatch(
117+
DomainModule[] calldata _domainModules
118+
) external onlyOwner {
119+
for (uint256 i = 0; i < _domainModules.length; ++i) {
120+
_add(_domainModules[i].domain, address(_domainModules[i].module));
121+
}
122+
}
123+
77124
/**
78125
* @notice Removes the specified origin domain
79126
* @param _domain The origin domain
@@ -82,6 +129,16 @@ contract DomainRoutingIsm is
82129
_remove(_domain);
83130
}
84131

132+
/**
133+
* @notice Removes the specified origin domains
134+
* @param _domains The origin domains to remove
135+
*/
136+
function removeBatch(uint32[] calldata _domains) external onlyOwner {
137+
for (uint256 i = 0; i < _domains.length; ++i) {
138+
_remove(_domains[i]);
139+
}
140+
}
141+
85142
function domains() external view returns (uint256[] memory) {
86143
return _modules.keys();
87144
}
@@ -117,6 +174,7 @@ contract DomainRoutingIsm is
117174
*/
118175
function _remove(uint32 _domain) internal virtual {
119176
require(_modules.remove(_domain), _originNotFoundError(_domain));
177+
emit ModuleRemoved(_domain);
120178
}
121179

122180
function _originNotFoundError(
@@ -125,6 +183,17 @@ contract DomainRoutingIsm is
125183
return string.concat("No ISM found for origin: ", _origin.toString());
126184
}
127185

186+
function _set(DomainModule[] calldata _domainModules) internal {
187+
for (uint256 i = 0; i < _domainModules.length; ++i) {
188+
_set(_domainModules[i].domain, address(_domainModules[i].module));
189+
}
190+
}
191+
192+
function _add(uint32 _domain, address _module) internal {
193+
require(!_modules.contains(_domain), "Domain already exists");
194+
_set(_domain, _module);
195+
}
196+
128197
/**
129198
* @notice Sets the ISM to be used for the specified origin domain
130199
* @param _domain The origin domain
@@ -133,5 +202,6 @@ contract DomainRoutingIsm is
133202
function _set(uint32 _domain, address _module) internal virtual {
134203
require(_module.isContract(), "ISM must be a contract");
135204
_modules.set(_domain, _module.addressToBytes32());
205+
emit ModuleSet(_domain, _module);
136206
}
137207
}

solidity/test/isms/DomainRoutingIsm.t.sol

Lines changed: 169 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,130 @@ contract DomainRoutingIsmTest is Test {
2727
return new TestIsm(abi.encode(requiredMetadata));
2828
}
2929

30+
function deployTestIsms(
31+
uint256 count
32+
) internal returns (IInterchainSecurityModule[] memory isms) {
33+
isms = new IInterchainSecurityModule[](count);
34+
for (uint32 i = 0; i < count; ++i) {
35+
isms[i] = IInterchainSecurityModule(deployTestIsm(bytes32(0)));
36+
}
37+
}
38+
39+
function uniqueDomains(
40+
uint32 domain,
41+
uint256 count
42+
) internal pure returns (uint32[] memory) {
43+
uint32[] memory domains = new uint32[](count);
44+
for (uint32 i = 0; i < count; ++i) {
45+
unchecked {
46+
domains[i] = domain + i;
47+
}
48+
}
49+
return domains;
50+
}
51+
52+
function buildDomainModules(
53+
uint32[] memory domains,
54+
IInterchainSecurityModule[] memory modules
55+
) internal pure returns (DomainRoutingIsm.DomainModule[] memory) {
56+
DomainRoutingIsm.DomainModule[]
57+
memory domainModules = new DomainRoutingIsm.DomainModule[](
58+
domains.length
59+
);
60+
for (uint256 i = 0; i < domains.length; ++i) {
61+
domainModules[i] = DomainRoutingIsm.DomainModule({
62+
domain: domains[i],
63+
module: modules[i]
64+
});
65+
}
66+
return domainModules;
67+
}
68+
3069
function getMetadata(uint32 domain) internal view returns (bytes memory) {
3170
return TestIsm(address(ism.module(domain))).requiredMetadata();
3271
}
3372

3473
function testSet(uint32 domain) public {
74+
vm.expectRevert("ISM must be a contract");
75+
ism.set(domain, IInterchainSecurityModule(address(0x0)));
76+
3577
TestIsm _ism = deployTestIsm(bytes32(0));
78+
vm.expectEmit(true, false, false, true);
79+
emit DomainRoutingIsm.ModuleSet(domain, address(_ism));
3680
ism.set(domain, _ism);
3781
assertEq(address(ism.module(domain)), address(_ism));
3882
}
3983

84+
function testSetBatch(uint32 domain, uint8 count) public {
85+
vm.assume(count > 0);
86+
uint32[] memory domains = uniqueDomains(domain, count);
87+
IInterchainSecurityModule[] memory modules = deployTestIsms(count);
88+
DomainRoutingIsm.DomainModule[]
89+
memory domainModules = buildDomainModules(domains, modules);
90+
91+
ism.setBatch(domainModules);
92+
for (uint256 i = 0; i < count; ++i) {
93+
assertEq(address(ism.module(domains[i])), address(modules[i]));
94+
}
95+
}
96+
97+
function testAdd(uint32 domain) public virtual {
98+
vm.expectRevert("ISM must be a contract");
99+
ism.add(domain, IInterchainSecurityModule(address(0x0)));
100+
101+
TestIsm _ism = deployTestIsm(bytes32(0));
102+
vm.expectEmit(true, false, false, true);
103+
emit DomainRoutingIsm.ModuleSet(domain, address(_ism));
104+
ism.add(domain, _ism);
105+
assertEq(address(ism.module(domain)), address(_ism));
106+
107+
vm.expectRevert("Domain already exists");
108+
ism.add(domain, _ism);
109+
}
110+
111+
function testAddBatch(uint32 domain, uint8 count) public virtual {
112+
vm.assume(count > 0);
113+
uint32[] memory domains = uniqueDomains(domain, count);
114+
IInterchainSecurityModule[] memory modules = deployTestIsms(count);
115+
DomainRoutingIsm.DomainModule[]
116+
memory domainModules = buildDomainModules(domains, modules);
117+
118+
ism.addBatch(domainModules);
119+
for (uint256 i = 0; i < count; ++i) {
120+
assertEq(address(ism.module(domains[i])), address(modules[i]));
121+
}
122+
vm.expectRevert("Domain already exists");
123+
ism.addBatch(domainModules);
124+
}
125+
40126
function testRemove(uint32 domain) public virtual {
41127
vm.expectRevert();
42128
ism.remove(domain);
43129

44130
TestIsm _ism = deployTestIsm(bytes32(0));
45131
ism.set(domain, _ism);
132+
vm.expectEmit(true, false, false, false);
133+
emit DomainRoutingIsm.ModuleRemoved(domain);
46134
ism.remove(domain);
47135
}
48136

49-
function testSetManyViaFactory(uint8 count, uint32 domain) public {
137+
function testRemoveBatch(uint32 domain, uint8 count) public virtual {
138+
vm.assume(count > 0);
139+
uint32[] memory domains = uniqueDomains(domain, count);
140+
IInterchainSecurityModule[] memory modules = deployTestIsms(count);
141+
DomainRoutingIsm.DomainModule[]
142+
memory domainModules = buildDomainModules(domains, modules);
143+
144+
ism.addBatch(domainModules);
145+
ism.removeBatch(domains);
146+
for (uint256 i = 0; i < count; ++i) {
147+
vm.expectRevert();
148+
ism.module(domains[i]);
149+
}
150+
}
151+
152+
function testSetManyViaFactory(uint8 count, uint32 domain) public virtual {
153+
vm.assume(count > 0);
50154
vm.assume(domain > count);
51155
DomainRoutingIsmFactory factory = new DomainRoutingIsmFactory();
52156
uint32[] memory _domains = new uint32[](count);
@@ -60,6 +164,9 @@ contract DomainRoutingIsmTest is Test {
60164
for (uint256 i = 0; i < count; ++i) {
61165
assertEq(address(ism.module(_domains[i])), address(_isms[i]));
62166
}
167+
uint32[] memory shortDomains = new uint32[](count - 1);
168+
vm.expectRevert("length mismatch");
169+
factory.deploy(address(this), shortDomains, _isms);
63170
}
64171

65172
function testSetNonOwner(
@@ -71,6 +178,53 @@ contract DomainRoutingIsmTest is Test {
71178
ism.set(domain, _ism);
72179
}
73180

181+
function testSetBatchNonOwner(uint32 domain) public {
182+
DomainRoutingIsm.DomainModule[]
183+
memory domainModules = new DomainRoutingIsm.DomainModule[](1);
184+
domainModules[0] = DomainRoutingIsm.DomainModule({
185+
domain: domain,
186+
module: IInterchainSecurityModule(address(0))
187+
});
188+
vm.prank(NON_OWNER);
189+
vm.expectRevert("Ownable: caller is not the owner");
190+
ism.setBatch(domainModules);
191+
}
192+
193+
function testAddNonOwner(
194+
uint32 domain,
195+
IInterchainSecurityModule _ism
196+
) public {
197+
vm.prank(NON_OWNER);
198+
vm.expectRevert("Ownable: caller is not the owner");
199+
ism.add(domain, _ism);
200+
}
201+
202+
function testAddBatchNonOwner(uint32 domain) public {
203+
DomainRoutingIsm.DomainModule[]
204+
memory domainModules = new DomainRoutingIsm.DomainModule[](1);
205+
domainModules[0] = DomainRoutingIsm.DomainModule({
206+
domain: domain,
207+
module: IInterchainSecurityModule(address(0))
208+
});
209+
vm.prank(NON_OWNER);
210+
vm.expectRevert("Ownable: caller is not the owner");
211+
ism.addBatch(domainModules);
212+
}
213+
214+
function testRemoveNonOwner(uint32 domain) public {
215+
vm.prank(NON_OWNER);
216+
vm.expectRevert("Ownable: caller is not the owner");
217+
ism.remove(domain);
218+
}
219+
220+
function testRemoveBatchNonOwner(uint32 domain) public {
221+
uint32[] memory domains = new uint32[](1);
222+
domains[0] = domain;
223+
vm.prank(NON_OWNER);
224+
vm.expectRevert("Ownable: caller is not the owner");
225+
ism.removeBatch(domains);
226+
}
227+
74228
function testVerify(uint32 domain, bytes32 seed) public {
75229
ism.set(domain, deployTestIsm(seed));
76230

@@ -123,6 +277,20 @@ contract DefaultFallbackRoutingIsmTest is DomainRoutingIsmTest {
123277
new DefaultFallbackRoutingIsm(address(0));
124278
}
125279

280+
function testRemoveBatch(uint32 domain, uint8 count) public override {
281+
vm.assume(count > 0);
282+
uint32[] memory domains = uniqueDomains(domain, count);
283+
IInterchainSecurityModule[] memory modules = deployTestIsms(count);
284+
DomainRoutingIsm.DomainModule[]
285+
memory domainModules = buildDomainModules(domains, modules);
286+
287+
ism.addBatch(domainModules);
288+
ism.removeBatch(domains);
289+
for (uint256 i = 0; i < count; ++i) {
290+
assertEq(address(ism.module(domains[i])), address(defaultIsm));
291+
}
292+
}
293+
126294
function testVerifyNoIsm(uint32 domain, bytes32 seed) public override {
127295
vm.assume(domain > 0);
128296
ism.set(domain, deployTestIsm(seed));

solidity/test/isms/IncrementalDomainRoutingIsm.t.sol

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import "forge-std/Test.sol";
55

66
import {IncrementalDomainRoutingIsm} from "../../contracts/isms/routing/IncrementalDomainRoutingIsm.sol";
77
import {IncrementalDomainRoutingIsmFactory} from "../../contracts/isms/routing/IncrementalDomainRoutingIsmFactory.sol";
8+
import {DomainRoutingIsm} from "../../contracts/isms/routing/DomainRoutingIsm.sol";
89
import {DomainRoutingIsmTest} from "./DomainRoutingIsm.t.sol";
910
import {IInterchainSecurityModule} from "../../contracts/interfaces/IInterchainSecurityModule.sol";
1011
import {TestIsm} from "./IsmTestUtils.sol";
@@ -29,6 +30,13 @@ contract IncrementalDomainRoutingIsmTest is DomainRoutingIsmTest {
2930
ism.remove(domain);
3031
}
3132

33+
function testRemoveBatch(uint32 domain, uint8 count) public override {
34+
vm.assume(count > 0);
35+
uint32[] memory domains = uniqueDomains(domain, count);
36+
vm.expectRevert("IncrementalDomainRoutingIsm: removal not supported");
37+
ism.removeBatch(domains);
38+
}
39+
3240
function testSetTwiceReverts(uint32 domain) public {
3341
TestIsm _ism = deployTestIsm(bytes32(0));
3442
TestIsm _ism2 = deployTestIsm(bytes32(uint256(1)));

0 commit comments

Comments
 (0)