Skip to content

Commit a79c91d

Browse files
nkrishangnkrishang
authored
Fix AccountPermsissions tracking of signers (#407)
* rename account factory callbacks * Create IAccountCore * pkg update * call _afterChangeRole in changeRole * replace delete with loop remove for EnumerableSet * add test for changeRole * fix setRoleRestrictions
1 parent 5c1a785 commit a79c91d

File tree

6 files changed

+494
-10
lines changed

6 files changed

+494
-10
lines changed

‎contracts/dynamic-contracts/extension/AccountPermissions.sol

+16-6
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,28 @@ abstract contract AccountPermissions is IAccountPermissions, EIP712 {
5858

5959
/// @notice Sets the restrictions for a given role.
6060
function setRoleRestrictions(RoleRestrictions calldata _restrictions) external virtual onlyAdmin {
61-
require(_restrictions.role != bytes32(0), "AccountPermissions: role cannot be empty");
61+
bytes32 role = _restrictions.role;
62+
63+
require(role != bytes32(0), "AccountPermissions: role cannot be empty");
6264

6365
AccountPermissionsStorage.Data storage data = AccountPermissionsStorage.accountPermissionsStorage();
64-
data.roleRestrictions[_restrictions.role] = RoleStatic(
65-
_restrictions.role,
66+
data.roleRestrictions[role] = RoleStatic(
67+
role,
6668
_restrictions.maxValuePerTransaction,
6769
_restrictions.startTimestamp,
6870
_restrictions.endTimestamp
6971
);
7072

73+
address[] memory currentTargets = data.approvedTargets[role].values();
74+
uint256 currentLen = currentTargets.length;
75+
76+
for (uint256 i = 0; i < currentLen; i += 1) {
77+
data.approvedTargets[role].remove(currentTargets[i]);
78+
}
79+
7180
uint256 len = _restrictions.approvedTargets.length;
72-
delete data.approvedTargets[_restrictions.role];
73-
for (uint256 i = 0; i < len; i++) {
74-
data.approvedTargets[_restrictions.role].add(_restrictions.approvedTargets[i]);
81+
for (uint256 i = 0; i < len; i += 1) {
82+
data.approvedTargets[role].add(_restrictions.approvedTargets[i]);
7583
}
7684

7785
emit RoleUpdated(_restrictions.role, _restrictions);
@@ -101,6 +109,8 @@ abstract contract AccountPermissions is IAccountPermissions, EIP712 {
101109
}
102110

103111
emit RoleAssignment(_req.role, _req.target, signer, _req);
112+
113+
_afterChangeRole(_req);
104114
}
105115

106116
/*///////////////////////////////////////////////////////////////

‎contracts/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@thirdweb-dev/contracts",
33
"description": "Collection of smart contracts deployable via the thirdweb SDK, dashboard and CLI",
4-
"version": "3.5.6",
4+
"version": "3.5.7-0",
55
"license": "Apache-2.0",
66
"repository": {
77
"type": "git",

‎contracts/smart-wallet/interfaces/IAccountCore.sol

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// SPDX-License-Identifier: Apache 2.0
2+
pragma solidity ^0.8.12;
3+
4+
import "./IAccount.sol";
5+
import "../../extension/interface/IAccountPermissions.sol";
6+
import "../../extension/interface/IMulticall.sol";
7+
8+
interface IAccountCore is IAccount, IAccountPermissions, IMulticall {
9+
/// @dev Returns the address of the factory from which the account was created.
10+
function factory() external view returns (address);
11+
}

‎contracts/smart-wallet/utils/AccountCore.sol

+3-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import "./BaseAccountFactory.sol";
1919
import "../non-upgradeable/Account.sol";
2020
import "../../openzeppelin-presets/utils/cryptography/ECDSA.sol";
2121

22+
import "../interfaces/IAccountCore.sol";
23+
2224
// $$\ $$\ $$\ $$\ $$\
2325
// $$ | $$ | \__| $$ | $$ |
2426
// $$$$$$\ $$$$$$$\ $$\ $$$$$$\ $$$$$$$ |$$\ $$\ $$\ $$$$$$\ $$$$$$$\
@@ -28,7 +30,7 @@ import "../../openzeppelin-presets/utils/cryptography/ECDSA.sol";
2830
// \$$$$ |$$ | $$ |$$ |$$ | \$$$$$$$ |\$$$$$\$$$$ |\$$$$$$$\ $$$$$$$ |
2931
// \____/ \__| \__|\__|\__| \_______| \_____\____/ \_______|\_______/
3032

31-
contract AccountCore is Initializable, Multicall, BaseAccount, ERC1271, AccountPermissions {
33+
contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, ERC1271, AccountPermissions {
3234
using ECDSA for bytes32;
3335
using EnumerableSet for EnumerableSet.AddressSet;
3436

‎docs/IAccountCore.md

+296
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
# IAccountCore
2+
3+
4+
5+
6+
7+
8+
9+
10+
11+
## Methods
12+
13+
### changeRole
14+
15+
```solidity
16+
function changeRole(IAccountPermissions.RoleRequest req, bytes signature) external nonpayable
17+
```
18+
19+
20+
21+
22+
23+
#### Parameters
24+
25+
| Name | Type | Description |
26+
|---|---|---|
27+
| req | IAccountPermissions.RoleRequest | undefined |
28+
| signature | bytes | undefined |
29+
30+
### factory
31+
32+
```solidity
33+
function factory() external view returns (address)
34+
```
35+
36+
37+
38+
*Returns the address of the factory from which the account was created.*
39+
40+
41+
#### Returns
42+
43+
| Name | Type | Description |
44+
|---|---|---|
45+
| _0 | address | undefined |
46+
47+
### getAllRoleMembers
48+
49+
```solidity
50+
function getAllRoleMembers(bytes32 role) external view returns (address[] members)
51+
```
52+
53+
Returns all accounts that have a role.
54+
55+
56+
57+
#### Parameters
58+
59+
| Name | Type | Description |
60+
|---|---|---|
61+
| role | bytes32 | undefined |
62+
63+
#### Returns
64+
65+
| Name | Type | Description |
66+
|---|---|---|
67+
| members | address[] | undefined |
68+
69+
### getRoleRestrictions
70+
71+
```solidity
72+
function getRoleRestrictions(bytes32 role) external view returns (struct IAccountPermissions.RoleRestrictions restrictions)
73+
```
74+
75+
Returns the role restrictions for a given role.
76+
77+
78+
79+
#### Parameters
80+
81+
| Name | Type | Description |
82+
|---|---|---|
83+
| role | bytes32 | undefined |
84+
85+
#### Returns
86+
87+
| Name | Type | Description |
88+
|---|---|---|
89+
| restrictions | IAccountPermissions.RoleRestrictions | undefined |
90+
91+
### getRoleRestrictionsForAccount
92+
93+
```solidity
94+
function getRoleRestrictionsForAccount(address account) external view returns (struct IAccountPermissions.RoleRestrictions role)
95+
```
96+
97+
Returns the role held by a given account along with its restrictions.
98+
99+
100+
101+
#### Parameters
102+
103+
| Name | Type | Description |
104+
|---|---|---|
105+
| account | address | undefined |
106+
107+
#### Returns
108+
109+
| Name | Type | Description |
110+
|---|---|---|
111+
| role | IAccountPermissions.RoleRestrictions | undefined |
112+
113+
### isAdmin
114+
115+
```solidity
116+
function isAdmin(address account) external view returns (bool)
117+
```
118+
119+
Returns whether the given account is an admin.
120+
121+
122+
123+
#### Parameters
124+
125+
| Name | Type | Description |
126+
|---|---|---|
127+
| account | address | undefined |
128+
129+
#### Returns
130+
131+
| Name | Type | Description |
132+
|---|---|---|
133+
| _0 | bool | undefined |
134+
135+
### multicall
136+
137+
```solidity
138+
function multicall(bytes[] data) external nonpayable returns (bytes[] results)
139+
```
140+
141+
142+
143+
*Receives and executes a batch of function calls on this contract.*
144+
145+
#### Parameters
146+
147+
| Name | Type | Description |
148+
|---|---|---|
149+
| data | bytes[] | undefined |
150+
151+
#### Returns
152+
153+
| Name | Type | Description |
154+
|---|---|---|
155+
| results | bytes[] | undefined |
156+
157+
### setAdmin
158+
159+
```solidity
160+
function setAdmin(address account, bool isAdmin) external nonpayable
161+
```
162+
163+
Adds / removes an account as an admin.
164+
165+
166+
167+
#### Parameters
168+
169+
| Name | Type | Description |
170+
|---|---|---|
171+
| account | address | undefined |
172+
| isAdmin | bool | undefined |
173+
174+
### setRoleRestrictions
175+
176+
```solidity
177+
function setRoleRestrictions(IAccountPermissions.RoleRestrictions role) external nonpayable
178+
```
179+
180+
181+
182+
183+
184+
#### Parameters
185+
186+
| Name | Type | Description |
187+
|---|---|---|
188+
| role | IAccountPermissions.RoleRestrictions | undefined |
189+
190+
### validateUserOp
191+
192+
```solidity
193+
function validateUserOp(UserOperation userOp, bytes32 userOpHash, uint256 missingAccountFunds) external nonpayable returns (uint256 validationData)
194+
```
195+
196+
197+
198+
199+
200+
#### Parameters
201+
202+
| Name | Type | Description |
203+
|---|---|---|
204+
| userOp | UserOperation | undefined |
205+
| userOpHash | bytes32 | undefined |
206+
| missingAccountFunds | uint256 | undefined |
207+
208+
#### Returns
209+
210+
| Name | Type | Description |
211+
|---|---|---|
212+
| validationData | uint256 | undefined |
213+
214+
### verifyRoleRequest
215+
216+
```solidity
217+
function verifyRoleRequest(IAccountPermissions.RoleRequest req, bytes signature) external view returns (bool success, address signer)
218+
```
219+
220+
221+
222+
223+
224+
#### Parameters
225+
226+
| Name | Type | Description |
227+
|---|---|---|
228+
| req | IAccountPermissions.RoleRequest | undefined |
229+
| signature | bytes | undefined |
230+
231+
#### Returns
232+
233+
| Name | Type | Description |
234+
|---|---|---|
235+
| success | bool | undefined |
236+
| signer | address | undefined |
237+
238+
239+
240+
## Events
241+
242+
### AdminUpdated
243+
244+
```solidity
245+
event AdminUpdated(address indexed account, bool isAdmin)
246+
```
247+
248+
Emitted when an admin is set or removed.
249+
250+
251+
252+
#### Parameters
253+
254+
| Name | Type | Description |
255+
|---|---|---|
256+
| account `indexed` | address | undefined |
257+
| isAdmin | bool | undefined |
258+
259+
### RoleAssignment
260+
261+
```solidity
262+
event RoleAssignment(bytes32 indexed role, address indexed account, address indexed signer, IAccountPermissions.RoleRequest request)
263+
```
264+
265+
Emitted when a role is granted / revoked by an authorized party.
266+
267+
268+
269+
#### Parameters
270+
271+
| Name | Type | Description |
272+
|---|---|---|
273+
| role `indexed` | bytes32 | undefined |
274+
| account `indexed` | address | undefined |
275+
| signer `indexed` | address | undefined |
276+
| request | IAccountPermissions.RoleRequest | undefined |
277+
278+
### RoleUpdated
279+
280+
```solidity
281+
event RoleUpdated(bytes32 indexed role, IAccountPermissions.RoleRestrictions restrictions)
282+
```
283+
284+
Emitted when the restrictions for a given role are updated.
285+
286+
287+
288+
#### Parameters
289+
290+
| Name | Type | Description |
291+
|---|---|---|
292+
| role `indexed` | bytes32 | undefined |
293+
| restrictions | IAccountPermissions.RoleRestrictions | undefined |
294+
295+
296+

0 commit comments

Comments
 (0)