-
Notifications
You must be signed in to change notification settings - Fork 12
[WIP] Conditions Hierarchy #444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: signing
Are you sure you want to change the base?
Changes from 1 commit
ffcd98d
5c394bd
b61c1cc
c28b8f7
207e960
23fd883
0570525
64a4578
0fae651
f1afe8a
718a12c
546021f
918ef37
25d3135
9ad0d30
4254cf3
b3137f1
20df5b2
602bca1
e6a1927
15dc5db
2908cc0
d2d20d1
ddd4b54
0303b13
3f557d9
7941cef
3d0b324
589daf9
e701331
47b4379
c68f1de
ebe47c0
17087f6
20620d8
b2adee2
c8f5e63
92609e9
7360cd0
e6ac365
ec420dd
c0da4a3
40bc0b3
9901070
90bd925
fdbd793
6ca8925
55e5243
a1328a9
538c519
1c17598
2613b84
d2f8bbf
4106327
8299bd6
61424c1
b6b4765
8bed9da
9083b57
1c646e4
1afcab2
34bb21a
aac4cc2
7d65022
d3646d5
0248308
1bd49cd
e414d9c
c9d25d6
ab01b9d
8a088aa
0e58a0a
c7ab113
ac07cf8
08c176e
6eddbdd
205e7df
d315bf8
720cb9b
8ec51dc
3485c46
df34d71
54578a6
6a64789
23517ad
6762e0a
86983b3
2e7f5ca
ff57ccb
66fc2dd
466eac6
b0d56dd
39722b2
3285d85
5804859
98c3f91
7588451
406295c
48273ab
cba3b3a
5a3f9a9
84c0de8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,8 +51,9 @@ contract SigningCoordinator is Initializable, AccessControlDefaultAdminRulesUpgr | |
| uint256[20] gap; | ||
| } | ||
|
|
||
| struct ServerAdmin { | ||
| bool active; | ||
| struct Server { | ||
| address[] serverAdmins; | ||
| mapping(address serverAdmin => bool active) activeServerAdmins; | ||
| mapping(uint256 chainId => bytes conditions) conditions; | ||
| } | ||
|
|
||
|
|
@@ -68,8 +69,7 @@ contract SigningCoordinator is Initializable, AccessControlDefaultAdminRulesUpgr | |
| SigningCohortParticipant[] signers; | ||
| uint256[] chains; | ||
| mapping(uint256 chainId => bytes conditions) conditions; // TODO: chainId -> condition itself or hash(condition) | ||
| address[] serverAdmins; | ||
| mapping(address serverAdmin => ServerAdmin) serverAdminConditions; | ||
| mapping(uint256 serverId => Server) serverConditions; | ||
| } | ||
|
|
||
| bytes32 public constant INITIATOR_ROLE = keccak256("INITIATOR_ROLE"); | ||
|
|
@@ -257,31 +257,32 @@ contract SigningCoordinator is Initializable, AccessControlDefaultAdminRulesUpgr | |
| return id; | ||
| } | ||
|
|
||
| function setServerAdmin(uint32 cohortId, address[] memory serverAdmins) external { | ||
| function setServerAdmins( | ||
| uint32 cohortId, | ||
| uint256 serverId, | ||
| address[] memory serverAdmins | ||
| ) external { | ||
| SigningCohort storage signingCohort = signingCohorts[cohortId]; | ||
| require(isCohortActive(signingCohort), "Cohort not active"); | ||
| require( | ||
| signingCohort.authority == msg.sender, | ||
| "Only the cohort authority can set server admin" | ||
| ); | ||
| for (uint256 i = 0; i < signingCohort.serverAdmins.length; i++) { | ||
| address serverAdmin = signingCohort.serverAdmins[i]; | ||
| signingCohort.serverAdminConditions[serverAdmin].active = false; | ||
| require(serverId > 0, "Server id can't be zero"); | ||
| Server storage server = signingCohort.serverConditions[serverId]; | ||
| for (uint256 i = 0; i < server.serverAdmins.length; i++) { | ||
| address serverAdmin = server.serverAdmins[i]; | ||
| server.activeServerAdmins[serverAdmin] = false; | ||
| } | ||
| signingCohort.serverAdmins = serverAdmins; | ||
| for (uint256 i = 0; i < signingCohort.serverAdmins.length; i++) { | ||
| address serverAdmin = signingCohort.serverAdmins[i]; | ||
| signingCohort.serverAdminConditions[serverAdmin].active = true; | ||
| server.serverAdmins = serverAdmins; | ||
| for (uint256 i = 0; i < server.serverAdmins.length; i++) { | ||
| address serverAdmin = server.serverAdmins[i]; | ||
| server.activeServerAdmins[serverAdmin] = true; | ||
| } | ||
| // TODO emit event | ||
| } | ||
|
|
||
| function setSigningCohortConditions( | ||
| uint32 cohortId, | ||
| uint256 chainId, | ||
| bytes calldata conditions | ||
| ) external { | ||
| SigningCohort storage signingCohort = signingCohorts[cohortId]; | ||
| function conditionsCheck(SigningCohort storage signingCohort, uint256 chainId) internal view { | ||
| require(isCohortActive(signingCohort), "Cohort not active"); | ||
| // chainId must already be deployed for the cohort | ||
| bool chainDeployed = false; | ||
|
|
@@ -292,40 +293,62 @@ contract SigningCoordinator is Initializable, AccessControlDefaultAdminRulesUpgr | |
| } | ||
| } | ||
| require(chainDeployed, "Not already deployed"); | ||
| ServerAdmin storage serverAdmin = signingCohort.serverAdminConditions[msg.sender]; | ||
| } | ||
|
|
||
| function setSigningCohortConditions( | ||
| uint32 cohortId, | ||
| uint256 chainId, | ||
| bytes calldata conditions | ||
| ) external { | ||
| SigningCohort storage signingCohort = signingCohorts[cohortId]; | ||
| conditionsCheck(signingCohort, chainId); | ||
| require( | ||
| signingCohort.authority == msg.sender, | ||
| "Only the cohort authority can set conditions" | ||
| ); | ||
| signingCohort.conditions[chainId] = conditions; | ||
| emit SigningCohortConditionsSet(cohortId, msg.sender, chainId, conditions); | ||
| } | ||
|
|
||
| function setSigningCohortConditions( | ||
| uint32 cohortId, | ||
| uint256 chainId, | ||
| bytes calldata conditions, | ||
| uint256 serverId | ||
| ) external { | ||
| SigningCohort storage signingCohort = signingCohorts[cohortId]; | ||
| conditionsCheck(signingCohort, chainId); | ||
| require(serverId > 0, "Server id can't be zero"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why it can't be zero?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some optimization for |
||
| Server storage server = signingCohort.serverConditions[serverId]; | ||
|
|
||
| require( | ||
| signingCohort.authority == msg.sender || serverAdmin.active, | ||
| signingCohort.authority == msg.sender || server.activeServerAdmins[msg.sender], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this mean that the cohort authority can override a server admin for setting the serverId-specific condition? If so, I don't think we want to give the cohort authority that power. Only the server admin can set the serverId-specific condition. (cc @arjunhassard )
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But cohort admin can set itself to server admin anyway, right?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right that technically they can take that route to set a serverid-specific condition. Typically the cohort admin and server admins should be separate. They may be the same in Collab.Land's Discord server, but shouldn't be for other Discord servers.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed privilege for cohort admin |
||
| "Only the cohort authority or a server admin can set conditions" | ||
| ); | ||
| if (serverAdmin.active) { | ||
| serverAdmin.conditions[chainId] = conditions; | ||
| } else { | ||
| signingCohort.conditions[chainId] = conditions; | ||
| } | ||
| emit SigningCohortConditionsSet(cohortId, msg.sender, chainId, conditions); | ||
| server.conditions[chainId] = conditions; | ||
| emit SigningCohortConditionsSet(cohortId, msg.sender, chainId, conditions); // TODO log serverId | ||
| } | ||
|
|
||
| function getSigningCohortConditions( | ||
| uint32 cohortId, | ||
| uint256 chainId | ||
| ) external view returns (bytes[] memory) { | ||
| return getSigningCohortConditions(cohortId, chainId, address(0)); | ||
| return getSigningCohortConditions(cohortId, chainId, 0); | ||
| } | ||
|
|
||
| function getSigningCohortConditions( | ||
| uint32 cohortId, | ||
| uint256 chainId, | ||
| address serverAdmin | ||
| uint256 serverId | ||
| ) public view returns (bytes[] memory conditions) { | ||
| SigningCohort storage signingCohort = signingCohorts[cohortId]; | ||
| bytes memory coreConditions = signingCohort.conditions[chainId]; | ||
| bytes memory serverAdminConditions = signingCohort | ||
| .serverAdminConditions[serverAdmin] | ||
| .conditions[chainId]; | ||
| if (serverAdminConditions.length > 0) { | ||
| bytes memory serverConditions = signingCohort.serverConditions[serverId].conditions[ | ||
| chainId | ||
| ]; | ||
| if (serverConditions.length > 0) { | ||
| conditions = new bytes[](2); | ||
| conditions[1] = serverAdminConditions; | ||
| conditions[1] = serverConditions; | ||
| } else { | ||
| conditions = new bytes[](1); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should find a more generic term for
Server, which is too specific to a particular use case. What about something likeDomain?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like
DomainThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️