From 8d2d3270af33dc5a778a9e06d26f3a7dc711db40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Mon, 7 Aug 2023 11:22:54 +0200 Subject: [PATCH 01/16] API for bidirectional mapping between public keys and successful rituals --- .../contracts/coordination/Coordinator.sol | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 69ec5906..c8d968b8 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -81,18 +81,18 @@ contract Coordinator is AccessControlDefaultAdminRules { bytes32 public constant INITIATOR_ROLE = keccak256("INITIATOR_ROLE"); - mapping(address => ParticipantKey[]) internal keysHistory; - IAccessControlApplication public immutable application; Ritual[] public rituals; uint32 public timeout; uint16 public maxDkgSize; bool public isInitiationPublic; - IFeeModel internal feeModel; // TODO: Consider making feeModel specific to each ritual - IReimbursementPool internal reimbursementPool; uint256 public totalPendingFees; mapping(uint256 => uint256) public pendingFees; + IFeeModel internal feeModel; // TODO: Consider making feeModel specific to each ritual + IReimbursementPool internal reimbursementPool; + mapping(address => ParticipantKey[]) internal keysHistory; + mapping(bytes32 => uint32) internal ritualPublicKeyRegistry; constructor( IAccessControlApplication _stakes, @@ -109,7 +109,6 @@ contract Coordinator is AccessControlDefaultAdminRules { } function getRitualState(uint32 ritualId) external view returns (RitualState) { - // TODO: restrict to ritualId < rituals.length? return getRitualState(rituals[ritualId]); } @@ -345,6 +344,12 @@ contract Coordinator is AccessControlDefaultAdminRules { ritual.totalAggregations++; if (ritual.totalAggregations == ritual.dkgSize) { processPendingFee(ritualId); + // Register ritualId + 1 to discern ritualID#0 from unregistered keys. + // See getRitualIdFromPublicKey() for inverse operation. + bytes32 registryKey = keccak256(abi.encodePacked( + BLS12381.g1PointToBytes(dkgPublicKey) + )); + ritualPublicKeyRegistry[registryKey] = ritualId + 1; emit EndRitual({ritualId: ritualId, successful: true}); // TODO: Consider including public key in event } @@ -353,6 +358,26 @@ contract Coordinator is AccessControlDefaultAdminRules { processReimbursement(initialGasLeft); } + function getRitualIdFromPublicKey( + BLS12381.G1Point memory dkgPublicKey + ) external view returns (uint32 ritualId) { + // If public key is not registered, result will produce underflow error + bytes32 registryKey = keccak256(abi.encodePacked( + BLS12381.g1PointToBytes(dkgPublicKey) + )); + return ritualPublicKeyRegistry[registryKey] - 1; + } + + function getPublicKeyFromRitualId(uint32 ritualId) + external view returns (BLS12381.G1Point memory dkgPublicKey) { + Ritual storage ritual = rituals[ritualId]; + require( + getRitualState(ritual) == RitualState.FINALIZED, + "Ritual not finalized" + ); + return ritual.publicKey; + } + function getParticipantFromProvider( Ritual storage ritual, address provider From b7f04c2b2cf4302040a4efbf01fa432b318cebfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Mon, 7 Aug 2023 12:18:45 +0200 Subject: [PATCH 02/16] Invalidate stored public key if ritual fails Closes #78 --- 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 c8d968b8..1012780b 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -337,6 +337,7 @@ contract Coordinator is AccessControlDefaultAdminRules { keccak256(ritual.aggregatedTranscript) != aggregatedTranscriptDigest ) { ritual.aggregationMismatch = true; + delete ritual.publicKey; emit EndRitual({ritualId: ritualId, successful: false}); } From 5f813801978442ffc8fed90a0ea910ffd6ee00e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Mon, 7 Aug 2023 14:18:17 +0200 Subject: [PATCH 03/16] Store threshold in ritual struct, but not allow initiator to set it for the moment We currently define it as the lowest value that produces a threshold set that's strictly greater than the 50% of the overall size. See https://github.com/nucypher/nucypher/issues/3095 --- contracts/contracts/coordination/Coordinator.sol | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 1012780b..a409b836 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -56,16 +56,18 @@ contract Coordinator is AccessControlDefaultAdminRules { bytes decryptionRequestStaticKey; } - // TODO: Optimize layout struct Ritual { address initiator; uint32 initTimestamp; uint32 endTimestamp; uint16 totalTranscripts; uint16 totalAggregations; + address authority; uint16 dkgSize; + uint16 threshold; bool aggregationMismatch; + IEncryptionAuthorizer accessController; BLS12381.G1Point publicKey; bytes aggregatedTranscript; @@ -206,6 +208,11 @@ contract Coordinator is AccessControlDefaultAdminRules { return ritual.participant; } + function getThresholdForRitualSize(uint16 size) public pure returns (uint16) { + return 1 + size / 2; + // Alternatively: 1 + 2*size/3 (for >66.6%) or 1 + 3*size/5 (for >60%) + } + function initiateRitual( address[] calldata providers, address authority, @@ -218,8 +225,7 @@ contract Coordinator is AccessControlDefaultAdminRules { isInitiationPublic || hasRole(INITIATOR_ROLE, msg.sender), "Sender can't initiate ritual" ); - // TODO: Validate service fees, expiration dates, threshold - uint256 length = providers.length; + uint16 length = uint16(providers.length); require(2 <= length && length <= maxDkgSize, "Invalid number of nodes"); require(duration > 0, "Invalid ritual duration"); // TODO: We probably want to restrict it more @@ -227,7 +233,8 @@ contract Coordinator is AccessControlDefaultAdminRules { Ritual storage ritual = rituals.push(); ritual.initiator = msg.sender; ritual.authority = authority; - ritual.dkgSize = uint16(length); + ritual.dkgSize = length; + ritual.threshold = getThresholdForRitualSize(length); ritual.initTimestamp = uint32(block.timestamp); ritual.endTimestamp = ritual.initTimestamp + duration; ritual.accessController = accessController; From 57cfa1c90f8f8909a183beeb7ef5b1ebf7c4349d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Mon, 7 Aug 2023 14:18:45 +0200 Subject: [PATCH 04/16] Define minimum ritual duration as 24 hours. See #106 --- 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 a409b836..1e932c48 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -227,7 +227,7 @@ contract Coordinator is AccessControlDefaultAdminRules { ); uint16 length = uint16(providers.length); require(2 <= length && length <= maxDkgSize, "Invalid number of nodes"); - require(duration > 0, "Invalid ritual duration"); // TODO: We probably want to restrict it more + require(duration >= 24 hours, "Invalid ritual duration"); // TODO: Define minimum duration #106 uint32 id = uint32(rituals.length); Ritual storage ritual = rituals.push(); From 675219de1c4ed3846b1a33dfd0d9c9f6ec518d29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Mon, 7 Aug 2023 16:48:18 +0200 Subject: [PATCH 05/16] Additional tests for coordinator --- tests/test_coordinator.py | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 2e40bdc8..c1f2a971 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -10,7 +10,7 @@ MAX_DKG_SIZE = 4 FEE_RATE = 42 ERC20_SUPPLY = 10**24 -DURATION = 1234 +DURATION = 48 * 60 * 60 RitualState = IntEnum( "RitualState", @@ -168,14 +168,29 @@ def test_initiate_ritual( allow_logic=global_allow_list, ) + ritualID = 0 events = coordinator.StartRitual.from_receipt(tx) assert len(events) == 1 event = events[0] - assert event["ritualId"] == 0 + assert event["ritualId"] == ritualID assert event["authority"] == authority assert event["participants"] == tuple(n.address.lower() for n in nodes) assert coordinator.getRitualState(0) == RitualState.AWAITING_TRANSCRIPTS + + ritual_struct = coordinator.rituals(ritualID) + assert ritual_struct[0] == initiator + init, end = ritual_struct[1], ritual_struct[2] + assert end - init == DURATION + total_transcripts, total_aggregations = ritual_struct[3], ritual_struct[4] + assert total_transcripts == total_aggregations == 0 + assert ritual_struct[5] == authority + assert ritual_struct[6] == len(nodes) + assert ritual_struct[7] == 1 + len(nodes) // 2 # threshold + assert not ritual_struct[8] # aggregationMismatch + assert ritual_struct[9] == global_allow_list.address # accessController + assert ritual_struct[10] == (b"\x00" * 32, b"\x00" * 16) # publicKey + assert not ritual_struct[11] # aggregatedTranscript def test_provider_public_key(coordinator, nodes): @@ -289,35 +304,40 @@ def test_post_aggregation( nodes=nodes, allow_logic=global_allow_list, ) + ritualID = 0 transcript = os.urandom(transcript_size(len(nodes), len(nodes))) for node in nodes: - coordinator.postTranscript(0, transcript, sender=node) + coordinator.postTranscript(ritualID, transcript, sender=node) aggregated = transcript # has the same size as 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): - assert coordinator.getRitualState(0) == RitualState.AWAITING_AGGREGATIONS + assert coordinator.getRitualState(ritualID) == RitualState.AWAITING_AGGREGATIONS tx = coordinator.postAggregation( - 0, aggregated, dkg_public_key, decryption_request_static_keys[i], sender=node + ritualID, aggregated, dkg_public_key, decryption_request_static_keys[i], + sender=node ) events = coordinator.AggregationPosted.from_receipt(tx) assert events == [ coordinator.AggregationPosted( - ritualId=0, node=node, aggregatedTranscriptDigest=Web3.keccak(aggregated) + ritualId=ritualID, node=node, aggregatedTranscriptDigest=Web3.keccak(aggregated) ) ] - participants = coordinator.getParticipants(0) + participants = coordinator.getParticipants(ritualID) for i, participant in enumerate(participants): assert participant.aggregated assert participant.decryptionRequestStaticKey == decryption_request_static_keys[i] - assert coordinator.getRitualState(0) == RitualState.FINALIZED + assert coordinator.getRitualState(ritualID) == RitualState.FINALIZED events = coordinator.EndRitual.from_receipt(tx) - assert events == [coordinator.EndRitual(ritualId=0, successful=True)] + assert events == [coordinator.EndRitual(ritualId=ritualID, successful=True)] + retrieved_public_key = coordinator.getPublicKeyFromRitualId(ritualID) + assert retrieved_public_key == dkg_public_key + assert coordinator.getRitualIdFromPublicKey(dkg_public_key) == ritualID def test_authorize_using_global_allow_list( coordinator, nodes, deployer, initiator, erc20, flat_rate_fee_model, global_allow_list From c4848aea3709f0d8efe60cb6086ab900220a1816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Fri, 11 Aug 2023 12:28:26 +0200 Subject: [PATCH 06/16] It's FlatRate, not FlateRate.... --- .../{FlateRateFeeModel.sol => FlatRateFeeModel.sol} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename contracts/contracts/coordination/{FlateRateFeeModel.sol => FlatRateFeeModel.sol} (97%) diff --git a/contracts/contracts/coordination/FlateRateFeeModel.sol b/contracts/contracts/coordination/FlatRateFeeModel.sol similarity index 97% rename from contracts/contracts/coordination/FlateRateFeeModel.sol rename to contracts/contracts/coordination/FlatRateFeeModel.sol index 5b49eee4..1510b549 100644 --- a/contracts/contracts/coordination/FlateRateFeeModel.sol +++ b/contracts/contracts/coordination/FlatRateFeeModel.sol @@ -8,7 +8,7 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; /** * @title FlatRateFeeModel - * @notice FlateRateFeeModel + * @notice FlatRateFeeModel */ contract FlatRateFeeModel is IFeeModel { IERC20 public immutable currency; From 6f3d2d261e0a0e21e383eeaef09a74278e9f2f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Fri, 11 Aug 2023 12:33:58 +0200 Subject: [PATCH 07/16] Simplify deployment by Coordinator extending from FlatRateFeeModel We can revert to a composition model later, but for the moment this will make our lives easier --- .../contracts/coordination/Coordinator.sol | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 1e932c48..0397baf4 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/access/AccessControlDefaultAdminRules.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import "./IFeeModel.sol"; +import "./FlatRateFeeModel.sol"; import "./IReimbursementPool.sol"; import "../lib/BLS12381.sol"; import "../../threshold/IAccessControlApplication.sol"; @@ -13,9 +13,9 @@ import "./IEncryptionAuthorizer.sol"; /** * @title Coordinator - * @notice Coordination layer for DKG-TDec + * @notice Coordination layer for Threshold Access Control (TACo 🌮) */ -contract Coordinator is AccessControlDefaultAdminRules { +contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { // Ritual event StartRitual(uint32 indexed ritualId, address indexed authority, address[] participants); event StartAggregationRound(uint32 indexed ritualId); @@ -101,13 +101,15 @@ contract Coordinator is AccessControlDefaultAdminRules { uint32 _timeout, uint16 _maxDkgSize, address _admin, - IFeeModel _feeModel - ) AccessControlDefaultAdminRules(0, _admin) { - require(address(_feeModel.stakes()) == address(_stakes), "Invalid stakes for fee model"); + IERC20 _currency, + uint256 _feeRatePerSecond + ) + AccessControlDefaultAdminRules(0, _admin) + FlatRateFeeModel(_currency, _feeRatePerSecond, _stakes) + { application = _stakes; timeout = _timeout; maxDkgSize = _maxDkgSize; - feeModel = IFeeModel(_feeModel); } function getRitualState(uint32 ritualId) external view returns (RitualState) { @@ -413,12 +415,11 @@ contract Coordinator is AccessControlDefaultAdminRules { address[] calldata providers, uint32 duration ) internal { - uint256 ritualCost = feeModel.getRitualInitiationCost(providers, duration); + uint256 ritualCost = 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; - IERC20 currency = IERC20(feeModel.currency()); currency.safeTransferFrom(msg.sender, address(this), ritualCost); // TODO: Define methods to manage these funds } @@ -447,7 +448,6 @@ contract Coordinator is AccessControlDefaultAdminRules { uint256 expectedTransactions = 2 * ritual.dkgSize; uint256 consumedFee = (pending * executedTransactions) / expectedTransactions; uint256 refundableFee = pending - consumedFee; - IERC20 currency = IERC20(feeModel.currency()); currency.transferFrom(address(this), ritual.initiator, refundableFee); } } From dae568aa04304fde7ea768622a7b0d017dc7cdcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Fri, 11 Aug 2023 12:41:53 +0200 Subject: [PATCH 08/16] Further simplify FeeModel contracts since Coordinator now extends them --- contracts/contracts/coordination/Coordinator.sol | 2 +- contracts/contracts/coordination/FlatRateFeeModel.sol | 7 ++----- contracts/contracts/coordination/IFeeModel.sol | 3 --- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 0397baf4..4da3fe8d 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -105,7 +105,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { uint256 _feeRatePerSecond ) AccessControlDefaultAdminRules(0, _admin) - FlatRateFeeModel(_currency, _feeRatePerSecond, _stakes) + FlatRateFeeModel(_currency, _feeRatePerSecond) { application = _stakes; timeout = _timeout; diff --git a/contracts/contracts/coordination/FlatRateFeeModel.sol b/contracts/contracts/coordination/FlatRateFeeModel.sol index 1510b549..7878db31 100644 --- a/contracts/contracts/coordination/FlatRateFeeModel.sol +++ b/contracts/contracts/coordination/FlatRateFeeModel.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.0; import "./IFeeModel.sol"; -import "../../threshold/IAccessControlApplication.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; /** @@ -13,18 +12,16 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; contract FlatRateFeeModel is IFeeModel { IERC20 public immutable currency; uint256 public immutable feeRatePerSecond; - IAccessControlApplication public immutable stakes; - constructor(IERC20 _currency, uint256 _feeRatePerSecond, address _stakes) { + constructor(IERC20 _currency, uint256 _feeRatePerSecond) { currency = _currency; feeRatePerSecond = _feeRatePerSecond; - stakes = IAccessControlApplication(_stakes); } function getRitualInitiationCost( address[] calldata providers, uint32 duration - ) external view returns (uint256) { + ) public view returns(uint256) { uint256 size = providers.length; require(duration > 0, "Invalid ritual duration"); require(size > 0, "Invalid ritual size"); diff --git a/contracts/contracts/coordination/IFeeModel.sol b/contracts/contracts/coordination/IFeeModel.sol index 5bd319f2..c53b4d2f 100644 --- a/contracts/contracts/coordination/IFeeModel.sol +++ b/contracts/contracts/coordination/IFeeModel.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; -import "../../threshold/IAccessControlApplication.sol"; /** * @title IFeeModel @@ -12,8 +11,6 @@ import "../../threshold/IAccessControlApplication.sol"; interface IFeeModel { function currency() external view returns (IERC20); - function stakes() external view returns (IAccessControlApplication); - function getRitualInitiationCost( address[] calldata providers, uint32 duration From 168c8cabb1aff829c189fc325ead67631afe5248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Fri, 11 Aug 2023 12:46:20 +0200 Subject: [PATCH 09/16] Adapt tests to new FeeModel situation --- tests/test_coordinator.py | 44 +++++++++++---------------------------- 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index c1f2a971..c9299a07 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -72,22 +72,15 @@ def erc20(project, initiator): @pytest.fixture() -def flat_rate_fee_model(project, deployer, stake_info, erc20): - contract = project.FlatRateFeeModel.deploy( - erc20.address, FEE_RATE, stake_info.address, sender=deployer - ) - return contract - - -@pytest.fixture() -def coordinator(project, deployer, stake_info, flat_rate_fee_model, initiator): +def coordinator(project, deployer, stake_info, erc20, initiator): admin = deployer contract = project.Coordinator.deploy( stake_info.address, TIMEOUT, MAX_DKG_SIZE, admin, - flat_rate_fee_model.address, + erc20.address, + FEE_RATE, sender=deployer, ) contract.grantRole(contract.INITIATOR_ROLE(), initiator, sender=admin) @@ -144,11 +137,11 @@ def test_invalid_initiate_ritual(coordinator, nodes, accounts, initiator, global ) -def initiate_ritual(coordinator, erc20, fee_model, allow_logic, authority, nodes): +def initiate_ritual(coordinator, erc20, allow_logic, authority, nodes): for node in nodes: public_key = gen_public_key() coordinator.setProviderPublicKey(public_key, sender=node) - cost = fee_model.getRitualInitiationCost(nodes, DURATION) + cost = coordinator.getRitualInitiationCost(nodes, DURATION) erc20.approve(coordinator.address, cost, sender=authority) tx = coordinator.initiateRitual( nodes, authority, DURATION, allow_logic.address, sender=authority @@ -156,13 +149,10 @@ def initiate_ritual(coordinator, erc20, fee_model, allow_logic, authority, nodes return authority, tx -def test_initiate_ritual( - coordinator, nodes, initiator, erc20, global_allow_list, flat_rate_fee_model -): +def test_initiate_ritual(coordinator, nodes, initiator, erc20, global_allow_list): authority, tx = initiate_ritual( coordinator=coordinator, erc20=erc20, - fee_model=flat_rate_fee_model, authority=initiator, nodes=nodes, allow_logic=global_allow_list, @@ -207,13 +197,10 @@ 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, global_allow_list -): +def test_post_transcript(coordinator, nodes, initiator, erc20, global_allow_list): initiate_ritual( coordinator=coordinator, erc20=erc20, - fee_model=flat_rate_fee_model, authority=initiator, nodes=nodes, allow_logic=global_allow_list, @@ -241,12 +228,11 @@ def test_post_transcript( def test_post_transcript_but_not_part_of_ritual( - coordinator, nodes, initiator, erc20, flat_rate_fee_model, global_allow_list + coordinator, nodes, initiator, erc20, global_allow_list ): initiate_ritual( coordinator=coordinator, erc20=erc20, - fee_model=flat_rate_fee_model, authority=initiator, nodes=nodes, allow_logic=global_allow_list, @@ -258,12 +244,11 @@ def test_post_transcript_but_not_part_of_ritual( def test_post_transcript_but_already_posted_transcript( - coordinator, nodes, initiator, erc20, flat_rate_fee_model, global_allow_list + coordinator, nodes, initiator, erc20, global_allow_list ): initiate_ritual( coordinator=coordinator, erc20=erc20, - fee_model=flat_rate_fee_model, authority=initiator, nodes=nodes, allow_logic=global_allow_list, @@ -275,12 +260,11 @@ 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, global_allow_list + coordinator, nodes, initiator, erc20, global_allow_list ): initiate_ritual( coordinator=coordinator, erc20=erc20, - fee_model=flat_rate_fee_model, authority=initiator, nodes=nodes, allow_logic=global_allow_list, @@ -293,13 +277,10 @@ 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, global_allow_list -): +def test_post_aggregation(coordinator, nodes, initiator, erc20, global_allow_list): initiate_ritual( coordinator=coordinator, erc20=erc20, - fee_model=flat_rate_fee_model, authority=initiator, nodes=nodes, allow_logic=global_allow_list, @@ -340,13 +321,12 @@ def test_post_aggregation( assert coordinator.getRitualIdFromPublicKey(dkg_public_key) == ritualID def test_authorize_using_global_allow_list( - coordinator, nodes, deployer, initiator, erc20, flat_rate_fee_model, global_allow_list + coordinator, nodes, deployer, initiator, erc20, global_allow_list ): initiate_ritual( coordinator=coordinator, erc20=erc20, - fee_model=flat_rate_fee_model, authority=initiator, nodes=nodes, allow_logic=global_allow_list, From 3e5278440b3afece1b3a22755144f84d94942430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 22 Aug 2023 11:18:48 +0200 Subject: [PATCH 10/16] Pass raw signature input when checking encryption authorization, instead of hash --- contracts/contracts/coordination/GlobalAllowList.sol | 3 ++- .../contracts/coordination/IEncryptionAuthorizer.sol | 6 +++--- tests/test_coordinator.py | 8 ++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index 4813b7f7..7fb1c84f 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -35,8 +35,9 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorize function isAuthorized( uint32 ritualId, bytes memory evidence, - bytes32 digest + bytes memory ciphertextHeader ) public view override returns (bool) { + bytes32 digest = keccak256(ciphertextHeader); address recoveredAddress = digest.toEthSignedMessageHash().recover(evidence); return authorizations[ritualId][recoveredAddress]; } diff --git a/contracts/contracts/coordination/IEncryptionAuthorizer.sol b/contracts/contracts/coordination/IEncryptionAuthorizer.sol index 4ff32d40..454c8ac5 100644 --- a/contracts/contracts/coordination/IEncryptionAuthorizer.sol +++ b/contracts/contracts/coordination/IEncryptionAuthorizer.sol @@ -2,8 +2,8 @@ pragma solidity ^0.8.0; interface IEncryptionAuthorizer { function isAuthorized( - uint32 ritualID, - bytes memory evidence, // signature - bytes32 digest // signed message hash + uint32 ritualId, + bytes memory evidence, // supporting evidence for authorization + bytes memory ciphertextHeader // data to be signed by authorized ) external view returns (bool); } diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index c9299a07..4ee1c3fa 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -369,16 +369,16 @@ def test_authorize_using_global_allow_list( global_allow_list.authorize(0, [deployer.address], sender=initiator) # Authorized - assert global_allow_list.isAuthorized(0, bytes(signature), bytes(digest)) + assert global_allow_list.isAuthorized(0, bytes(signature), bytes(data)) # Deauthorize global_allow_list.deauthorize(0, [deployer.address], sender=initiator) - assert not global_allow_list.isAuthorized(0, bytes(signature), bytes(digest)) + assert not global_allow_list.isAuthorized(0, bytes(signature), bytes(data)) # 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)) + assert global_allow_list.isAuthorized(0, bytes(initiator_signature), bytes(data)) + assert global_allow_list.isAuthorized(0, bytes(signature), bytes(data)) From ff0fee6fd4f9f4c135f7f9d8e01541e8029b38b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 22 Aug 2023 11:32:22 +0200 Subject: [PATCH 11/16] Optimize lookup mapping in GlobalAllowList Closes #97 --- .../coordination/GlobalAllowList.sol | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index 7fb1c84f..6c958118 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -8,7 +8,7 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorize using ECDSA for bytes32; Coordinator public coordinator; - mapping(uint256 => mapping(address => bool)) public authorizations; + mapping(bytes32 => bool) authorizations; constructor( Coordinator _coordinator, @@ -32,39 +32,49 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorize coordinator = _coordinator; } + function lookupKey(uint32 ritualId, address encryptor) internal pure returns (bytes32) { + return keccak256(abi.encodePacked(ritualId, encryptor)); + } + + function isAddressAuthorized(uint32 ritualId, address encryptor) public view returns (bool) { + return authorizations[lookupKey(ritualId, encryptor)]; + } + function isAuthorized( uint32 ritualId, bytes memory evidence, bytes memory ciphertextHeader - ) public view override returns (bool) { + ) external view override returns (bool) { bytes32 digest = keccak256(ciphertextHeader); address recoveredAddress = digest.toEthSignedMessageHash().recover(evidence); - return authorizations[ritualId][recoveredAddress]; + return isAddressAuthorized(ritualId, recoveredAddress); } function authorize( uint32 ritualId, address[] calldata addresses - ) public onlyAuthority(ritualId) { - require( - coordinator.isRitualFinalized(ritualId), - "Only active rituals can add authorizations" - ); - for (uint256 i = 0; i < addresses.length; i++) { - authorizations[ritualId][addresses[i]] = true; - } + ) external onlyAuthority(ritualId) { + setAuthorizations(ritualId, addresses, true); } function deauthorize( uint32 ritualId, address[] calldata addresses - ) public onlyAuthority(ritualId) { + ) external onlyAuthority(ritualId) { + setAuthorizations(ritualId, addresses, false); + } + + function setAuthorizations( + uint32 ritualId, + address[] calldata addresses, + bool value + ) internal { require( coordinator.isRitualFinalized(ritualId), "Only active rituals can add authorizations" ); for (uint256 i = 0; i < addresses.length; i++) { - authorizations[ritualId][addresses[i]] = false; + authorizations[lookupKey(ritualId, addresses[i])] = value; } } } From d05d20210253796ea5139da4c72b4cb92f1a40af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 22 Aug 2023 16:15:53 +0200 Subject: [PATCH 12/16] Further checks when setting Coordinator in AllowList contracts --- contracts/contracts/coordination/GlobalAllowList.sol | 9 ++++----- tests/test_coordinator.py | 4 ++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index 6c958118..401b5541 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -14,9 +14,7 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorize 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; + setCoordinator(_coordinator); } modifier onlyAuthority(uint32 ritualId) { @@ -27,8 +25,9 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorize _; } - function setCoordinator(Coordinator _coordinator) public { - require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Only admin can set coordinator"); + function setCoordinator(Coordinator _coordinator) public onlyRole(DEFAULT_ADMIN_ROLE){ + require(address(_coordinator) != address(0), "Coordinator cannot be zero address"); + require(_coordinator.numberOfRituals() >= 0, "Invalid coordinator"); coordinator = _coordinator; } diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 4ee1c3fa..aaabd785 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -332,6 +332,10 @@ def test_authorize_using_global_allow_list( allow_logic=global_allow_list, ) + access_control_error_message = f"AccessControl: account {initiator.address.lower()} is missing role 0x{'00'*32}" + with ape.reverts(access_control_error_message): + global_allow_list.setCoordinator(coordinator.address, sender=initiator) + global_allow_list.setCoordinator(coordinator.address, sender=deployer) # This block mocks the signature of a threshold decryption request From 7de9a24e6b2c6e5c6491fbd67c6c79f4d4c45251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 23 Aug 2023 09:36:01 +0200 Subject: [PATCH 13/16] Define TREASURY_ROLE to manage DKG fees --- .../contracts/coordination/Coordinator.sol | 14 ++++++- tests/test_coordinator.py | 42 +++++++++++++++++-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 4da3fe8d..394ab6a1 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -82,6 +82,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { using SafeERC20 for IERC20; bytes32 public constant INITIATOR_ROLE = keccak256("INITIATOR_ROLE"); + bytes32 public constant TREASURY_ROLE = keccak256("TREASURY_ROLE"); IAccessControlApplication public immutable application; @@ -421,7 +422,6 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { assert(pendingFees[ritualId] == 0); // TODO: This is an invariant, not sure if actually needed pendingFees[ritualId] += ritualCost; currency.safeTransferFrom(msg.sender, address(this), ritualCost); - // TODO: Define methods to manage these funds } } @@ -448,7 +448,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { uint256 expectedTransactions = 2 * ritual.dkgSize; uint256 consumedFee = (pending * executedTransactions) / expectedTransactions; uint256 refundableFee = pending - consumedFee; - currency.transferFrom(address(this), ritual.initiator, refundableFee); + currency.safeTransfer(ritual.initiator, refundableFee); } } @@ -463,4 +463,14 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { } } } + + function withdrawTokens(IERC20 token, uint256 amount) external onlyRole(TREASURY_ROLE) { + if (address(token) == address(currency)){ + require( + amount <= token.balanceOf(address(this)) - totalPendingFees, + "Can't withdraw pending fees" + ); + } + token.safeTransfer(msg.sender, amount); + } } diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index aaabd785..956b4b55 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -36,6 +36,11 @@ def gen_public_key(): return (os.urandom(32), os.urandom(32), os.urandom(32)) +def access_control_error_message(address, role=None): + role = Web3.to_hex(role or b'\x00'*32) + return f"AccessControl: account {address.lower()} is missing role {role}" + + @pytest.fixture(scope="module") def nodes(accounts): return sorted(accounts[:MAX_DKG_SIZE], key=lambda x: x.address.lower()) @@ -55,6 +60,13 @@ def deployer(accounts): return accounts[deployer_index] +@pytest.fixture(scope="module") +def treasury(accounts): + treasury_index = MAX_DKG_SIZE + 3 + assert len(accounts) >= treasury_index + return accounts[treasury_index] + + @pytest.fixture() def stake_info(project, deployer, nodes): contract = project.StakeInfo.deploy([deployer], sender=deployer) @@ -149,7 +161,7 @@ def initiate_ritual(coordinator, erc20, allow_logic, authority, nodes): return authority, tx -def test_initiate_ritual(coordinator, nodes, initiator, erc20, global_allow_list): +def test_initiate_ritual(coordinator, nodes, initiator, erc20, global_allow_list, deployer, treasury): authority, tx = initiate_ritual( coordinator=coordinator, erc20=erc20, @@ -182,6 +194,18 @@ def test_initiate_ritual(coordinator, nodes, initiator, erc20, global_allow_list assert ritual_struct[10] == (b"\x00" * 32, b"\x00" * 16) # publicKey assert not ritual_struct[11] # aggregatedTranscript + fee = coordinator.getRitualInitiationCost(nodes, DURATION) + assert erc20.balanceOf(coordinator) == fee + assert coordinator.totalPendingFees() == fee + assert coordinator.pendingFees(ritualID) == fee + + with ape.reverts(access_control_error_message(treasury.address, coordinator.TREASURY_ROLE())): + coordinator.withdrawTokens(erc20.address, 1, sender=treasury) + + coordinator.grantRole(coordinator.TREASURY_ROLE(), treasury, sender=deployer) + with ape.reverts("Can't withdraw pending fees"): + coordinator.withdrawTokens(erc20.address, 1, sender=treasury) + def test_provider_public_key(coordinator, nodes): selected_provider = nodes[0] @@ -277,7 +301,7 @@ def test_post_transcript_but_not_waiting_for_transcripts( coordinator.postTranscript(0, transcript, sender=nodes[1]) -def test_post_aggregation(coordinator, nodes, initiator, erc20, global_allow_list): +def test_post_aggregation(coordinator, nodes, initiator, erc20, global_allow_list, treasury, deployer): initiate_ritual( coordinator=coordinator, erc20=erc20, @@ -320,6 +344,17 @@ def test_post_aggregation(coordinator, nodes, initiator, erc20, global_allow_lis assert retrieved_public_key == dkg_public_key assert coordinator.getRitualIdFromPublicKey(dkg_public_key) == ritualID + fee = coordinator.getRitualInitiationCost(nodes, DURATION) + assert erc20.balanceOf(coordinator) == fee + assert coordinator.totalPendingFees() == 0 + assert coordinator.pendingFees(ritualID) == 0 + + coordinator.grantRole(coordinator.TREASURY_ROLE(), treasury, sender=deployer) + with ape.reverts("Can't withdraw pending fees"): + coordinator.withdrawTokens(erc20.address, fee + 1, sender=treasury) + coordinator.withdrawTokens(erc20.address, fee, sender=treasury) + + def test_authorize_using_global_allow_list( coordinator, nodes, deployer, initiator, erc20, global_allow_list ): @@ -332,8 +367,7 @@ def test_authorize_using_global_allow_list( allow_logic=global_allow_list, ) - access_control_error_message = f"AccessControl: account {initiator.address.lower()} is missing role 0x{'00'*32}" - with ape.reverts(access_control_error_message): + with ape.reverts(access_control_error_message(initiator.address, role=0)): global_allow_list.setCoordinator(coordinator.address, sender=initiator) global_allow_list.setCoordinator(coordinator.address, sender=deployer) From 76c1af529d11c72a1af149c69bdc856a7efd1af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 23 Aug 2023 11:19:14 +0200 Subject: [PATCH 14/16] Change fee refund approach to everything minus cost for a day --- contracts/contracts/coordination/Coordinator.sol | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 394ab6a1..5beb08cc 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -442,12 +442,10 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { 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. + // Refund everything minus cost of renting cohort for a day // TODO: Validate if this is enough to remove griefing attacks - uint256 executedTransactions = ritual.totalTranscripts + ritual.totalAggregations; - uint256 expectedTransactions = 2 * ritual.dkgSize; - uint256 consumedFee = (pending * executedTransactions) / expectedTransactions; - uint256 refundableFee = pending - consumedFee; + uint256 duration = ritual.endTimestamp - ritual.initTimestamp; + uint256 refundableFee = pending * (duration - 1 days) / duration; currency.safeTransfer(ritual.initiator, refundableFee); } } From e36477f4f89999b957f39c5a7901e220479c5b2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 23 Aug 2023 11:22:51 +0200 Subject: [PATCH 15/16] Remove outdated TODOs --- contracts/contracts/coordination/Coordinator.sol | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 5beb08cc..8cb09626 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -19,7 +19,6 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { // Ritual event StartRitual(uint32 indexed ritualId, address indexed authority, address[] participants); event StartAggregationRound(uint32 indexed ritualId); - // TODO: Do we want the public key here? If so, we want 2 events or do we reuse this event? event EndRitual(uint32 indexed ritualId, bool successful); // Node @@ -52,7 +51,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { struct Participant { address provider; bool aggregated; - bytes transcript; // TODO: Consider event processing complexity vs storage cost + bytes transcript; bytes decryptionRequestStaticKey; } @@ -138,11 +137,11 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { return RitualState.AWAITING_AGGREGATIONS; // solhint-disable-next-line no-empty-blocks } else { - // TODO: Is it possible to reach this state? + // It shouldn't be possible to reach this state: // - No public key // - All transcripts and all aggregations // - Still within the deadline - revert("Invalid ritual state"); + assert(false); } } @@ -260,7 +259,6 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { processRitualPayment(id, providers, duration); - // TODO: Include cohort fingerprint in StartRitual event? emit StartRitual(id, ritual.authority, providers); return id; } @@ -288,7 +286,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { // Nodes commit to their transcript bytes32 transcriptDigest = keccak256(transcript); - participant.transcript = transcript; // TODO: ??? + participant.transcript = transcript; emit TranscriptPosted(ritualId, provider, transcriptDigest); ritual.totalTranscripts++; @@ -362,7 +360,6 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { )); ritualPublicKeyRegistry[registryKey] = ritualId + 1; emit EndRitual({ritualId: ritualId, successful: true}); - // TODO: Consider including public key in event } } @@ -419,8 +416,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { uint256 ritualCost = 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; + pendingFees[ritualId] = ritualCost; currency.safeTransferFrom(msg.sender, address(this), ritualCost); } } @@ -452,7 +448,6 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { function processReimbursement(uint256 initialGasLeft) internal { if (address(reimbursementPool) != address(0)) { - // TODO: Consider defining a method uint256 gasUsed = initialGasLeft - gasleft(); try reimbursementPool.refund(gasUsed, msg.sender) { return; From 2263b47d295a66907b9933e24ba09f6b131c371d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Fri, 25 Aug 2023 12:01:38 +0200 Subject: [PATCH 16/16] Solhint changes --- .github/workflows/main.yaml | 4 +- .../contracts/coordination/Coordinator.sol | 37 ++++++++----------- .../coordination/FlatRateFeeModel.sol | 2 +- .../coordination/GlobalAllowList.sol | 10 ++--- .../coordination/IEncryptionAuthorizer.sol | 4 +- 5 files changed, 23 insertions(+), 34 deletions(-) diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 285b6060..37dbf131 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -67,10 +67,10 @@ jobs: - name: Install Solidity plugin for Prettier run: npm install prettier-plugin-solidity - - name: Solidty Lint + - name: Solidity Lint run: solhint 'contracts/**/*.sol' - - name: Python lint + - name: Python Lint uses: cclauss/GitHub-Action-for-pylint@0.7.0 continue-on-error: true with: diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 8cb09626..95e4f309 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -61,12 +61,12 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { uint32 endTimestamp; uint16 totalTranscripts; uint16 totalAggregations; - + // address authority; uint16 dkgSize; uint16 threshold; bool aggregationMismatch; - + // IEncryptionAuthorizer accessController; BLS12381.G1Point publicKey; bytes aggregatedTranscript; @@ -103,10 +103,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { address _admin, IERC20 _currency, uint256 _feeRatePerSecond - ) - AccessControlDefaultAdminRules(0, _admin) - FlatRateFeeModel(_currency, _feeRatePerSecond) - { + ) AccessControlDefaultAdminRules(0, _admin) FlatRateFeeModel(_currency, _feeRatePerSecond) { application = _stakes; timeout = _timeout; maxDkgSize = _maxDkgSize; @@ -229,7 +226,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { ); uint16 length = uint16(providers.length); require(2 <= length && length <= maxDkgSize, "Invalid number of nodes"); - require(duration >= 24 hours, "Invalid ritual duration"); // TODO: Define minimum duration #106 + require(duration >= 24 hours, "Invalid ritual duration"); // TODO: Define minimum duration #106 uint32 id = uint32(rituals.length); Ritual storage ritual = rituals.push(); @@ -355,9 +352,9 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { processPendingFee(ritualId); // Register ritualId + 1 to discern ritualID#0 from unregistered keys. // See getRitualIdFromPublicKey() for inverse operation. - bytes32 registryKey = keccak256(abi.encodePacked( - BLS12381.g1PointToBytes(dkgPublicKey) - )); + bytes32 registryKey = keccak256( + abi.encodePacked(BLS12381.g1PointToBytes(dkgPublicKey)) + ); ritualPublicKeyRegistry[registryKey] = ritualId + 1; emit EndRitual({ritualId: ritualId, successful: true}); } @@ -370,20 +367,16 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { BLS12381.G1Point memory dkgPublicKey ) external view returns (uint32 ritualId) { // If public key is not registered, result will produce underflow error - bytes32 registryKey = keccak256(abi.encodePacked( - BLS12381.g1PointToBytes(dkgPublicKey) - )); + bytes32 registryKey = keccak256(abi.encodePacked(BLS12381.g1PointToBytes(dkgPublicKey))); return ritualPublicKeyRegistry[registryKey] - 1; } - function getPublicKeyFromRitualId(uint32 ritualId) - external view returns (BLS12381.G1Point memory dkgPublicKey) { + function getPublicKeyFromRitualId( + uint32 ritualId + ) external view returns (BLS12381.G1Point memory dkgPublicKey) { Ritual storage ritual = rituals[ritualId]; - require( - getRitualState(ritual) == RitualState.FINALIZED, - "Ritual not finalized" - ); - return ritual.publicKey; + require(getRitualState(ritual) == RitualState.FINALIZED, "Ritual not finalized"); + return ritual.publicKey; } function getParticipantFromProvider( @@ -441,7 +434,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { // Refund everything minus cost of renting cohort for a day // TODO: Validate if this is enough to remove griefing attacks uint256 duration = ritual.endTimestamp - ritual.initTimestamp; - uint256 refundableFee = pending * (duration - 1 days) / duration; + uint256 refundableFee = (pending * (duration - 1 days)) / duration; currency.safeTransfer(ritual.initiator, refundableFee); } } @@ -458,7 +451,7 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel { } function withdrawTokens(IERC20 token, uint256 amount) external onlyRole(TREASURY_ROLE) { - if (address(token) == address(currency)){ + if (address(token) == address(currency)) { require( amount <= token.balanceOf(address(this)) - totalPendingFees, "Can't withdraw pending fees" diff --git a/contracts/contracts/coordination/FlatRateFeeModel.sol b/contracts/contracts/coordination/FlatRateFeeModel.sol index 7878db31..152955c0 100644 --- a/contracts/contracts/coordination/FlatRateFeeModel.sol +++ b/contracts/contracts/coordination/FlatRateFeeModel.sol @@ -21,7 +21,7 @@ contract FlatRateFeeModel is IFeeModel { function getRitualInitiationCost( address[] calldata providers, uint32 duration - ) public view returns(uint256) { + ) public view returns (uint256) { uint256 size = providers.length; require(duration > 0, "Invalid ritual duration"); require(size > 0, "Invalid ritual size"); diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index 401b5541..f0cd5c12 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -8,7 +8,7 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorize using ECDSA for bytes32; Coordinator public coordinator; - mapping(bytes32 => bool) authorizations; + mapping(bytes32 => bool) internal authorizations; constructor( Coordinator _coordinator, @@ -25,7 +25,7 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorize _; } - function setCoordinator(Coordinator _coordinator) public onlyRole(DEFAULT_ADMIN_ROLE){ + function setCoordinator(Coordinator _coordinator) public onlyRole(DEFAULT_ADMIN_ROLE) { require(address(_coordinator) != address(0), "Coordinator cannot be zero address"); require(_coordinator.numberOfRituals() >= 0, "Invalid coordinator"); coordinator = _coordinator; @@ -63,11 +63,7 @@ contract GlobalAllowList is AccessControlDefaultAdminRules, IEncryptionAuthorize setAuthorizations(ritualId, addresses, false); } - function setAuthorizations( - uint32 ritualId, - address[] calldata addresses, - bool value - ) internal { + function setAuthorizations(uint32 ritualId, address[] calldata addresses, bool value) internal { require( coordinator.isRitualFinalized(ritualId), "Only active rituals can add authorizations" diff --git a/contracts/contracts/coordination/IEncryptionAuthorizer.sol b/contracts/contracts/coordination/IEncryptionAuthorizer.sol index 454c8ac5..a38259c6 100644 --- a/contracts/contracts/coordination/IEncryptionAuthorizer.sol +++ b/contracts/contracts/coordination/IEncryptionAuthorizer.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; interface IEncryptionAuthorizer { function isAuthorized( uint32 ritualId, - bytes memory evidence, // supporting evidence for authorization - bytes memory ciphertextHeader // data to be signed by authorized + bytes memory evidence, // supporting evidence for authorization + bytes memory ciphertextHeader // data to be signed by authorized ) external view returns (bool); }