From 8d3dbcd17b4a1f86d2397fd8acfadd4af6eb60d2 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Wed, 21 Jun 2023 15:58:53 +0200 Subject: [PATCH 01/19] IAccessController contract --- contracts/contracts/coordination/IAccessControler.sol | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 contracts/contracts/coordination/IAccessControler.sol diff --git a/contracts/contracts/coordination/IAccessControler.sol b/contracts/contracts/coordination/IAccessControler.sol new file mode 100644 index 00000000..5a8c8c9f --- /dev/null +++ b/contracts/contracts/coordination/IAccessControler.sol @@ -0,0 +1,9 @@ +pragma solidity ^0.8.0; + +interface IAccessController { + function isEnricoAuthorized( + uint256 ritualID, + bytes memory evidence, + bytes memory ciphertextHash + ) external view returns(bool); +} From 546a8b95923fea31661f2242afef9da8aa338790 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Wed, 21 Jun 2023 15:59:11 +0200 Subject: [PATCH 02/19] first version of AllowList contract --- .../contracts/coordination/AllowList.sol | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 contracts/contracts/coordination/AllowList.sol diff --git a/contracts/contracts/coordination/AllowList.sol b/contracts/contracts/coordination/AllowList.sol new file mode 100644 index 00000000..fc65e145 --- /dev/null +++ b/contracts/contracts/coordination/AllowList.sol @@ -0,0 +1,50 @@ +pragma solidity ^0.8.0; +import "@openzeppelin/contracts/access/AccessControl.sol"; +import "./IAccessController.sol"; +import "./Coordinator.sol"; + +contract AllowList is AccessControl, IAccessController { + Coordinator public coordinator; + + // mapp + mapping(uint256 => mapping(address => bool)) public rituals; + + constructor(Coordinator _coordinator) { + coordinator = _coordinator; + } + + function bytesToAddress(bytes memory bys) private pure returns (address addr) { + assembly { + addr := mload(add(bys, 20)) + } + } + + function isEnricoAuthorized( + uint256 ritualID, + bytes memory evidence, + bytes memory ciphertextHash + ) public view override returns(bool) { + enricoAddress = address(uint160(bytes20(evidence))); + return rituals[ritualID][enricoAddress]; + } + + function authorize(uint256 ritualID, address[] calldata addresses) public { + require(coordinator.rituals(ritualId).authority == msg.sender, + "Only ritual authority is permitted"); + require(coordinator.getRitualStatus(ritualId) == RitualStatus.ACTIVE, + "Only active rituals can add authorizations"); + for (uint i=0; i Date: Wed, 21 Jun 2023 16:40:15 +0200 Subject: [PATCH 03/19] Implements per-ritual customization of authorization contract, new rituals use a default authorization contract deployemnt (AllowList). Co-authored-by: James Campbell Co-authored-by: Kieran Prasch --- .../contracts/coordination/AllowList.sol | 23 ++++++----- .../contracts/coordination/Coordinator.sol | 41 +++++++++++++++++-- 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/contracts/contracts/coordination/AllowList.sol b/contracts/contracts/coordination/AllowList.sol index fc65e145..53b63d86 100644 --- a/contracts/contracts/coordination/AllowList.sol +++ b/contracts/contracts/coordination/AllowList.sol @@ -1,22 +1,25 @@ pragma solidity ^0.8.0; -import "@openzeppelin/contracts/access/AccessControl.sol"; +import "@openzeppelin/contracts/access/AccessControlDefaultAdminRules.sol"; import "./IAccessController.sol"; import "./Coordinator.sol"; -contract AllowList is AccessControl, IAccessController { + +contract AllowList is AccessControlDefaultAdminRules, IAccessController { Coordinator public coordinator; // mapp mapping(uint256 => mapping(address => bool)) public rituals; - constructor(Coordinator _coordinator) { + constructor( + Coordinator _coordinator, + address _admin + ) AccessControlDefaultAdminRules(0, _admin) { coordinator = _coordinator; } - function bytesToAddress(bytes memory bys) private pure returns (address addr) { - assembly { - addr := mload(add(bys, 20)) - } + function setCoordinator(Coordinator _coordinator) public { + require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Only admin can set coordinator"); + coordinator = _coordinator; } function isEnricoAuthorized( @@ -24,14 +27,14 @@ contract AllowList is AccessControl, IAccessController { bytes memory evidence, bytes memory ciphertextHash ) public view override returns(bool) { - enricoAddress = address(uint160(bytes20(evidence))); + address enricoAddress = address(uint160(bytes20(evidence))); return rituals[ritualID][enricoAddress]; } function authorize(uint256 ritualID, address[] calldata addresses) public { require(coordinator.rituals(ritualId).authority == msg.sender, "Only ritual authority is permitted"); - require(coordinator.getRitualStatus(ritualId) == RitualStatus.ACTIVE, + require(coordinator.getRitualStatus(ritualId) == RitualStatus.FINALIZED, "Only active rituals can add authorizations"); for (uint i=0; i Date: Wed, 21 Jun 2023 17:27:31 +0200 Subject: [PATCH 04/19] Respond to RFCs in PR #89 --- contracts/contracts/coordination/AllowList.sol | 7 +++---- contracts/contracts/coordination/Coordinator.sol | 6 ++++-- .../{IAccessControler.sol => IRitualAuthorizer.sol} | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) rename contracts/contracts/coordination/{IAccessControler.sol => IRitualAuthorizer.sol} (71%) diff --git a/contracts/contracts/coordination/AllowList.sol b/contracts/contracts/coordination/AllowList.sol index 53b63d86..d0504884 100644 --- a/contracts/contracts/coordination/AllowList.sol +++ b/contracts/contracts/coordination/AllowList.sol @@ -1,14 +1,13 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/access/AccessControlDefaultAdminRules.sol"; -import "./IAccessController.sol"; +import "./IRitualAuthorizer.sol"; import "./Coordinator.sol"; -contract AllowList is AccessControlDefaultAdminRules, IAccessController { +contract AllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { Coordinator public coordinator; - // mapp - mapping(uint256 => mapping(address => bool)) public rituals; + mapping(uint256 => mapping(address => bool)) public authorizations; constructor( Coordinator _coordinator, diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 4f72f40f..498c1dc9 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -9,6 +9,8 @@ import "./IFeeModel.sol"; import "./IReimbursementPool.sol"; import "../lib/BLS12381.sol"; import "../../threshold/IAccessControlApplication.sol"; +import "../../../../nucypher/tests/acceptance/contracts/.cache/openzeppelin/v4.8.1/access/IAccessControl.sol"; +import "./IRitualAuthorizer.sol"; /** * @title Coordinator @@ -58,7 +60,7 @@ contract Coordinator is AccessControlDefaultAdminRules { address authority; uint16 dkgSize; bool aggregationMismatch; - address accessController; + IRitualAuthorizer accessController; BLS12381.G1Point publicKey; bytes aggregatedTranscript; Participant[] participant; @@ -92,7 +94,7 @@ contract Coordinator is AccessControlDefaultAdminRules { uint16 _maxDkgSize, address _admin, IFeeModel _feeModel, - address _defaultAccessController + IRitualAuthorizer _defaultAccessController ) AccessControlDefaultAdminRules(0, _admin) { require(address(_feeModel.stakes()) == address(_stakes), "Invalid stakes for fee model"); diff --git a/contracts/contracts/coordination/IAccessControler.sol b/contracts/contracts/coordination/IRitualAuthorizer.sol similarity index 71% rename from contracts/contracts/coordination/IAccessControler.sol rename to contracts/contracts/coordination/IRitualAuthorizer.sol index 5a8c8c9f..8685eaa8 100644 --- a/contracts/contracts/coordination/IAccessControler.sol +++ b/contracts/contracts/coordination/IRitualAuthorizer.sol @@ -1,7 +1,7 @@ pragma solidity ^0.8.0; -interface IAccessController { - function isEnricoAuthorized( +interface IRitualAuthorizer { + function isAuthorized( uint256 ritualID, bytes memory evidence, bytes memory ciphertextHash From 4a9f72f602abc645c14a741268fb6f6dcc4209d8 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Sun, 9 Jul 2023 23:42:37 +0200 Subject: [PATCH 05/19] AllowList is now for everybody. --- .../coordination/{AllowList.sol => GlobalAllowList.sol} | 1 + 1 file changed, 1 insertion(+) rename contracts/contracts/coordination/{AllowList.sol => GlobalAllowList.sol} (96%) diff --git a/contracts/contracts/coordination/AllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol similarity index 96% rename from contracts/contracts/coordination/AllowList.sol rename to contracts/contracts/coordination/GlobalAllowList.sol index d0504884..b33847c9 100644 --- a/contracts/contracts/coordination/AllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -3,6 +3,7 @@ import "@openzeppelin/contracts/access/AccessControlDefaultAdminRules.sol"; import "./IRitualAuthorizer.sol"; import "./Coordinator.sol"; +contract GlobalAllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { contract AllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { Coordinator public coordinator; From 29b3f369ea4c29edef5d2121713df484662b2972 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Sun, 9 Jul 2023 23:47:27 +0200 Subject: [PATCH 06/19] What... I mean... how...? Resolved: to blame AI. --- contracts/contracts/coordination/Coordinator.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 498c1dc9..cddc99ca 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -9,7 +9,6 @@ import "./IFeeModel.sol"; import "./IReimbursementPool.sol"; import "../lib/BLS12381.sol"; import "../../threshold/IAccessControlApplication.sol"; -import "../../../../nucypher/tests/acceptance/contracts/.cache/openzeppelin/v4.8.1/access/IAccessControl.sol"; import "./IRitualAuthorizer.sol"; /** From 8918d491e4cdcea8f34dc51b4fe54e6a666d0ee7 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Sun, 9 Jul 2023 23:47:53 +0200 Subject: [PATCH 07/19] Fixing mangled rebase. --- ape-config.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/ape-config.yaml b/ape-config.yaml index 2a9c00c2..9727fd79 100644 --- a/ape-config.yaml +++ b/ape-config.yaml @@ -52,8 +52,6 @@ deployments: pre_base_penalty: 2 pre_penalty_history_coefficient: 0 pre_percentage_penalty_coefficient: 100000 - pre_min_authorization: 40000000000000000000000 - pre_min_operator_seconds: 86400 # one day in seconds reward_duration: 604800 # one week in seconds deauthorization_duration: 5184000 # 60 days in seconds verify: False From 54993485d90feff2981aa262dfed7bcb3fdd504a Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Sun, 9 Jul 2023 23:51:47 +0200 Subject: [PATCH 08/19] Removed defaultAccessController from Ritual init. --- .../contracts/coordination/Coordinator.sol | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index cddc99ca..b51539bc 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -92,8 +92,7 @@ contract Coordinator is AccessControlDefaultAdminRules { uint32 _timeout, uint16 _maxDkgSize, address _admin, - IFeeModel _feeModel, - IRitualAuthorizer _defaultAccessController + IFeeModel _feeModel ) AccessControlDefaultAdminRules(0, _admin) { require(address(_feeModel.stakes()) == address(_stakes), "Invalid stakes for fee model"); @@ -101,8 +100,6 @@ contract Coordinator is AccessControlDefaultAdminRules { timeout = _timeout; maxDkgSize = _maxDkgSize; feeModel = IFeeModel(_feeModel); - - defaultAccessController = _defaultAccessController; } function getRitualState(uint256 ritualId) external view returns (RitualState){ @@ -200,7 +197,7 @@ contract Coordinator is AccessControlDefaultAdminRules { address[] calldata providers, address authority, uint32 duration, - address accessController + IRitualAuthorizer accessController ) internal returns (uint32) { require(authority =! address(0), "Invalid authority"); @@ -253,19 +250,11 @@ contract Coordinator is AccessControlDefaultAdminRules { address[] calldata providers, address authority, uint32 duration, - address accessController + IRitualAuthorizer accessController ) external returns (uint32) { return _initiateRitual(providers, authority, duration, accessController); } - function initiateRitual( - address[] calldata providers, - address authority, - uint32 duration - ) external returns (uint32) { - return _initiateRitual(providers, authority, duration, defaultAccessController); - } - function cohortFingerprint(address[] calldata nodes) public pure returns (bytes32) { return keccak256(abi.encode(nodes)); } From 95ccb8b0c4c726eeb54459c5c6781f2b0d2ee432 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Mon, 10 Jul 2023 00:01:11 +0200 Subject: [PATCH 09/19] ritualID is uint32. It used to be, but it is now, too. --- contracts/contracts/coordination/Coordinator.sol | 8 ++++---- contracts/contracts/coordination/GlobalAllowList.sol | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index b51539bc..65e3ae3d 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -102,7 +102,7 @@ contract Coordinator is AccessControlDefaultAdminRules { feeModel = IFeeModel(_feeModel); } - function getRitualState(uint256 ritualId) external view returns (RitualState){ + function getRitualState(uint32 ritualId) external view returns (RitualState){ // TODO: restrict to ritualID < rituals.length? return getRitualState(rituals[ritualId]); } @@ -382,13 +382,13 @@ contract Coordinator is AccessControlDefaultAdminRules { } function getParticipantFromProvider( - uint256 ritualID, + uint32 ritualID, address provider ) external view returns (Participant memory) { return getParticipantFromProvider(rituals[ritualID], provider); } - function processRitualPayment(uint256 ritualID, address[] calldata providers, uint32 duration) internal { + function processRitualPayment(uint32 ritualID, address[] calldata providers, uint32 duration) internal { uint256 ritualCost = feeModel.getRitualInitiationCost(providers, duration); if (ritualCost > 0) { totalPendingFees += ritualCost; @@ -400,7 +400,7 @@ contract Coordinator is AccessControlDefaultAdminRules { } } - function processPendingFee(uint256 ritualID) public { + function processPendingFee(uint32 ritualID) public { Ritual storage ritual = rituals[ritualID]; RitualState state = getRitualState(ritual); require( diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index b33847c9..a7891bb9 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -22,8 +22,8 @@ contract AllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { coordinator = _coordinator; } - function isEnricoAuthorized( - uint256 ritualID, + function isAuthorized( + uint32 ritualID, bytes memory evidence, bytes memory ciphertextHash ) public view override returns(bool) { @@ -31,8 +31,8 @@ contract AllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { return rituals[ritualID][enricoAddress]; } - function authorize(uint256 ritualID, address[] calldata addresses) public { - require(coordinator.rituals(ritualId).authority == msg.sender, + function authorize(uint32 ritualID, address[] calldata addresses) public { + require(coordinator.getAuthority(ritualID) == msg.sender, "Only ritual authority is permitted"); require(coordinator.getRitualStatus(ritualId) == RitualStatus.FINALIZED, "Only active rituals can add authorizations"); @@ -41,8 +41,8 @@ contract AllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { } } - function deauthorize(uint256 ritualID, address[] calldata addresses) public { - require(coordinator.rituals(ritualId).authority == msg.sender, + function deauthorize(uint32 ritualID, address[] calldata addresses) public { + require(coordinator.getAuthority(ritualID) == msg.sender, "Only ritual authority is permitted"); require(coordinator.getRitualStatus(ritualId) == RitualStatus.FINALIZED, "Only active rituals can add authorizations"); From 7e8b9df496722d517b7833614f5245e11cf99133 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Mon, 10 Jul 2023 00:02:22 +0200 Subject: [PATCH 10/19] New functions for getting authority from ritual ID and checking finalization status. --- contracts/contracts/coordination/Coordinator.sol | 8 ++++++++ contracts/contracts/coordination/GlobalAllowList.sol | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 65e3ae3d..ca3d86e6 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -107,6 +107,10 @@ contract Coordinator is AccessControlDefaultAdminRules { return getRitualState(rituals[ritualId]); } + function isRitualFinalized(uint32 ritualId) external view returns (bool){ + return getRitualState(rituals[ritualId]) == RitualState.FINALIZED; + } + function getRitualState(Ritual storage ritual) internal view returns (RitualState){ uint32 t0 = ritual.initTimestamp; uint32 deadline = t0 + timeout; @@ -295,6 +299,10 @@ contract Coordinator is AccessControlDefaultAdminRules { processReimbursement(initialGasLeft); } + function getAuthority(uint32 ritualId) external view returns (address) { + return rituals[ritualId].authority; + } + function postAggregation( uint32 ritualId, bytes calldata aggregatedTranscript, diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index a7891bb9..2bec69ae 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -34,7 +34,7 @@ contract AllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { function authorize(uint32 ritualID, address[] calldata addresses) public { require(coordinator.getAuthority(ritualID) == msg.sender, "Only ritual authority is permitted"); - require(coordinator.getRitualStatus(ritualId) == RitualStatus.FINALIZED, + require(coordinator.isRitualFinalized(ritualID), "Only active rituals can add authorizations"); for (uint i=0; i Date: Mon, 10 Jul 2023 00:03:57 +0200 Subject: [PATCH 11/19] Guardrail for state that we don't think is reachable anyhow. --- contracts/contracts/coordination/Coordinator.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index ca3d86e6..9ecc5126 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -131,6 +131,7 @@ contract Coordinator is AccessControlDefaultAdminRules { // - No public key // - All transcripts and all aggregations // - Still within the deadline + revert("Invalid ritual state"); } } From 908c593e63975ffd9821ba98ddcbb5474ddc91f2 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Mon, 10 Jul 2023 00:08:40 +0200 Subject: [PATCH 12/19] Invoking internal function by passing ritual storage pointer rather than int. --- contracts/contracts/coordination/Coordinator.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 9ecc5126..48ac2ba8 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -184,7 +184,7 @@ contract Coordinator is AccessControlDefaultAdminRules { function setRitualAuthority(uint32 ritualId, address authority) external { Ritual storage ritual = rituals[ritualId]; - require(getRitualState(ritualId) == RitualState.FINALIZED, "Ritual not finalized"); + require(getRitualState(ritual) == RitualState.FINALIZED, "Ritual not finalized"); require(msg.sender == ritual.authority, "Sender not ritual authority"); ritual.authority = authority; } From 83b406c178ab744078dae3030109898a400432f9 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Mon, 10 Jul 2023 00:10:25 +0200 Subject: [PATCH 13/19] We decided to implement the operator in the correct order of its characters. --- contracts/contracts/coordination/Coordinator.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 48ac2ba8..f4476ad4 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -205,7 +205,7 @@ contract Coordinator is AccessControlDefaultAdminRules { IRitualAuthorizer accessController ) internal returns (uint32) { - require(authority =! address(0), "Invalid authority"); + require(authority != address(0), "Invalid authority"); require( isInitiationPublic || hasRole(INITIATOR_ROLE, msg.sender), From 5a3bf8804fa78368efb970b36c54eacc5c0f83be Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Mon, 10 Jul 2023 00:13:16 +0200 Subject: [PATCH 14/19] AllowLogic enforcement via ECDSA 'evidence' check. --- contracts/contracts/coordination/GlobalAllowList.sol | 12 +++++++----- .../contracts/coordination/IRitualAuthorizer.sol | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index 2bec69ae..93ff22d9 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -1,13 +1,13 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/access/AccessControlDefaultAdminRules.sol"; +import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "./IRitualAuthorizer.sol"; import "./Coordinator.sol"; contract GlobalAllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { + using ECDSA for bytes32; -contract AllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { Coordinator public coordinator; - mapping(uint256 => mapping(address => bool)) public authorizations; constructor( @@ -25,10 +25,12 @@ contract AllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { function isAuthorized( uint32 ritualID, bytes memory evidence, - bytes memory ciphertextHash + bytes memory digest ) public view override returns(bool) { - address enricoAddress = address(uint160(bytes20(evidence))); - return rituals[ritualID][enricoAddress]; + address recovered_address = keccak256(digest) + .toEthSignedMessageHash() + .recover(evidence); + return authorizations[ritualID][recovered_address]; } function authorize(uint32 ritualID, address[] calldata addresses) public { diff --git a/contracts/contracts/coordination/IRitualAuthorizer.sol b/contracts/contracts/coordination/IRitualAuthorizer.sol index 8685eaa8..e624eff3 100644 --- a/contracts/contracts/coordination/IRitualAuthorizer.sol +++ b/contracts/contracts/coordination/IRitualAuthorizer.sol @@ -2,8 +2,8 @@ pragma solidity ^0.8.0; interface IRitualAuthorizer { function isAuthorized( - uint256 ritualID, - bytes memory evidence, - bytes memory ciphertextHash + uint32 ritualID, + bytes memory evidence, // signature + bytes memory digest // signed message hash ) external view returns(bool); } From 7bbae0365fe704a945183ad55e5a2bc791667638 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Mon, 10 Jul 2023 00:14:55 +0200 Subject: [PATCH 15/19] This mapping now tracks authorizations by ritual. --- contracts/contracts/coordination/GlobalAllowList.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index 93ff22d9..b63fe1b6 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -39,7 +39,7 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { require(coordinator.isRitualFinalized(ritualID), "Only active rituals can add authorizations"); for (uint i=0; i Date: Wed, 12 Jul 2023 19:59:03 +0200 Subject: [PATCH 16/19] updates coordinator tests to include allow logic contracts during ritual initialization --- tests/test_coordinator.py | 128 +++++++++++++++++++++++++++++++------- 1 file changed, 104 insertions(+), 24 deletions(-) diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 9ce4cbac..54b982f3 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -93,49 +93,93 @@ def coordinator(project, deployer, stake_info, flat_rate_fee_model, initiator): return contract +@pytest.fixture() +def global_allow_list(project, deployer, coordinator): + contract = project.GlobalAllowList.deploy( + coordinator.address, + deployer, # admin + sender=deployer + ) + return contract + + def test_initial_parameters(coordinator): assert coordinator.maxDkgSize() == MAX_DKG_SIZE assert coordinator.timeout() == TIMEOUT assert coordinator.numberOfRituals() == 0 -def test_invalid_initiate_ritual(coordinator, nodes, accounts, initiator): +def test_invalid_initiate_ritual(coordinator, nodes, accounts, initiator, global_allow_list): with ape.reverts("Sender can't initiate ritual"): sender = accounts[3] - coordinator.initiateRitual(nodes, sender, DURATION, sender=sender) + coordinator.initiateRitual( + nodes, sender, DURATION, global_allow_list.address, sender=sender + ) with ape.reverts("Invalid number of nodes"): - coordinator.initiateRitual(nodes[:5] * 20, initiator, DURATION, sender=initiator) + coordinator.initiateRitual( + nodes[:5] * 20, + initiator, + DURATION, + global_allow_list.address, + sender=initiator + ) with ape.reverts("Invalid ritual duration"): - coordinator.initiateRitual(nodes, initiator, 0, sender=initiator) + coordinator.initiateRitual(nodes, initiator, 0, global_allow_list.address, sender=initiator) with ape.reverts("Provider has not set their public key"): - coordinator.initiateRitual(nodes, initiator, DURATION, sender=initiator) + coordinator.initiateRitual(nodes, initiator, DURATION, global_allow_list.address, sender=initiator) for node in nodes: public_key = gen_public_key() coordinator.setProviderPublicKey(public_key, sender=node) + with ape.reverts("Providers must be sorted"): - coordinator.initiateRitual(nodes[1:] + [nodes[0]], initiator, DURATION, sender=initiator) + coordinator.initiateRitual( + nodes[1:] + [nodes[0]], + initiator, + DURATION, + global_allow_list.address, + sender=initiator + ) with ape.reverts("ERC20: insufficient allowance"): # Sender didn't approve enough tokens - coordinator.initiateRitual(nodes, initiator, DURATION, sender=initiator) + coordinator.initiateRitual( + nodes, + initiator, + DURATION, + global_allow_list.address, + sender=initiator + ) -def initiate_ritual(coordinator, erc20, flat_rate_fee_model, initiator, nodes): +def initiate_ritual(coordinator, erc20, fee_model, allow_logic, authority, nodes): for node in nodes: public_key = gen_public_key() coordinator.setProviderPublicKey(public_key, sender=node) - cost = flat_rate_fee_model.getRitualInitiationCost(nodes, DURATION) - erc20.approve(coordinator.address, cost, sender=initiator) - tx = coordinator.initiateRitual(nodes, initiator, DURATION, sender=initiator) - return initiator, tx + cost = fee_model.getRitualInitiationCost(nodes, DURATION) + erc20.approve(coordinator.address, cost, sender=authority) + tx = coordinator.initiateRitual( + nodes, + authority, + DURATION, + allow_logic.address, + sender=authority + ) + return authority, tx -def test_initiate_ritual(coordinator, nodes, initiator, erc20, flat_rate_fee_model): - authority, tx = initiate_ritual(coordinator, erc20, flat_rate_fee_model, initiator, nodes) +def test_initiate_ritual(coordinator, nodes, initiator, erc20, global_allow_list, flat_rate_fee_model): + authority, tx = initiate_ritual( + coordinator=coordinator, + erc20=erc20, + fee_model=flat_rate_fee_model, + authority=initiator, + nodes=nodes, + allow_logic=global_allow_list + ) events = list(coordinator.StartRitual.from_receipt(tx)) assert len(events) == 1 @@ -161,8 +205,15 @@ def test_provider_public_key(coordinator, nodes): assert coordinator.getProviderPublicKey(selected_provider, ritual_id) == public_key -def test_post_transcript(coordinator, nodes, initiator, erc20, flat_rate_fee_model): - initiate_ritual(coordinator, erc20, flat_rate_fee_model, initiator, nodes) +def test_post_transcript(coordinator, nodes, initiator, erc20, flat_rate_fee_model, global_allow_list): + initiate_ritual( + coordinator=coordinator, + erc20=erc20, + fee_model=flat_rate_fee_model, + authority=initiator, + nodes=nodes, + allow_logic=global_allow_list + ) transcript = os.urandom(transcript_size(len(nodes), len(nodes))) for node in nodes: @@ -186,18 +237,33 @@ def test_post_transcript(coordinator, nodes, initiator, erc20, flat_rate_fee_mod def test_post_transcript_but_not_part_of_ritual( - coordinator, nodes, initiator, erc20, flat_rate_fee_model + coordinator, nodes, initiator, erc20, flat_rate_fee_model, global_allow_list ): - initiate_ritual(coordinator, erc20, flat_rate_fee_model, initiator, nodes) + initiate_ritual( + coordinator=coordinator, + erc20=erc20, + fee_model=flat_rate_fee_model, + authority=initiator, + nodes=nodes, + allow_logic=global_allow_list + ) + transcript = os.urandom(transcript_size(len(nodes), len(nodes))) with ape.reverts("Participant not part of ritual"): coordinator.postTranscript(0, transcript, sender=initiator) def test_post_transcript_but_already_posted_transcript( - coordinator, nodes, initiator, erc20, flat_rate_fee_model + coordinator, nodes, initiator, erc20, flat_rate_fee_model, global_allow_list ): - initiate_ritual(coordinator, erc20, flat_rate_fee_model, initiator, nodes) + initiate_ritual( + coordinator=coordinator, + erc20=erc20, + fee_model=flat_rate_fee_model, + authority=initiator, + nodes=nodes, + allow_logic=global_allow_list + ) transcript = os.urandom(transcript_size(len(nodes), len(nodes))) coordinator.postTranscript(0, transcript, sender=nodes[0]) with ape.reverts("Node already posted transcript"): @@ -205,9 +271,16 @@ def test_post_transcript_but_already_posted_transcript( def test_post_transcript_but_not_waiting_for_transcripts( - coordinator, nodes, initiator, erc20, flat_rate_fee_model + coordinator, nodes, initiator, erc20, flat_rate_fee_model, global_allow_list ): - initiate_ritual(coordinator, erc20, flat_rate_fee_model, initiator, nodes) + initiate_ritual( + coordinator=coordinator, + erc20=erc20, + fee_model=flat_rate_fee_model, + authority=initiator, + nodes=nodes, + allow_logic=global_allow_list + ) transcript = os.urandom(transcript_size(len(nodes), len(nodes))) for node in nodes: coordinator.postTranscript(0, transcript, sender=node) @@ -216,8 +289,15 @@ def test_post_transcript_but_not_waiting_for_transcripts( coordinator.postTranscript(0, transcript, sender=nodes[1]) -def test_post_aggregation(coordinator, nodes, initiator, erc20, flat_rate_fee_model): - initiate_ritual(coordinator, erc20, flat_rate_fee_model, initiator, nodes) +def test_post_aggregation(coordinator, nodes, initiator, erc20, flat_rate_fee_model, global_allow_list): + initiate_ritual( + coordinator=coordinator, + erc20=erc20, + fee_model=flat_rate_fee_model, + authority=initiator, + nodes=nodes, + allow_logic=global_allow_list + ) transcript = os.urandom(transcript_size(len(nodes), len(nodes))) for node in nodes: coordinator.postTranscript(0, transcript, sender=node) From d4a1c401b69d871a4c84d1730ad821eb9b364e0a Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Fri, 18 Aug 2023 15:15:18 +0200 Subject: [PATCH 17/19] fixes typing bug for ECDSA usage in GlobalAllowList. --- contracts/contracts/coordination/GlobalAllowList.sol | 7 +++---- contracts/contracts/coordination/IRitualAuthorizer.sol | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index b63fe1b6..a2b01c47 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -4,6 +4,7 @@ import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "./IRitualAuthorizer.sol"; import "./Coordinator.sol"; + contract GlobalAllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { using ECDSA for bytes32; @@ -25,11 +26,9 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { function isAuthorized( uint32 ritualID, bytes memory evidence, - bytes memory digest + bytes32 digest ) public view override returns(bool) { - address recovered_address = keccak256(digest) - .toEthSignedMessageHash() - .recover(evidence); + address recovered_address = digest.toEthSignedMessageHash().recover(evidence); return authorizations[ritualID][recovered_address]; } diff --git a/contracts/contracts/coordination/IRitualAuthorizer.sol b/contracts/contracts/coordination/IRitualAuthorizer.sol index e624eff3..3c4524cb 100644 --- a/contracts/contracts/coordination/IRitualAuthorizer.sol +++ b/contracts/contracts/coordination/IRitualAuthorizer.sol @@ -4,6 +4,6 @@ interface IRitualAuthorizer { function isAuthorized( uint32 ritualID, bytes memory evidence, // signature - bytes memory digest // signed message hash + bytes32 digest // signed message hash ) external view returns(bool); } From ef675bfb680eacb656da5ecfd742c35da509ed77 Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Fri, 18 Aug 2023 15:15:50 +0200 Subject: [PATCH 18/19] tests for GlobalAllowList auth functions. --- tests/test_coordinator.py | 70 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 54b982f3..6dd9be84 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -3,6 +3,7 @@ import ape import pytest +from eth_account.messages import encode_defunct from web3 import Web3 TIMEOUT = 1000 @@ -329,3 +330,72 @@ def test_post_aggregation(coordinator, nodes, initiator, erc20, flat_rate_fee_mo event = events[0] assert event["ritualId"] == 0 assert event["successful"] + + +def test_authorize_using_global_allow_list( + coordinator, + nodes, + deployer, + initiator, + erc20, + flat_rate_fee_model, + global_allow_list +): + + initiate_ritual( + coordinator=coordinator, + erc20=erc20, + fee_model=flat_rate_fee_model, + authority=initiator, + nodes=nodes, + allow_logic=global_allow_list + ) + + global_allow_list.setCoordinator(coordinator.address, sender=deployer) + + # This block mocks the signature of a threshold decryption request + w3 = Web3() + data = os.urandom(32) + digest = Web3.keccak(data) + signable_message = encode_defunct(digest) + signed_digest = w3.eth.account.sign_message(signable_message, private_key=deployer.private_key) + signature = signed_digest.signature + + # Not authorized + assert not global_allow_list.isAuthorized(0, bytes(signature), bytes(digest)) + + # Negative test cases for authorization + with ape.reverts("Only ritual authority is permitted"): + global_allow_list.authorize(0, [deployer.address], sender=deployer) + + with ape.reverts("Only active rituals can add authorizations"): + global_allow_list.authorize(0, [deployer.address], sender=initiator) + + # Finalize ritual + transcript = os.urandom(transcript_size(len(nodes), len(nodes))) + for node in nodes: + coordinator.postTranscript(0, transcript, sender=node) + + aggregated = transcript + decryption_request_static_keys = [os.urandom(42) for _ in nodes] + dkg_public_key = (os.urandom(32), os.urandom(16)) + for i, node in enumerate(nodes): + coordinator.postAggregation(0, aggregated, dkg_public_key, decryption_request_static_keys[i], sender=node) + + # Actually authorize + global_allow_list.authorize(0, [deployer.address], sender=initiator) + + # Authorized + assert global_allow_list.isAuthorized(0, bytes(signature), bytes(digest)) + + # Deauthorize + global_allow_list.deauthorize(0, [deployer.address], sender=initiator) + assert not global_allow_list.isAuthorized(0, bytes(signature), bytes(digest)) + + # Reauthorize in batch + addresses_to_authorize = [deployer.address, initiator.address] + global_allow_list.authorize(0, addresses_to_authorize, sender=initiator) + signed_digest = w3.eth.account.sign_message(signable_message, private_key=initiator.private_key) + initiator_signature = signed_digest.signature + assert global_allow_list.isAuthorized(0, bytes(initiator_signature), bytes(digest)) + assert global_allow_list.isAuthorized(0, bytes(signature), bytes(digest)) From 06103326e0c130c40d1d4ffad6432d6c21ffe38d Mon Sep 17 00:00:00 2001 From: Kieran Prasch Date: Fri, 18 Aug 2023 15:29:46 +0200 Subject: [PATCH 19/19] Apply RFCs for PR #89 --- .../contracts/coordination/Coordinator.sol | 39 +++++++------------ .../coordination/GlobalAllowList.sol | 36 +++++++++-------- ...thorizer.sol => IEncryptionAuthorizer.sol} | 2 +- 3 files changed, 36 insertions(+), 41 deletions(-) rename contracts/contracts/coordination/{IRitualAuthorizer.sol => IEncryptionAuthorizer.sol} (85%) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index f4476ad4..b49a127b 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -9,7 +9,7 @@ import "./IFeeModel.sol"; import "./IReimbursementPool.sol"; import "../lib/BLS12381.sol"; import "../../threshold/IAccessControlApplication.sol"; -import "./IRitualAuthorizer.sol"; +import "./IEncryptionAuthorizer.sol"; /** * @title Coordinator @@ -59,7 +59,7 @@ contract Coordinator is AccessControlDefaultAdminRules { address authority; uint16 dkgSize; bool aggregationMismatch; - IRitualAuthorizer accessController; + IEncryptionAuthorizer accessController; BLS12381.G1Point publicKey; bytes aggregatedTranscript; Participant[] participant; @@ -103,7 +103,7 @@ contract Coordinator is AccessControlDefaultAdminRules { } function getRitualState(uint32 ritualId) external view returns (RitualState){ - // TODO: restrict to ritualID < rituals.length? + // TODO: restrict to ritualId < rituals.length? return getRitualState(rituals[ritualId]); } @@ -198,12 +198,12 @@ contract Coordinator is AccessControlDefaultAdminRules { return ritual.participant; } - function _initiateRitual( + function initiateRitual( address[] calldata providers, address authority, uint32 duration, - IRitualAuthorizer accessController - ) internal returns (uint32) { + IEncryptionAuthorizer accessController + ) external returns (uint32) { require(authority != address(0), "Invalid authority"); @@ -251,15 +251,6 @@ contract Coordinator is AccessControlDefaultAdminRules { return id; } - function initiateRitual( - address[] calldata providers, - address authority, - uint32 duration, - IRitualAuthorizer accessController - ) external returns (uint32) { - return _initiateRitual(providers, authority, duration, accessController); - } - function cohortFingerprint(address[] calldata nodes) public pure returns (bytes32) { return keccak256(abi.encode(nodes)); } @@ -391,26 +382,26 @@ contract Coordinator is AccessControlDefaultAdminRules { } function getParticipantFromProvider( - uint32 ritualID, + uint32 ritualId, address provider ) external view returns (Participant memory) { - return getParticipantFromProvider(rituals[ritualID], provider); + return getParticipantFromProvider(rituals[ritualId], provider); } - function processRitualPayment(uint32 ritualID, address[] calldata providers, uint32 duration) internal { + function processRitualPayment(uint32 ritualId, address[] calldata providers, uint32 duration) internal { uint256 ritualCost = feeModel.getRitualInitiationCost(providers, duration); if (ritualCost > 0) { totalPendingFees += ritualCost; - assert(pendingFees[ritualID] == 0); // TODO: This is an invariant, not sure if actually needed - pendingFees[ritualID] += ritualCost; + assert(pendingFees[ritualId] == 0); // TODO: This is an invariant, not sure if actually needed + pendingFees[ritualId] += ritualCost; IERC20 currency = IERC20(feeModel.currency()); currency.safeTransferFrom(msg.sender, address(this), ritualCost); // TODO: Define methods to manage these funds } } - function processPendingFee(uint32 ritualID) public { - Ritual storage ritual = rituals[ritualID]; + function processPendingFee(uint32 ritualId) public { + Ritual storage ritual = rituals[ritualId]; RitualState state = getRitualState(ritual); require( state == RitualState.TIMEOUT || @@ -418,12 +409,12 @@ contract Coordinator is AccessControlDefaultAdminRules { state == RitualState.FINALIZED, "Ritual is not ended" ); - uint256 pending = pendingFees[ritualID]; + uint256 pending = pendingFees[ritualId]; require(pending > 0, "No pending fees for this ritual"); // Finalize fees for this ritual totalPendingFees -= pending; - delete pendingFees[ritualID]; + delete pendingFees[ritualId]; // Transfer fees back to initiator if failed if (state == RitualState.TIMEOUT || state == RitualState.INVALID) { // Amount to refund depends on how much work nodes did for the ritual. diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index a2b01c47..24da89fd 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -1,11 +1,11 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/access/AccessControlDefaultAdminRules.sol"; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import "./IRitualAuthorizer.sol"; +import "./IEncryptionAuthorizer.sol"; import "./Coordinator.sol"; -contract GlobalAllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { +contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorizer { using ECDSA for bytes32; Coordinator public coordinator; @@ -15,40 +15,44 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IRitualAuthorizer { Coordinator _coordinator, address _admin ) AccessControlDefaultAdminRules(0, _admin) { + require(address(_coordinator) != address(0), "Coordinator cannot be zero address"); + require(_coordinator.numberOfRituals() >= 0, "Invalid coordinator"); coordinator = _coordinator; } + modifier onlyAuthority(uint32 ritualId) { + require(coordinator.getAuthority(ritualId) == msg.sender, + "Only ritual authority is permitted"); + _; + } + function setCoordinator(Coordinator _coordinator) public { require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Only admin can set coordinator"); coordinator = _coordinator; } function isAuthorized( - uint32 ritualID, + uint32 ritualId, bytes memory evidence, bytes32 digest ) public view override returns(bool) { address recovered_address = digest.toEthSignedMessageHash().recover(evidence); - return authorizations[ritualID][recovered_address]; + return authorizations[ritualId][recovered_address]; } - function authorize(uint32 ritualID, address[] calldata addresses) public { - require(coordinator.getAuthority(ritualID) == msg.sender, - "Only ritual authority is permitted"); - require(coordinator.isRitualFinalized(ritualID), + function authorize(uint32 ritualId, address[] calldata addresses) public onlyAuthority(ritualId) { + require(coordinator.isRitualFinalized(ritualId), "Only active rituals can add authorizations"); - for (uint i=0; i