Skip to content

Commit 1e66afe

Browse files
ItsNickBarryItsNickBarry
committed
fix ECDSAMultisigWallet visibility layers and move custom errors to internal interface
1 parent 96aef1e commit 1e66afe

9 files changed

+133
-83
lines changed

‎abi/ECDSAMultisigWallet.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,14 @@
11
[
2+
{
3+
"inputs": [],
4+
"name": "ECDSAMultisigWallet__AddSignerFailed",
5+
"type": "error"
6+
},
7+
{
8+
"inputs": [],
9+
"name": "ECDSAMultisigWallet__InsufficientSigners",
10+
"type": "error"
11+
},
212
{
313
"inputs": [],
414
"name": "ECDSAMultisigWallet__InvalidNonce",
@@ -19,11 +29,21 @@
1929
"name": "ECDSAMultisigWallet__RecoveredSignerNotAuthorized",
2030
"type": "error"
2131
},
32+
{
33+
"inputs": [],
34+
"name": "ECDSAMultisigWallet__RemoveSignerFailed",
35+
"type": "error"
36+
},
2237
{
2338
"inputs": [],
2439
"name": "ECDSAMultisigWallet__SignerAlreadySigned",
2540
"type": "error"
2641
},
42+
{
43+
"inputs": [],
44+
"name": "ECDSAMultisigWallet__SignerLimitReached",
45+
"type": "error"
46+
},
2747
{
2848
"inputs": [],
2949
"name": "ECDSA__InvalidS",

‎abi/ECDSAMultisigWalletInternal.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,14 @@
11
[
2+
{
3+
"inputs": [],
4+
"name": "ECDSAMultisigWallet__AddSignerFailed",
5+
"type": "error"
6+
},
7+
{
8+
"inputs": [],
9+
"name": "ECDSAMultisigWallet__InsufficientSigners",
10+
"type": "error"
11+
},
212
{
313
"inputs": [],
414
"name": "ECDSAMultisigWallet__InvalidNonce",
@@ -19,9 +29,19 @@
1929
"name": "ECDSAMultisigWallet__RecoveredSignerNotAuthorized",
2030
"type": "error"
2131
},
32+
{
33+
"inputs": [],
34+
"name": "ECDSAMultisigWallet__RemoveSignerFailed",
35+
"type": "error"
36+
},
2237
{
2338
"inputs": [],
2439
"name": "ECDSAMultisigWallet__SignerAlreadySigned",
2540
"type": "error"
41+
},
42+
{
43+
"inputs": [],
44+
"name": "ECDSAMultisigWallet__SignerLimitReached",
45+
"type": "error"
2646
}
2747
]

‎abi/ECDSAMultisigWalletStorage.json‎

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

‎abi/IECDSAMultisigWallet.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,14 @@
11
[
2+
{
3+
"inputs": [],
4+
"name": "ECDSAMultisigWallet__AddSignerFailed",
5+
"type": "error"
6+
},
7+
{
8+
"inputs": [],
9+
"name": "ECDSAMultisigWallet__InsufficientSigners",
10+
"type": "error"
11+
},
212
{
313
"inputs": [],
414
"name": "ECDSAMultisigWallet__InvalidNonce",
@@ -19,11 +29,21 @@
1929
"name": "ECDSAMultisigWallet__RecoveredSignerNotAuthorized",
2030
"type": "error"
2131
},
32+
{
33+
"inputs": [],
34+
"name": "ECDSAMultisigWallet__RemoveSignerFailed",
35+
"type": "error"
36+
},
2237
{
2338
"inputs": [],
2439
"name": "ECDSAMultisigWallet__SignerAlreadySigned",
2540
"type": "error"
2641
},
42+
{
43+
"inputs": [],
44+
"name": "ECDSAMultisigWallet__SignerLimitReached",
45+
"type": "error"
46+
},
2747
{
2848
"inputs": [
2949
{

‎abi/IECDSAMultisigWalletInternal.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,14 @@
11
[
2+
{
3+
"inputs": [],
4+
"name": "ECDSAMultisigWallet__AddSignerFailed",
5+
"type": "error"
6+
},
7+
{
8+
"inputs": [],
9+
"name": "ECDSAMultisigWallet__InsufficientSigners",
10+
"type": "error"
11+
},
212
{
313
"inputs": [],
414
"name": "ECDSAMultisigWallet__InvalidNonce",
@@ -19,9 +29,19 @@
1929
"name": "ECDSAMultisigWallet__RecoveredSignerNotAuthorized",
2030
"type": "error"
2131
},
32+
{
33+
"inputs": [],
34+
"name": "ECDSAMultisigWallet__RemoveSignerFailed",
35+
"type": "error"
36+
},
2237
{
2338
"inputs": [],
2439
"name": "ECDSAMultisigWallet__SignerAlreadySigned",
2540
"type": "error"
41+
},
42+
{
43+
"inputs": [],
44+
"name": "ECDSAMultisigWallet__SignerLimitReached",
45+
"type": "error"
2646
}
2747
]

‎contracts/multisig/ECDSAMultisigWalletInternal.sol‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,51 @@ abstract contract ECDSAMultisigWalletInternal is IECDSAMultisigWalletInternal {
1616
using EnumerableSet for EnumerableSet.AddressSet;
1717
using ECDSAMultisigWalletStorage for ECDSAMultisigWalletStorage.Layout;
1818

19+
function _isInvalidNonce(address account, uint256 nonce)
20+
internal
21+
view
22+
returns (bool)
23+
{
24+
return ECDSAMultisigWalletStorage.layout().nonces[account][nonce];
25+
}
26+
27+
function _setInvalidNonce(address account, uint256 nonce) internal {
28+
ECDSAMultisigWalletStorage.layout().nonces[account][nonce] = true;
29+
}
30+
31+
function _setQuorum(uint256 quorum) internal {
32+
ECDSAMultisigWalletStorage.Layout storage l = ECDSAMultisigWalletStorage
33+
.layout();
34+
35+
if (quorum > l.signers.length())
36+
revert ECDSAMultisigWallet__InsufficientSigners();
37+
l.quorum = quorum;
38+
}
39+
40+
function _isSigner(address account) internal view returns (bool) {
41+
return ECDSAMultisigWalletStorage.layout().signers.contains(account);
42+
}
43+
44+
function _addSigner(address account) internal {
45+
ECDSAMultisigWalletStorage.Layout storage l = ECDSAMultisigWalletStorage
46+
.layout();
47+
48+
if (l.signers.length() >= 256)
49+
revert ECDSAMultisigWallet__SignerLimitReached();
50+
if (!l.signers.add(account))
51+
revert ECDSAMultisigWallet__AddSignerFailed();
52+
}
53+
54+
function _removeSigner(address account) internal {
55+
ECDSAMultisigWalletStorage.Layout storage l = ECDSAMultisigWalletStorage
56+
.layout();
57+
58+
if (l.quorum > l.signers.length() - 1)
59+
revert ECDSAMultisigWallet__InsufficientSigners();
60+
if (!l.signers.remove(account))
61+
revert ECDSAMultisigWallet__RemoveSignerFailed();
62+
}
63+
1964
/**
2065
* @notice verify signatures and execute "call" or "delegatecall" with given parameters
2166
* @dev message parameters must be included in signature
@@ -103,10 +148,10 @@ abstract contract ECDSAMultisigWalletInternal is IECDSAMultisigWalletInternal {
103148

104149
if (index >= 256)
105150
revert ECDSAMultisigWallet__RecoveredSignerNotAuthorized();
106-
if (l.isInvalidNonce(signer, signature.nonce))
151+
if (_isInvalidNonce(signer, signature.nonce))
107152
revert ECDSAMultisigWallet__InvalidNonce();
108153

109-
l.setInvalidNonce(signer, signature.nonce);
154+
_setInvalidNonce(signer, signature.nonce);
110155

111156
uint256 shift = 1 << index;
112157

‎contracts/multisig/ECDSAMultisigWalletMock.sol‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,13 @@
33
pragma solidity ^0.8.8;
44

55
import { ECDSAMultisigWallet } from './ECDSAMultisigWallet.sol';
6-
import { ECDSAMultisigWalletStorage } from './ECDSAMultisigWalletStorage.sol';
76

87
contract ECDSAMultisigWalletMock is ECDSAMultisigWallet {
9-
using ECDSAMultisigWalletStorage for ECDSAMultisigWalletStorage.Layout;
10-
118
constructor(address[] memory signers, uint256 quorum) {
12-
ECDSAMultisigWalletStorage.Layout storage l = ECDSAMultisigWalletStorage
13-
.layout();
14-
159
for (uint256 i; i < signers.length; i++) {
16-
l.addSigner(signers[i]);
10+
_addSigner(signers[i]);
1711
}
1812

19-
l.setQuorum(quorum);
13+
_setQuorum(quorum);
2014
}
2115
}

‎contracts/multisig/ECDSAMultisigWalletStorage.sol‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,6 @@ pragma solidity ^0.8.8;
55
import { EnumerableSet } from '../utils/EnumerableSet.sol';
66

77
library ECDSAMultisigWalletStorage {
8-
using EnumerableSet for EnumerableSet.AddressSet;
9-
10-
error ECDSAMultisigWalletStorage__AddSignerFailed();
11-
error ECDSAMultisigWalletStorage__InsufficientSigners();
12-
error ECDSAMultisigWalletStorage__RemoveSignerFailed();
13-
error ECDSAMultisigWalletStorage__SignerLimitReached();
14-
158
struct Layout {
169
uint256 quorum;
1710
EnumerableSet.AddressSet signers;
@@ -27,48 +20,4 @@ library ECDSAMultisigWalletStorage {
2720
l.slot := slot
2821
}
2922
}
30-
31-
function isInvalidNonce(
32-
Layout storage l,
33-
address account,
34-
uint256 nonce
35-
) internal view returns (bool) {
36-
return l.nonces[account][nonce];
37-
}
38-
39-
function setInvalidNonce(
40-
Layout storage l,
41-
address account,
42-
uint256 nonce
43-
) internal {
44-
l.nonces[account][nonce] = true;
45-
}
46-
47-
function setQuorum(Layout storage l, uint256 quorum) internal {
48-
if (quorum > l.signers.length())
49-
revert ECDSAMultisigWalletStorage__InsufficientSigners();
50-
l.quorum = quorum;
51-
}
52-
53-
function isSigner(Layout storage l, address account)
54-
internal
55-
view
56-
returns (bool)
57-
{
58-
return l.signers.contains(account);
59-
}
60-
61-
function addSigner(Layout storage l, address account) internal {
62-
if (l.signers.length() >= 256)
63-
revert ECDSAMultisigWalletStorage__SignerLimitReached();
64-
if (!l.signers.add(account))
65-
revert ECDSAMultisigWalletStorage__AddSignerFailed();
66-
}
67-
68-
function removeSigner(Layout storage l, address account) internal {
69-
if (l.quorum > l.signers.length() - 1)
70-
revert ECDSAMultisigWalletStorage__InsufficientSigners();
71-
if (!l.signers.remove(account))
72-
revert ECDSAMultisigWalletStorage__RemoveSignerFailed();
73-
}
7423
}

‎contracts/multisig/IECDSAMultisigWalletInternal.sol‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ interface IECDSAMultisigWalletInternal {
88
error ECDSAMultisigWallet__QuorumNotReached();
99
error ECDSAMultisigWallet__RecoveredSignerNotAuthorized();
1010
error ECDSAMultisigWallet__SignerAlreadySigned();
11+
error ECDSAMultisigWallet__AddSignerFailed();
12+
error ECDSAMultisigWallet__InsufficientSigners();
13+
error ECDSAMultisigWallet__RemoveSignerFailed();
14+
error ECDSAMultisigWallet__SignerLimitReached();
1115

1216
struct Parameters {
1317
address payable target;

0 commit comments

Comments
 (0)