Skip to content

Commit fefba0f

Browse files
committed
chore(withdrawals): move withdrawals feature to extensions directory
1 parent 6a0e9d3 commit fefba0f

16 files changed

+190
-116
lines changed

contracts/src/chains/OPStackKeystoreWithdrawable.sol

Lines changed: 0 additions & 93 deletions
This file was deleted.

contracts/src/Keystore.sol renamed to contracts/src/core/Keystore.sol

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ abstract contract Keystore {
116116
// CONSTRUCTOR //
117117
////////////////////////////////////////////////////////////////////////////////////////////////////
118118

119-
/// @notice Creates the Keystore.
119+
/// @notice Constructor.
120120
///
121121
/// @param masterChainId_ The master chain id.
122122
constructor(uint256 masterChainId_) {
@@ -196,7 +196,7 @@ abstract contract Keystore {
196196
// confirmation flow.
197197
if (isSet) {
198198
// Ensure the `newConfirmedConfig` matches with the extracted `newConfirmedConfigHash`.
199-
ConfigLib.verify({configHash: newConfirmedConfigHash, config: newConfirmedConfig});
199+
ConfigLib.verify({config: newConfirmedConfig, account: address(this), configHash: newConfirmedConfigHash});
200200

201201
_applyNewConfirmedConfig({
202202
newConfirmedConfigHash: newConfirmedConfigHash,
@@ -324,7 +324,7 @@ abstract contract Keystore {
324324
require(config.nonce == 0, InitialNonceIsNotZero());
325325

326326
// Initialize the internal Keystore storage.
327-
bytes32 configHash = ConfigLib.hash(config);
327+
bytes32 configHash = ConfigLib.hash({config: config, account: address(this)});
328328
if (block.chainid == masterChainId) {
329329
require(_sMaster().configHash == 0, KeystoreAlreadyInitialized());
330330
_sMaster().configHash = configHash;
@@ -394,7 +394,7 @@ abstract contract Keystore {
394394
///
395395
/// @return The new config hash.
396396
function _applyMasterConfig(ConfigLib.Config calldata newConfig) private returns (bytes32) {
397-
bytes32 newConfigHash = ConfigLib.hash(newConfig);
397+
bytes32 newConfigHash = ConfigLib.hash({config: newConfig, account: address(this)});
398398
_sMaster().configHash = newConfigHash;
399399
_sMaster().configNonce = newConfig.nonce;
400400

@@ -407,7 +407,7 @@ abstract contract Keystore {
407407
///
408408
/// @return The new config hash.
409409
function _applyReplicaConfig(ConfigLib.Config calldata newConfig) private returns (bytes32) {
410-
bytes32 newConfigHash = ConfigLib.hash(newConfig);
410+
bytes32 newConfigHash = ConfigLib.hash({config: newConfig, account: address(this)});
411411
_setPreconfirmedConfig({preconfirmedConfigHash: newConfigHash, preconfirmedConfigNonce: newConfig.nonce});
412412

413413
return newConfigHash;
File renamed without changes.

contracts/src/libs/ConfigLib.sol renamed to contracts/src/core/libs/ConfigLib.sol

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,29 @@ library ConfigLib {
3434
///
3535
/// @dev Reverts if the parameters hashes do not match.
3636
///
37-
/// @param configHash The Keystore config hash.
3837
/// @param config The Keystore config.
39-
function verify(bytes32 configHash, Config calldata config) internal view {
38+
/// @param account The account address.
39+
/// @param configHash The Keystore config hash.
40+
function verify(Config calldata config, address account, bytes32 configHash) internal pure {
4041
// Ensure the recomputed config hash matches witht the given `configHash` parameter.
41-
bytes32 recomputedConfigHash = hash(config);
42+
bytes32 recomputedConfigHash = hash({config: config, account: account});
4243

4344
require(
4445
recomputedConfigHash == configHash,
4546
InvalidConfig({configHash: configHash, recomputedConfigHash: recomputedConfigHash})
4647
);
4748
}
4849

49-
/// @notice Computed the hash of the provided `config`.
50+
/// @notice Computes the hash of the provided `config`.
5051
///
51-
/// @dev To avoid replay of similar configs on different wallets with the same signers, the wallet address is also
52-
/// part of the hashed data.
52+
/// @dev To avoid replay of similar config signatures on different wallets with the same signers, the account
53+
/// address is also hashed with the config.
5354
///
5455
/// @param config The Keystore config.
56+
/// @param account The account address.
5557
///
5658
/// @return The corresponding config hash.
57-
function hash(Config calldata config) internal view returns (bytes32) {
58-
return keccak256(abi.encodePacked(address(this), config.nonce, config.data));
59+
function hash(Config calldata config, address account) internal pure returns (bytes32) {
60+
return keccak256(abi.encodePacked(account, config.nonce, config.data));
5961
}
6062
}

contracts/src/examples/MultiOwnableWallet.opstack.sol

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@ import {UserOperation} from "aa/interfaces/UserOperation.sol";
66
import {Receiver} from "solady/accounts/Receiver.sol";
77
import {SignatureCheckerLib} from "solady/utils/SignatureCheckerLib.sol";
88

9-
import {OPStackKeystore} from "../chains/OPStackKeystore.sol";
10-
11-
import {Keystore} from "../Keystore.sol";
12-
import {ConfigLib} from "../KeystoreLibs.sol";
9+
import {Keystore} from "../core/Keystore.sol";
10+
import {ConfigLib} from "../core/KeystoreLibs.sol";
11+
import {OPStackKeystore} from "../core/chains/OPStackKeystore.sol";
1312

1413
import {ERC1271} from "./ERC1271.sol";
1514
import {TransientUUPSUpgradeable} from "./TransientUUPSUpgradeable.sol";
@@ -210,7 +209,7 @@ contract MultiOwnableWallet is OPStackKeystore, ERC1271, TransientUUPSUpgradeabl
210209
{
211210
// NOTE: Because this hook is limited to a view function, no special access control logic is required.
212211

213-
bytes32 newConfigHash = ConfigLib.hash(newConfig);
212+
bytes32 newConfigHash = ConfigLib.hash({config: newConfig, account: address(this)});
214213
(, bytes memory signatureUpdate) = abi.decode(validationProof, (bytes, bytes));
215214
(uint256 sigUpdateSignerIndex, bytes memory signature) = abi.decode(signatureUpdate, (uint256, bytes));
216215

@@ -259,7 +258,7 @@ contract MultiOwnableWallet is OPStackKeystore, ERC1271, TransientUUPSUpgradeabl
259258
override
260259
returns (bool)
261260
{
262-
bytes32 newConfigHash = ConfigLib.hash(newConfig);
261+
bytes32 newConfigHash = ConfigLib.hash({config: newConfig, account: address(this)});
263262
(bytes memory signatureAuth,) = abi.decode(authorizationProof, (bytes, bytes));
264263

265264
// Ensure the update is authorized.
@@ -288,7 +287,7 @@ contract MultiOwnableWallet is OPStackKeystore, ERC1271, TransientUUPSUpgradeabl
288287

289288
// Otherwise set the new signers.
290289
(, address[] memory signers) = abi.decode(newConfig.data, (address, address[]));
291-
bytes32 configHash = ConfigLib.hash(newConfig);
290+
bytes32 configHash = ConfigLib.hash({config: newConfig, account: address(this)});
292291
mapping(address signer => bool isSigner) storage signers_ = _sWallet().keystoreConfig[configHash].signers;
293292
for (uint256 i; i < signers.length; i++) {
294293
signers_[signers[i]] = true;

contracts/src/examples/MultiOwnableWalletFactory.opstack.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ pragma solidity ^0.8.27;
33

44
import {LibClone} from "solady/utils/LibClone.sol";
55

6-
import {ConfigLib} from "../KeystoreLibs.sol";
6+
import {ConfigLib} from "../core/KeystoreLibs.sol";
77

88
import {MultiOwnableWallet} from "./MultiOwnableWallet.opstack.sol";
99

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
// SPDX-License-Identifier: UNLICENSED
2+
pragma solidity ^0.8.27;
3+
4+
import {Keystore} from "../../core/Keystore.sol";
5+
import {ConfigLib} from "../../core/KeystoreLibs.sol";
6+
7+
abstract contract WithdrawableKeystore is Keystore {
8+
////////////////////////////////////////////////////////////////////////////////////////////////////
9+
// ERRORS //
10+
////////////////////////////////////////////////////////////////////////////////////////////////////
11+
12+
/// @notice Thrown when the call is not performed on the master chain.
13+
///
14+
/// @param chainId The current `block.chainid`.
15+
/// @param masterChainId The master chain id.
16+
error NotOnMasterChain(uint256 chainId, uint256 masterChainId);
17+
18+
/// @notice Thrown when the call is not performed on the L1.
19+
///
20+
/// @param chainId The current `block.chainid`.
21+
error NotOnL1(uint256 chainId);
22+
23+
/// @notice Thrown when the call to `withdrawConfigReceiver()` was not initiated by a Keystore config withdrawal
24+
/// on the master chain.
25+
error CallNotInitiatedByWithdrawalFromMasterChain();
26+
27+
////////////////////////////////////////////////////////////////////////////////////////////////////
28+
// MODIFIERS //
29+
////////////////////////////////////////////////////////////////////////////////////////////////////
30+
31+
/// @notice Ensures the call is performed on the master chain.
32+
modifier onlyOnMasterChain() {
33+
require(
34+
block.chainid == masterChainId, NotOnMasterChain({chainId: block.chainid, masterChainId: masterChainId})
35+
);
36+
_;
37+
}
38+
39+
/// @notice Ensures the call is performed on the L1.
40+
modifier onlyOnL1() {
41+
require(block.chainid == 1, NotOnL1(block.chainid));
42+
_;
43+
}
44+
45+
////////////////////////////////////////////////////////////////////////////////////////////////////
46+
// PUBLIC FUNCTIONS //
47+
////////////////////////////////////////////////////////////////////////////////////////////////////
48+
49+
/// @notice Withdraws the master Keystore config to the L1.
50+
///
51+
/// @dev Reverts if not called on the master chain.
52+
///
53+
/// @param masterConfig The master Keystore config to withdraw.
54+
function withdrawMasterConfig(ConfigLib.Config calldata masterConfig) external onlyOnMasterChain {
55+
// Ensure the provided `masterConfig` hashes to `masterConfigHash`.
56+
(bytes32 masterConfigHash, uint256 masterBlockTimestamp) = _confirmedConfigHash();
57+
ConfigLib.verify({config: masterConfig, account: address(this), configHash: masterConfigHash});
58+
59+
// Withdraw the config to L1.
60+
// FIXME: If the contract on L1 is compromised it could lead to account takeover on all chains.
61+
// Current solution would be to not withdraw to the account directly but to a dedicated L1 contract.
62+
_withdrawConfig({masterConfig: masterConfig, masterBlockTimestamp: masterBlockTimestamp});
63+
}
64+
65+
/// @notice Receives a Keystore config withdrawal on L1.
66+
///
67+
/// @dev Reverts if not called on the L1.
68+
///
69+
/// @param masterConfig The master Keystore config to apply.
70+
/// @param newMasterBlockTimestamp The master chain block timestamp.
71+
function withdrawConfigReceiver(ConfigLib.Config calldata masterConfig, uint256 newMasterBlockTimestamp)
72+
external
73+
onlyOnL1
74+
{
75+
require(_isWithdrawalFromMasterKeystore(), CallNotInitiatedByWithdrawalFromMasterChain());
76+
77+
// Ensure we are going forward when confirming a new config.
78+
(, uint256 masterBlockTimestamp) = _confirmedConfigHash();
79+
require(
80+
newMasterBlockTimestamp > masterBlockTimestamp,
81+
ConfirmedConfigOutdated({
82+
currentMasterBlockTimestamp: masterBlockTimestamp,
83+
newMasterBlockTimestamp: newMasterBlockTimestamp
84+
})
85+
);
86+
87+
// Apply the new confirmed config to the Keystore storage.
88+
_applyNewConfirmedConfig({
89+
newConfirmedConfigHash: ConfigLib.hash({config: masterConfig, account: address(this)}),
90+
newConfirmedConfig: masterConfig,
91+
newMasterBlockTimestamp: newMasterBlockTimestamp
92+
});
93+
}
94+
95+
////////////////////////////////////////////////////////////////////////////////////////////////////
96+
// INTERNAL FUNCTIONS //
97+
////////////////////////////////////////////////////////////////////////////////////////////////////
98+
99+
/// @notice Checks if the call was initiated by withdrawal from the master Keystore.
100+
///
101+
/// @return A boolean indicating whether the withdrawal originates from the master Keystore.
102+
function _isWithdrawalFromMasterKeystore() internal virtual returns (bool);
103+
104+
/// @notice Performs a chain-specific Keystore config withdrawal to L1.
105+
///
106+
/// @param masterConfig The master Keystore config to withdraw.
107+
/// @param masterBlockTimestamp The master chain block timestamp.
108+
function _withdrawConfig(ConfigLib.Config calldata masterConfig, uint256 masterBlockTimestamp) internal virtual;
109+
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// SPDX-License-Identifier: UNLICENSED
2+
pragma solidity ^0.8.27;
3+
4+
import {Predeploys} from "optimism-contracts/libraries/Predeploys.sol";
5+
import {ICrossDomainMessenger} from "optimism-interfaces/universal/ICrossDomainMessenger.sol";
6+
7+
import {ConfigLib} from "../../../core/KeystoreLibs.sol";
8+
9+
import {WithdrawableKeystore} from "../WithdrawableKeystore.sol";
10+
11+
abstract contract OPStackWithdrawableKeystore is WithdrawableKeystore {
12+
////////////////////////////////////////////////////////////////////////////////////////////////////
13+
// CONSTANTS //
14+
////////////////////////////////////////////////////////////////////////////////////////////////////
15+
16+
/// @notice The minimum gas limit required for cross-chain message execution.
17+
uint32 constant MIN_GAS_LIMIT = 200_000;
18+
19+
/// @notice The `L1CrossDomainMessenger` address.
20+
address public immutable l1CrossDomainMessenger;
21+
22+
////////////////////////////////////////////////////////////////////////////////////////////////////
23+
// CONSTRUCTOR //
24+
////////////////////////////////////////////////////////////////////////////////////////////////////
25+
26+
/// @notice Constructor.
27+
///
28+
/// @param l1CrossDomainMessenger_ The `L1CrossDomainMessenger` address.
29+
constructor(address l1CrossDomainMessenger_) {
30+
l1CrossDomainMessenger = l1CrossDomainMessenger_;
31+
}
32+
33+
////////////////////////////////////////////////////////////////////////////////////////////////////
34+
// INTERNAL FUNCTIONS //
35+
////////////////////////////////////////////////////////////////////////////////////////////////////
36+
37+
/// @inheritdoc WithdrawableKeystore
38+
function _isWithdrawalFromMasterKeystore() internal virtual override returns (bool) {
39+
// Checks the tx sender is the `L1CrossDomainMessenger` and that the message was sent from this contract.
40+
return msg.sender == l1CrossDomainMessenger
41+
&& ICrossDomainMessenger(l1CrossDomainMessenger).xDomainMessageSender() == address(this);
42+
}
43+
44+
/// @inheritdoc WithdrawableKeystore
45+
function _withdrawConfig(ConfigLib.Config calldata masterConfig, uint256 masterBlockTimestamp)
46+
internal
47+
virtual
48+
override
49+
{
50+
// Send a message to the `L2CrossDomainMessenger`.
51+
ICrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER).sendMessage({
52+
_target: address(this),
53+
_message: abi.encodeCall(this.withdrawConfigReceiver, (masterConfig, masterBlockTimestamp)),
54+
_minGasLimit: MIN_GAS_LIMIT
55+
});
56+
}
57+
}

0 commit comments

Comments
 (0)