Skip to content

Commit 1fc0a0a

Browse files
janjakubnanistajanjakubnanista
committed
chore: Use bytes32 for ProtocolVersion externally
1 parent db67941 commit 1fc0a0a

File tree

2 files changed

+58
-48
lines changed

2 files changed

+58
-48
lines changed

‎packages/contracts-bedrock/scripts/deploy/DeploySuperchain2.s.sol

Lines changed: 46 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,21 @@ import { Solarray } from "scripts/libraries/Solarray.sol";
1717
// since they are set to the initial proxy admin owner.
1818
contract DeploySuperchain2 is Script {
1919
struct Input {
20+
// Role inputs.
21+
address guardian;
22+
address protocolVersionsOwner;
23+
address superchainProxyAdminOwner;
24+
// Other inputs.
25+
bool paused;
26+
bytes32 recommendedProtocolVersion;
27+
bytes32 requiredProtocolVersion;
28+
}
29+
30+
/// @notice InternalInput is created based on Input by converting the bytes32 protocol versions to ProtocolVersion types
31+
//
32+
// ProtocolVersion type is based on uint256 which conflicts with downstream types (like e.g. ProtocolVersion from op-geth)
33+
// so to keep the ABI externally compatible, we expose it simply as bytes32
34+
struct InternalInput {
2035
// Role inputs.
2136
address guardian;
2237
address protocolVersionsOwner;
@@ -40,7 +55,9 @@ contract DeploySuperchain2 is Script {
4055
// -------- Core Deployment Methods --------
4156

4257
function run(Input memory _input) public returns (Output memory output_) {
43-
assertValidInput(_input);
58+
InternalInput memory internalInput = toInternalInput(_input);
59+
60+
assertValidInput(internalInput);
4461

4562
// Notice that we do not do any explicit verification here that inputs are set. This is because
4663
// the verification happens elsewhere:
@@ -52,23 +69,23 @@ contract DeploySuperchain2 is Script {
5269
// additional verification logic.
5370

5471
// Deploy the proxy admin, with the owner set to the deployer.
55-
deploySuperchainProxyAdmin(_input, output_);
72+
deploySuperchainProxyAdmin(internalInput, output_);
5673

5774
// Deploy and initialize the superchain contracts.
58-
deploySuperchainImplementationContracts(_input, output_);
59-
deployAndInitializeSuperchainConfig(_input, output_);
60-
deployAndInitializeProtocolVersions(_input, output_);
75+
deploySuperchainImplementationContracts(internalInput, output_);
76+
deployAndInitializeSuperchainConfig(internalInput, output_);
77+
deployAndInitializeProtocolVersions(internalInput, output_);
6178

6279
// Transfer ownership of the ProxyAdmin from the deployer to the specified owner.
63-
transferProxyAdminOwnership(_input, output_);
80+
transferProxyAdminOwnership(internalInput, output_);
6481

6582
// Output assertions, to make sure outputs were assigned correctly.
66-
assertValidOutput(_input, output_);
83+
assertValidOutput(internalInput, output_);
6784
}
6885

6986
// -------- Deployment Steps --------
7087

71-
function deploySuperchainProxyAdmin(Input memory, Output memory _output) internal {
88+
function deploySuperchainProxyAdmin(InternalInput memory, Output memory _output) private {
7289
// Deploy the proxy admin, with the owner set to the deployer.
7390
// We explicitly specify the deployer as `msg.sender` because for testing we deploy this script from a test
7491
// contract. If we provide no argument, the foundry default sender would be the broadcaster during test, but the
@@ -85,7 +102,7 @@ contract DeploySuperchain2 is Script {
85102
_output.superchainProxyAdmin = superchainProxyAdmin;
86103
}
87104

88-
function deploySuperchainImplementationContracts(Input memory, Output memory _output) internal {
105+
function deploySuperchainImplementationContracts(InternalInput memory, Output memory _output) private {
89106
// Deploy implementation contracts.
90107
ISuperchainConfig superchainConfigImpl = ISuperchainConfig(
91108
DeployUtils.createDeterministic({
@@ -109,9 +126,7 @@ contract DeploySuperchain2 is Script {
109126
_output.protocolVersionsImpl = protocolVersionsImpl;
110127
}
111128

112-
function deployAndInitializeSuperchainConfig(Input memory _input, Output memory _output) internal {
113-
assertValidGuardianInput(_input);
114-
129+
function deployAndInitializeSuperchainConfig(InternalInput memory _input, Output memory _output) private {
115130
address guardian = _input.guardian;
116131
bool paused = _input.paused;
117132

@@ -138,9 +153,7 @@ contract DeploySuperchain2 is Script {
138153
_output.superchainConfigProxy = superchainConfigProxy;
139154
}
140155

141-
function deployAndInitializeProtocolVersions(Input memory _input, Output memory _output) internal {
142-
assertValidProtocolInput(_input);
143-
156+
function deployAndInitializeProtocolVersions(InternalInput memory _input, Output memory _output) private {
144157
address protocolVersionsOwner = _input.protocolVersionsOwner;
145158
ProtocolVersion requiredProtocolVersion = _input.requiredProtocolVersion;
146159
ProtocolVersion recommendedProtocolVersion = _input.recommendedProtocolVersion;
@@ -171,9 +184,7 @@ contract DeploySuperchain2 is Script {
171184
_output.protocolVersionsProxy = protocolVersionsProxy;
172185
}
173186

174-
function transferProxyAdminOwnership(Input memory _input, Output memory _output) internal {
175-
assertValidProxyInput(_input);
176-
187+
function transferProxyAdminOwnership(InternalInput memory _input, Output memory _output) private {
177188
address superchainProxyAdminOwner = _input.superchainProxyAdminOwner;
178189

179190
IProxyAdmin superchainProxyAdmin = _output.superchainProxyAdmin;
@@ -183,17 +194,8 @@ contract DeploySuperchain2 is Script {
183194
superchainProxyAdmin.transferOwnership(superchainProxyAdminOwner);
184195
}
185196

186-
function assertValidInput(Input memory _input) internal pure {
187-
assertValidGuardianInput(_input);
188-
assertValidProxyInput(_input);
189-
assertValidProtocolInput(_input);
190-
}
191-
192-
function assertValidGuardianInput(Input memory _input) internal pure {
197+
function assertValidInput(InternalInput memory _input) internal pure {
193198
require(_input.guardian != address(0), "DeploySuperchain: guardian not set");
194-
}
195-
196-
function assertValidProtocolInput(Input memory _input) internal pure {
197199
require(_input.protocolVersionsOwner != address(0), "DeploySuperchain: protocolVersionsOwner not set");
198200
require(
199201
ProtocolVersion.unwrap(_input.requiredProtocolVersion) != 0,
@@ -203,20 +205,17 @@ contract DeploySuperchain2 is Script {
203205
ProtocolVersion.unwrap(_input.recommendedProtocolVersion) != 0,
204206
"DeploySuperchain: recommendedProtocolVersion not set"
205207
);
206-
}
207-
208-
function assertValidProxyInput(Input memory _input) internal pure {
209208
require(_input.superchainProxyAdminOwner != address(0), "DeploySuperchain: superchainProxyAdminOwner not set");
210209
}
211210

212-
function assertValidOutput(Input memory _input, Output memory _output) public {
211+
function assertValidOutput(InternalInput memory _input, Output memory _output) public {
213212
assertValidContractAddresses(_input, _output);
214213
assertValidSuperchainProxyAdmin(_input, _output);
215214
assertValidSuperchainConfig(_input, _output);
216215
assertValidProtocolVersions(_input, _output);
217216
}
218217

219-
function assertValidContractAddresses(Input memory, Output memory _output) internal {
218+
function assertValidContractAddresses(InternalInput memory, Output memory _output) internal {
220219
address[] memory addrs = Solarray.addresses(
221220
address(_output.superchainProxyAdmin),
222221
address(_output.superchainConfigImpl),
@@ -238,11 +237,11 @@ contract DeploySuperchain2 is Script {
238237
// sol-style-malformed-require
239238
}
240239

241-
function assertValidSuperchainProxyAdmin(Input memory _input, Output memory _output) internal view {
240+
function assertValidSuperchainProxyAdmin(InternalInput memory _input, Output memory _output) internal view {
242241
require(_output.superchainProxyAdmin.owner() == _input.superchainProxyAdminOwner, "SPA-10");
243242
}
244243

245-
function assertValidSuperchainConfig(Input memory _input, Output memory _output) internal {
244+
function assertValidSuperchainConfig(InternalInput memory _input, Output memory _output) internal {
246245
// Proxy checks.
247246
ISuperchainConfig superchainConfig = _output.superchainConfigProxy;
248247
DeployUtils.assertInitialized({
@@ -270,7 +269,7 @@ contract DeploySuperchain2 is Script {
270269
require(superchainConfig.paused() == false, "SUPCON-60");
271270
}
272271

273-
function assertValidProtocolVersions(Input memory _input, Output memory _output) internal {
272+
function assertValidProtocolVersions(InternalInput memory _input, Output memory _output) internal {
274273
// Proxy checks.
275274
IProtocolVersions pv = _output.protocolVersionsProxy;
276275
DeployUtils.assertInitialized({ _contractAddress: address(pv), _isProxy: true, _slot: 0, _offset: 0 });
@@ -294,4 +293,15 @@ contract DeploySuperchain2 is Script {
294293
require(ProtocolVersion.unwrap(pv.required()) == 0, "PV-70");
295294
require(ProtocolVersion.unwrap(pv.recommended()) == 0, "PV-80");
296295
}
296+
297+
function toInternalInput(Input memory _input) internal pure returns (InternalInput memory input_) {
298+
input_ = InternalInput(
299+
_input.guardian,
300+
_input.protocolVersionsOwner,
301+
_input.superchainProxyAdminOwner,
302+
_input.paused,
303+
ProtocolVersion.wrap(uint256(_input.recommendedProtocolVersion)),
304+
ProtocolVersion.wrap(uint256(_input.requiredProtocolVersion))
305+
);
306+
}
297307
}

‎packages/contracts-bedrock/test/opcm/DeploySuperchain2.t.sol

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ contract DeploySuperchain2_Test is Test {
1515
address defaultProtocolVersionsOwner = makeAddr("defaultProtocolVersionsOwner");
1616
address defaultGuardian = makeAddr("defaultGuardian");
1717
bool defaultPaused = false;
18-
ProtocolVersion defaultRequiredProtocolVersion = ProtocolVersion.wrap(1);
19-
ProtocolVersion defaultRecommendedProtocolVersion = ProtocolVersion.wrap(2);
18+
bytes32 defaultRequiredProtocolVersion = bytes32(uint256(1));
19+
bytes32 defaultRecommendedProtocolVersion = bytes32(uint256(2));
2020

2121
function setUp() public {
2222
deploySuperchain = new DeploySuperchain2();
2323
}
2424

25-
function unwrap(ProtocolVersion _pv) internal pure returns (uint256) {
26-
return ProtocolVersion.unwrap(_pv);
25+
function unwrap(ProtocolVersion _pv) internal pure returns (bytes32) {
26+
return bytes32(ProtocolVersion.unwrap(_pv));
2727
}
2828

2929
function hash(bytes32 _seed, uint256 _i) internal pure returns (bytes32) {
@@ -35,16 +35,16 @@ contract DeploySuperchain2_Test is Test {
3535
address _protocolVersionsOwner,
3636
address _guardian,
3737
bool _paused,
38-
ProtocolVersion _recommendedProtocolVersion,
39-
ProtocolVersion _requiredProtocolVersion
38+
bytes32 _recommendedProtocolVersion,
39+
bytes32 _requiredProtocolVersion
4040
)
4141
public
4242
{
4343
vm.assume(_superchainProxyAdminOwner != address(0));
4444
vm.assume(_protocolVersionsOwner != address(0));
4545
vm.assume(_guardian != address(0));
46-
vm.assume(unwrap(_recommendedProtocolVersion) != 0);
47-
vm.assume(unwrap(_requiredProtocolVersion) != 0);
46+
vm.assume(_recommendedProtocolVersion != bytes32(0));
47+
vm.assume(_requiredProtocolVersion != bytes32(0));
4848

4949
DeploySuperchain2.Input memory dsi = DeploySuperchain2.Input(
5050
_guardian,
@@ -63,8 +63,8 @@ contract DeploySuperchain2_Test is Test {
6363
assertEq(address(dso.protocolVersionsProxy.owner()), _protocolVersionsOwner, "200");
6464
assertEq(address(dso.superchainConfigProxy.guardian()), _guardian, "300");
6565
assertEq(dso.superchainConfigProxy.paused(), _paused, "400");
66-
assertEq(unwrap(dso.protocolVersionsProxy.required()), unwrap(_requiredProtocolVersion), "500");
67-
assertEq(unwrap(dso.protocolVersionsProxy.recommended()), unwrap(_recommendedProtocolVersion), "600");
66+
assertEq(unwrap(dso.protocolVersionsProxy.required()), _requiredProtocolVersion, "500");
67+
assertEq(unwrap(dso.protocolVersionsProxy.recommended()), _recommendedProtocolVersion, "600");
6868

6969
// Architecture assertions.
7070
// We prank as the zero address due to the Proxy's `proxyCallIfNotAdmin` modifier.
@@ -98,12 +98,12 @@ contract DeploySuperchain2_Test is Test {
9898
deploySuperchain.run(input);
9999

100100
input = defaultInput();
101-
input.requiredProtocolVersion = ProtocolVersion.wrap(0);
101+
input.requiredProtocolVersion = bytes32(0);
102102
vm.expectRevert("DeploySuperchain: requiredProtocolVersion not set");
103103
deploySuperchain.run(input);
104104

105105
input = defaultInput();
106-
input.recommendedProtocolVersion = ProtocolVersion.wrap(0);
106+
input.recommendedProtocolVersion = bytes32(0);
107107
vm.expectRevert("DeploySuperchain: recommendedProtocolVersion not set");
108108
deploySuperchain.run(input);
109109
}

0 commit comments

Comments
 (0)