From 728eb6166da7f8b9930a20773adf21f66a43ebb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 14 Nov 2023 11:51:38 +0100 Subject: [PATCH 01/10] Return refunded fee amount in processPendingFee --- contracts/contracts/coordination/Coordinator.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 82783aa4..dc4ff19c 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -438,7 +438,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable currency.safeTransferFrom(msg.sender, address(this), ritualCost); } - function processPendingFee(uint32 ritualId) public { + function processPendingFee(uint32 ritualId) public returns(uint256){ Ritual storage ritual = rituals[ritualId]; RitualState state = getRitualState(ritual); require( @@ -454,13 +454,15 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable totalPendingFees -= pending; delete pendingFees[ritualId]; // Transfer fees back to initiator if failed + uint256 refundableFee = 0; if (state == RitualState.TIMEOUT || state == RitualState.INVALID) { // 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; + refundableFee = (pending * (duration - 1 days)) / duration; currency.safeTransfer(ritual.initiator, refundableFee); } + return refundableFee; } function processReimbursement(uint256 initialGasLeft) internal { From 79dbc9e64ce64595ede403b55325782ab4c3f576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 14 Nov 2023 12:04:19 +0100 Subject: [PATCH 02/10] Add ReimbursementPoolSet event --- contracts/contracts/coordination/Coordinator.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index dc4ff19c..8165cb05 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -33,6 +33,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable // Admin event TimeoutChanged(uint32 oldTimeout, uint32 newTimeout); event MaxDkgSizeChanged(uint16 oldSize, uint16 newSize); + event ReimbursementPoolSet(address indexed pool); event ParticipantPublicKeySet( uint32 indexed ritualId, @@ -203,7 +204,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable "Invalid ReimbursementPool" ); reimbursementPool = pool; - // TODO: Events + emit ReimbursementPoolSet(address(pool)); } function setRitualAuthority(uint32 ritualId, address authority) external { From 8e714a235d6a77b2f51148a7652c6ab1d22be47a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 14 Nov 2023 12:25:42 +0100 Subject: [PATCH 03/10] Check for min authorization of ritual nodes --- contracts/contracts/coordination/Coordinator.sol | 6 +++++- contracts/test/CoordinatorTestSet.sol | 3 +++ contracts/threshold/ITACoChildApplication.sol | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 8165cb05..209a0dd3 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -254,6 +254,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable ritual.endTimestamp = ritual.initTimestamp + duration; ritual.accessController = accessController; + uint96 minAuthorization = application.minimumAuthorization(); address previous = address(0); for (uint256 i = 0; i < length; i++) { Participant storage newParticipant = ritual.participant.push(); @@ -265,7 +266,10 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable require(previous < current, "Providers must be sorted"); // TODO: Improve check for eligible nodes (staking, etc) - nucypher#3109 // TODO: Change check to isAuthorized(), without amount - require(application.authorizedStake(current) > 0, "Not enough authorization"); + require( + application.authorizedStake(current) >= minAuthorization, + "Not enough authorization" + ); newParticipant.provider = current; previous = current; } diff --git a/contracts/test/CoordinatorTestSet.sol b/contracts/test/CoordinatorTestSet.sol index b36cc008..88998088 100644 --- a/contracts/test/CoordinatorTestSet.sol +++ b/contracts/test/CoordinatorTestSet.sol @@ -8,6 +8,9 @@ import "../threshold/ITACoChildApplication.sol"; * @notice Contract for testing Coordinator contract */ contract ChildApplicationForCoordinatorMock is ITACoChildApplication { + + uint96 public constant minimumAuthorization = 0; + mapping(address => uint96) public authorizedStake; mapping(address => address) public operatorFromStakingProvider; mapping(address => address) public stakingProviderFromOperator; diff --git a/contracts/threshold/ITACoChildApplication.sol b/contracts/threshold/ITACoChildApplication.sol index 05f392e2..f79fbff9 100644 --- a/contracts/threshold/ITACoChildApplication.sol +++ b/contracts/threshold/ITACoChildApplication.sol @@ -9,5 +9,6 @@ interface ITACoChildApplication is ITACoChildToRoot { function authorizedStake(address _stakingProvider) external view returns (uint96); + function minimumAuthorization() external view returns (uint96); //TODO: Function to get locked stake duration? } From 027b52f2c7b24ce0173212e8610cbec7764beaf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 14 Nov 2023 12:54:39 +0100 Subject: [PATCH 04/10] Function to check if a ritual is active (i.e., DKG successful and not expired) --- contracts/contracts/coordination/Coordinator.sol | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 209a0dd3..7ccf774b 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -124,6 +124,17 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable return getRitualState(rituals[ritualId]) == RitualState.FINALIZED; } + function isRitualActive(Ritual storage ritual) internal view returns (bool) { + bool dkgIsFinalized = getRitualState(ritual) == RitualState.FINALIZED; + bool ritualIsNotExpired = block.timestamp <= ritual.endTimestamp; + return dkgIsFinalized && ritualIsNotExpired; + } + + function isRitualActive(uint32 ritualId) external view returns (bool) { + Ritual storage ritual = rituals[ritualId]; + return isRitualActive(ritual); + } + function getRitualState(Ritual storage ritual) internal view returns (RitualState) { uint32 t0 = ritual.initTimestamp; uint32 deadline = t0 + timeout; From eab052bb18b892f844647241931de48ef15f1229 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 14 Nov 2023 13:05:24 +0100 Subject: [PATCH 05/10] Change setRitualAuthority to transferRitualAuthority Emit RitualAuthorityTransferred event --- .../contracts/coordination/Coordinator.sol | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 7ccf774b..b76ece8d 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -17,12 +17,10 @@ import "./IEncryptionAuthorizer.sol"; * @notice Coordination layer for Threshold Access Control (TACo 🌮) */ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable, FlatRateFeeModel { - // Ritual + // DKG Protocol event StartRitual(uint32 indexed ritualId, address indexed authority, address[] participants); event StartAggregationRound(uint32 indexed ritualId); event EndRitual(uint32 indexed ritualId, bool successful); - - // Node event TranscriptPosted(uint32 indexed ritualId, address indexed node, bytes32 transcriptDigest); event AggregationPosted( uint32 indexed ritualId, @@ -30,11 +28,17 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable bytes32 aggregatedTranscriptDigest ); - // Admin + // Protocol administration event TimeoutChanged(uint32 oldTimeout, uint32 newTimeout); event MaxDkgSizeChanged(uint16 oldSize, uint16 newSize); event ReimbursementPoolSet(address indexed pool); + // Cohort administration + event RitualAuthorityTransferred( + uint32 indexed ritualId, + address indexed previousAuthority, + address indexed newAuthority + ); event ParticipantPublicKeySet( uint32 indexed ritualId, address indexed participant, @@ -218,11 +222,13 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable emit ReimbursementPoolSet(address(pool)); } - function setRitualAuthority(uint32 ritualId, address authority) external { + function transferRitualAuthority(uint32 ritualId, address newAuthority) external { Ritual storage ritual = rituals[ritualId]; - require(getRitualState(ritual) == RitualState.FINALIZED, "Ritual not finalized"); - require(msg.sender == ritual.authority, "Sender not ritual authority"); - ritual.authority = authority; + require(isRitualActive(ritual), "Ritual is not active"); + address previousAuthority = ritual.authority; + require(msg.sender == previousAuthority, "Sender not ritual authority"); + ritual.authority = newAuthority; + emit RitualAuthorityTransferred(ritualId, previousAuthority, newAuthority); } function numberOfRituals() external view returns (uint256) { From 83288aab2c32c30d0639beb3637e9065d778e32e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Tue, 14 Nov 2023 17:37:25 +0100 Subject: [PATCH 06/10] Refactor RitualState to split FINALIZED into ACTIVE and EXPIRED --- .../contracts/coordination/Coordinator.sol | 61 +++++++++++-------- .../coordination/GlobalAllowList.sol | 2 +- tests/test_coordinator.py | 23 +++---- 3 files changed, 48 insertions(+), 38 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index b76ece8d..d2b1af43 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -47,11 +47,12 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable enum RitualState { NON_INITIATED, - AWAITING_TRANSCRIPTS, - AWAITING_AGGREGATIONS, - TIMEOUT, - INVALID, - FINALIZED + DKG_AWAITING_TRANSCRIPTS, + DKG_AWAITING_AGGREGATIONS, + DKG_TIMEOUT, + DKG_INVALID, + ACTIVE, + EXPIRED } struct Participant { @@ -124,14 +125,8 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable return getRitualState(rituals[ritualId]); } - function isRitualFinalized(uint32 ritualId) external view returns (bool) { - return getRitualState(rituals[ritualId]) == RitualState.FINALIZED; - } - function isRitualActive(Ritual storage ritual) internal view returns (bool) { - bool dkgIsFinalized = getRitualState(ritual) == RitualState.FINALIZED; - bool ritualIsNotExpired = block.timestamp <= ritual.endTimestamp; - return dkgIsFinalized && ritualIsNotExpired; + return getRitualState(ritual) == RitualState.ACTIVE; } function isRitualActive(uint32 ritualId) external view returns (bool) { @@ -145,16 +140,24 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable if (t0 == 0) { return RitualState.NON_INITIATED; } else if (ritual.totalAggregations == ritual.dkgSize) { - return RitualState.FINALIZED; + // DKG was succesful + if(block.timestamp <= ritual.endTimestamp) { + return RitualState.ACTIVE; + } else { + return RitualState.EXPIRED; + } } else if (ritual.aggregationMismatch) { - return RitualState.INVALID; + // DKG failed due to invalid transcripts + return RitualState.DKG_INVALID; } else if (block.timestamp > deadline) { - return RitualState.TIMEOUT; + // DKG failed due to timeout + return RitualState.DKG_TIMEOUT; } else if (ritual.totalTranscripts < ritual.dkgSize) { - return RitualState.AWAITING_TRANSCRIPTS; + // DKG still waiting for transcripts + return RitualState.DKG_AWAITING_TRANSCRIPTS; } else if (ritual.totalAggregations < ritual.dkgSize) { - return RitualState.AWAITING_AGGREGATIONS; - // solhint-disable-next-line no-empty-blocks + // DKG still waiting for aggregations + return RitualState.DKG_AWAITING_AGGREGATIONS; } else { // It shouldn't be possible to reach this state: // - No public key @@ -306,7 +309,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable Ritual storage ritual = rituals[ritualId]; require( - getRitualState(ritual) == RitualState.AWAITING_TRANSCRIPTS, + getRitualState(ritual) == RitualState.DKG_AWAITING_TRANSCRIPTS, "Not waiting for transcripts" ); @@ -345,7 +348,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable Ritual storage ritual = rituals[ritualId]; require( - getRitualState(ritual) == RitualState.AWAITING_AGGREGATIONS, + getRitualState(ritual) == RitualState.DKG_AWAITING_AGGREGATIONS, "Not waiting for aggregations" ); @@ -412,7 +415,12 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable uint32 ritualId ) external view returns (BLS12381.G1Point memory dkgPublicKey) { Ritual storage ritual = rituals[ritualId]; - require(getRitualState(ritual) == RitualState.FINALIZED, "Ritual not finalized"); + RitualState state = getRitualState(ritual); + require( + state == RitualState.ACTIVE + || state == RitualState.EXPIRED, + "Ritual not finalized" + ); return ritual.publicKey; } @@ -444,7 +452,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable bytes memory ciphertextHeader ) external view returns (bool) { Ritual storage ritual = rituals[ritualId]; - require(getRitualState(ritual) == RitualState.FINALIZED, "Ritual not finalized"); + require(getRitualState(ritual) == RitualState.ACTIVE, "Ritual not active"); return ritual.accessController.isAuthorized(ritualId, evidence, ciphertextHeader); } @@ -464,9 +472,10 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable Ritual storage ritual = rituals[ritualId]; RitualState state = getRitualState(ritual); require( - state == RitualState.TIMEOUT || - state == RitualState.INVALID || - state == RitualState.FINALIZED, + state == RitualState.DKG_TIMEOUT || + state == RitualState.DKG_INVALID || + state == RitualState.ACTIVE || + state == RitualState.EXPIRED, "Ritual is not ended" ); uint256 pending = pendingFees[ritualId]; @@ -477,7 +486,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable delete pendingFees[ritualId]; // Transfer fees back to initiator if failed uint256 refundableFee = 0; - if (state == RitualState.TIMEOUT || state == RitualState.INVALID) { + if (state == RitualState.DKG_TIMEOUT || state == RitualState.DKG_INVALID) { // 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; diff --git a/contracts/contracts/coordination/GlobalAllowList.sol b/contracts/contracts/coordination/GlobalAllowList.sol index 9402ddb2..3042798e 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -63,7 +63,7 @@ contract GlobalAllowList is IEncryptionAuthorizer { function setAuthorizations(uint32 ritualId, address[] calldata addresses, bool value) internal { require( - coordinator.isRitualFinalized(ritualId), + coordinator.isRitualActive(ritualId), "Only active rituals can add authorizations" ); for (uint256 i = 0; i < addresses.length; i++) { diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 805b72a4..63c82c7d 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -16,11 +16,12 @@ "RitualState", [ "NON_INITIATED", - "AWAITING_TRANSCRIPTS", - "AWAITING_AGGREGATIONS", - "TIMEOUT", - "INVALID", - "FINALIZED", + "DKG_AWAITING_TRANSCRIPTS", + "DKG_AWAITING_AGGREGATIONS", + "DKG_TIMEOUT", + "DKG_INVALID", + "ACTIVE", + "EXPIRED" ], start=0, ) @@ -190,7 +191,7 @@ def test_initiate_ritual( assert event["authority"] == authority assert event["participants"] == tuple(n.address.lower() for n in nodes) - assert coordinator.getRitualState(0) == RitualState.AWAITING_TRANSCRIPTS + assert coordinator.getRitualState(0) == RitualState.DKG_AWAITING_TRANSCRIPTS ritual_struct = coordinator.rituals(ritualID) assert ritual_struct[0] == initiator @@ -248,7 +249,7 @@ def test_post_transcript(coordinator, nodes, initiator, erc20, global_allow_list transcript = os.urandom(transcript_size(len(nodes), len(nodes))) for node in nodes: - assert coordinator.getRitualState(0) == RitualState.AWAITING_TRANSCRIPTS + assert coordinator.getRitualState(0) == RitualState.DKG_AWAITING_TRANSCRIPTS tx = coordinator.postTranscript(0, transcript, sender=node) @@ -264,7 +265,7 @@ def test_post_transcript(coordinator, nodes, initiator, erc20, global_allow_list assert not participant.aggregated assert not participant.decryptionRequestStaticKey - assert coordinator.getRitualState(0) == RitualState.AWAITING_AGGREGATIONS + assert coordinator.getRitualState(0) == RitualState.DKG_AWAITING_AGGREGATIONS def test_post_transcript_but_not_part_of_ritual( @@ -336,7 +337,7 @@ def test_post_aggregation( 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(ritualID) == RitualState.AWAITING_AGGREGATIONS + assert coordinator.getRitualState(ritualID) == RitualState.DKG_AWAITING_AGGREGATIONS tx = coordinator.postAggregation( ritualID, aggregated, dkg_public_key, decryption_request_static_keys[i], sender=node ) @@ -353,7 +354,7 @@ def test_post_aggregation( assert participant.aggregated assert participant.decryptionRequestStaticKey == decryption_request_static_keys[i] - assert coordinator.getRitualState(ritualID) == RitualState.FINALIZED + assert coordinator.getRitualState(ritualID) == RitualState.ACTIVE events = coordinator.EndRitual.from_receipt(tx) assert events == [coordinator.EndRitual(ritualId=ritualID, successful=True)] @@ -402,7 +403,7 @@ def test_authorize_using_global_allow_list( with ape.reverts("Only active rituals can add authorizations"): global_allow_list.authorize(0, [deployer.address], sender=initiator) - with ape.reverts("Ritual not finalized"): + with ape.reverts("Ritual not active"): coordinator.isEncryptionAuthorized(0, bytes(signature), bytes(digest)) # Finalize ritual From 24e933b45d013c30092a2a5a823d0d957d76437f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 15 Nov 2023 12:19:44 +0100 Subject: [PATCH 07/10] Test for aggregation round failure --- tests/test_coordinator.py | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 63c82c7d..1aa20c8e 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -11,6 +11,7 @@ FEE_RATE = 42 ERC20_SUPPLY = 10**24 DURATION = 48 * 60 * 60 +ONE_DAY = 24 * 60 * 60 RitualState = IntEnum( "RitualState", @@ -373,6 +374,67 @@ def test_post_aggregation( coordinator.withdrawTokens(erc20.address, fee, sender=treasury) +def test_post_aggregation_fails( + coordinator, nodes, initiator, erc20, global_allow_list, treasury, deployer +): + coordinator.grantRole(coordinator.TREASURY_ROLE(), treasury, sender=deployer) + initiator_balance_before_payment = erc20.balanceOf(initiator) + + initiate_ritual( + coordinator=coordinator, + erc20=erc20, + authority=initiator, + nodes=nodes, + allow_logic=global_allow_list, + ) + ritualID = 0 + transcript = os.urandom(transcript_size(len(nodes), len(nodes))) + for node in nodes: + 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)) + + # First node does their thing + _ = coordinator.postAggregation( + ritualID, aggregated, dkg_public_key, decryption_request_static_keys[0], sender=nodes[0] + ) + + # Second node screws up everything + bad_aggregated = os.urandom(transcript_size(len(nodes), len(nodes))) + tx = coordinator.postAggregation( + ritualID, bad_aggregated, dkg_public_key, decryption_request_static_keys[1], sender=nodes[1] + ) + + assert coordinator.getRitualState(ritualID) == RitualState.DKG_INVALID + events = coordinator.EndRitual.from_receipt(tx) + assert events == [coordinator.EndRitual(ritualId=ritualID, successful=False)] + + # Fees are still pending + fee = coordinator.getRitualInitiationCost(nodes, DURATION) + assert erc20.balanceOf(coordinator) == fee + assert coordinator.totalPendingFees() == fee + assert coordinator.pendingFees(ritualID) == fee + with ape.reverts("Can't withdraw pending fees"): + coordinator.withdrawTokens(erc20.address, 1, sender=treasury) + + # Anyone can trigger processing of pending fees + + initiator_balance_before_refund = erc20.balanceOf(initiator) + assert initiator_balance_before_refund == initiator_balance_before_payment - fee + + coordinator.processPendingFee(ritualID, sender=treasury) + + initiator_balance_after_refund = erc20.balanceOf(initiator) + coordinator_balance_after_refund = erc20.balanceOf(coordinator) + refund = initiator_balance_after_refund - initiator_balance_before_refund + assert refund == coordinator.getRitualInitiationCost(nodes, ONE_DAY) + assert coordinator_balance_after_refund + refund == fee + assert coordinator.totalPendingFees() == 0 + assert coordinator.pendingFees(ritualID) == 0 + coordinator.withdrawTokens(erc20.address, coordinator_balance_after_refund, sender=treasury) + def test_authorize_using_global_allow_list( coordinator, nodes, deployer, initiator, erc20, global_allow_list ): From c6ad0186bc5e1d1909c36fb08bee174203f3ba68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 15 Nov 2023 15:26:20 +0100 Subject: [PATCH 08/10] Linting --- contracts/contracts/coordination/Coordinator.sol | 9 ++++----- contracts/contracts/coordination/GlobalAllowList.sol | 5 +---- contracts/test/CoordinatorTestSet.sol | 3 +-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index d2b1af43..301f5256 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -141,7 +141,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable return RitualState.NON_INITIATED; } else if (ritual.totalAggregations == ritual.dkgSize) { // DKG was succesful - if(block.timestamp <= ritual.endTimestamp) { + if (block.timestamp <= ritual.endTimestamp) { return RitualState.ACTIVE; } else { return RitualState.EXPIRED; @@ -287,7 +287,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable // TODO: Improve check for eligible nodes (staking, etc) - nucypher#3109 // TODO: Change check to isAuthorized(), without amount require( - application.authorizedStake(current) >= minAuthorization, + application.authorizedStake(current) >= minAuthorization, "Not enough authorization" ); newParticipant.provider = current; @@ -417,8 +417,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable Ritual storage ritual = rituals[ritualId]; RitualState state = getRitualState(ritual); require( - state == RitualState.ACTIVE - || state == RitualState.EXPIRED, + state == RitualState.ACTIVE || state == RitualState.EXPIRED, "Ritual not finalized" ); return ritual.publicKey; @@ -468,7 +467,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable currency.safeTransferFrom(msg.sender, address(this), ritualCost); } - function processPendingFee(uint32 ritualId) public returns(uint256){ + function processPendingFee(uint32 ritualId) public returns (uint256) { 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 3042798e..c4bed5fe 100644 --- a/contracts/contracts/coordination/GlobalAllowList.sol +++ b/contracts/contracts/coordination/GlobalAllowList.sol @@ -62,10 +62,7 @@ contract GlobalAllowList is IEncryptionAuthorizer { } function setAuthorizations(uint32 ritualId, address[] calldata addresses, bool value) internal { - require( - coordinator.isRitualActive(ritualId), - "Only active rituals can add authorizations" - ); + require(coordinator.isRitualActive(ritualId), "Only active rituals can add authorizations"); for (uint256 i = 0; i < addresses.length; i++) { authorizations[lookupKey(ritualId, addresses[i])] = value; } diff --git a/contracts/test/CoordinatorTestSet.sol b/contracts/test/CoordinatorTestSet.sol index 88998088..38d8707d 100644 --- a/contracts/test/CoordinatorTestSet.sol +++ b/contracts/test/CoordinatorTestSet.sol @@ -8,8 +8,7 @@ import "../threshold/ITACoChildApplication.sol"; * @notice Contract for testing Coordinator contract */ contract ChildApplicationForCoordinatorMock is ITACoChildApplication { - - uint96 public constant minimumAuthorization = 0; + uint96 public minimumAuthorization = 0; mapping(address => uint96) public authorizedStake; mapping(address => address) public operatorFromStakingProvider; From 50bc5d7716b5cdab6ef130101ada7c695ed67ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 15 Nov 2023 21:41:40 +0100 Subject: [PATCH 09/10] Some optimizations suggested by Vicky Co-authored-by: Victoria Zotova --- contracts/contracts/coordination/Coordinator.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 301f5256..dc5efd2a 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -92,6 +92,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable bytes32 public constant TREASURY_ROLE = keccak256("TREASURY_ROLE"); ITACoChildApplication public immutable application; + uint96 private immutable minAuthorization; Ritual[] public rituals; uint32 public timeout; @@ -110,6 +111,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable uint256 _feeRatePerSecond ) FlatRateFeeModel(_currency, _feeRatePerSecond) { application = _application; + minAuthorization = _application.minimumAuthorization(); } /** @@ -274,7 +276,6 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable ritual.endTimestamp = ritual.initTimestamp + duration; ritual.accessController = accessController; - uint96 minAuthorization = application.minimumAuthorization(); address previous = address(0); for (uint256 i = 0; i < length; i++) { Participant storage newParticipant = ritual.participant.push(); @@ -467,7 +468,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable currency.safeTransferFrom(msg.sender, address(this), ritualCost); } - function processPendingFee(uint32 ritualId) public returns (uint256) { + function processPendingFee(uint32 ritualId) public returns (uint256 refundableFee) { Ritual storage ritual = rituals[ritualId]; RitualState state = getRitualState(ritual); require( @@ -484,7 +485,6 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable totalPendingFees -= pending; delete pendingFees[ritualId]; // Transfer fees back to initiator if failed - uint256 refundableFee = 0; if (state == RitualState.DKG_TIMEOUT || state == RitualState.DKG_INVALID) { // Refund everything minus cost of renting cohort for a day // TODO: Validate if this is enough to remove griefing attacks From 37ab81db1193f055fba224b6a42ae3cfadc462eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 15 Nov 2023 22:04:32 +0100 Subject: [PATCH 10/10] Update package-lock.json --- package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 190592f7..a8feed4e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@nucypher/nucypher-contracts", - "version": "0.10.0", + "version": "0.11.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@nucypher/nucypher-contracts", - "version": "0.10.0", + "version": "0.11.1", "license": "AGPL-3.0-or-later", "devDependencies": { "@typescript-eslint/eslint-plugin": "^6.9.0",