wip: cleanup/refactor activity and rewards contracts.

- merge two libraries into a single one. - place libraries in lib/ directory. - adjust error types. - decouple the rewards logic from the consensus claims logic. - simplify code; remove indirection; cleanup naming; docs. TODO: - rework "distribution" parts. - reintegrate.
consensus-shipyard · Nov 26, 2024 · ba0b609 · ba0b609
1 parent 2b9c3b4
commit ba0b609
Show file tree

Hide file tree

Showing 10 changed files with 181 additions and 222 deletions.
diff --git a/contracts/contracts/SubnetActorDiamond.sol b/contracts/contracts/SubnetActorDiamond.sol
@@ -15,7 +15,7 @@ import {SubnetIDHelper} from "./lib/SubnetIDHelper.sol";
 import {LibStaking} from "./lib/LibStaking.sol";
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {AssetHelper} from "./lib/AssetHelper.sol";
-import {LibValidatorReward} from "./activities/ValidatorRewardFacet.sol";
+import {LibActivity} from "./lib/LibActivity.sol";
 
 error FunctionNotFound(bytes4 _functionSelector);
 
@@ -107,7 +107,7 @@ contract SubnetActorDiamond {
         }
 
         if (params.validatorRewarder != address(0)) {
-            LibValidatorReward.setRewarder(params.validatorRewarder);
+            LibActivity.setRewarder(params.validatorRewarder);
         }
     }
 

diff --git a/contracts/contracts/activities/IValidatorRewarder.sol b/contracts/contracts/activities/IValidatorRewarder.sol
@@ -9,12 +9,14 @@ import {Consensus} from "./Activity.sol";
 /// @dev Implement this interface and supply the address of the implementation contract at subnet creation to process
 /// subnet summaries at this level, and disburse rewards to validators based on their block production activity.
 ///
-/// This interface will be called by the subnet actor when a relayer presents a
+/// This interface will be called by the subnet actor when a validator presents a _valid_ proof of consensus activity,
+/// via the ValidatorRewardFacet#claim
+///
 interface IValidatorRewarder {
     /// @notice Called by the subnet manager contract to instruct the rewarder to process the subnet summary and
     /// disburse any relevant rewards.
     /// @dev This method should revert if the summary is invalid; this will cause the
-    function disburseRewards(SubnetID calldata id, Consensus.ValidatorData calldata detail) external;
+    function disburseRewards(SubnetID calldata id, Consensus.ValidatorData calldata validatedData) external;
 }
 
 /// @title Validator reward setup interface

diff --git a/contracts/contracts/activities/LibActivityMerkleVerifier.sol b/contracts/contracts/activities/LibActivityMerkleVerifier.sol
diff --git a/contracts/contracts/activities/ValidatorRewardFacet.sol b/contracts/contracts/activities/ValidatorRewardFacet.sol
@@ -2,13 +2,9 @@
 pragma solidity ^0.8.23;
 
 import {Consensus} from "./Activity.sol";
-
-import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol";
-import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
-import {IValidatorRewarder, IValidatorRewardSetup} from "./IValidatorRewarder.sol";
-import {LibActivityMerkleVerifier} from "./LibActivityMerkleVerifier.sol";
+import {LibActivity} from "../lib/LibActivity.sol";
 import {LibDiamond} from "../lib/LibDiamond.sol";
-import {NotAuthorized, SubnetNoTargetCommitment, CommitmentAlreadyInitialized, ValidatorAlreadyClaimed, NotGateway, NotOwner} from "../errors/IPCErrors.sol";
+import {NotAuthorized} from "../errors/IPCErrors.sol";
 import {Pausable} from "../lib/LibPausable.sol";
 import {ReentrancyGuard} from "../lib/LibReentrancyGuard.sol";
 import {SubnetIDHelper} from "../lib/SubnetIDHelper.sol";
@@ -53,185 +49,12 @@ contract ValidatorRewardFacet is ReentrancyGuard, Pausable {
         Consensus.ValidatorData calldata detail,
         Consensus.MerkleHash[] calldata proof
     ) internal {
-        ValidatorRewardStorage storage s = LibValidatorReward.facetStorage();
-
         // Note: No need to check if the subnet is active. If the subnet is not active, the checkpointHeight
         // will never exist.
         if (msg.sender != detail.validator) {
             revert NotAuthorized(msg.sender);
         }
 
-        LibValidatorReward.handleDistribution(subnetId, checkpointHeight, detail, proof);
-    }
-}
-
-/// The activity summary commiment that is currently under reward distribution
-struct RewardDistribution {
-    /// Total number of valdators to claim the distribution
-    uint64 totalValidators;
-    /// The list of validators that have claimed the reward
-    EnumerableSet.AddressSet claimed;
-}
-
-/// Used by the SubnetActor to track the rewards for each validator
-struct ValidatorRewardStorage {
-    /// @notice The contract address for the validator rewarder.
-    address validatorRewarder;
-    /// @notice Summaries look up pending to be processed.
-    /// If the validator rewarder is non-zero, these denote summaries presentable at this level.
-    /// If the validator rewarder is zero, these summaries must be relayed upwards in the next bottom-up checkpoint.
-    /// Partitioned by subnet ID (hash) then by checkpoint block height in the child subnet to the commitment
-    mapping(bytes32 => EnumerableMap.Bytes32ToBytes32Map) commitments;
-    /// @notice Index over presentable summaries back to the subnet ID, so we can locate them quickly when they're presented.
-    /// Only used if the validator rewarder is non-zero.
-    /// Partitioned by subnet ID (hash) then by checkpoint block height in the child subnet to the commitment
-    mapping(bytes32 => mapping(uint64 => RewardDistribution)) distributions;
-}
-
-/// The payload for list commitments query
-struct ListCommitmentDetail {
-    /// The child subnet checkpoint height
-    uint64 checkpointHeight;
-    /// The actual commiment of the activities
-    bytes32 commitment;
-}
-
-library LibValidatorReward {
-    bytes32 private constant NAMESPACE = keccak256("validator.reward.parent");
-
-    using SubnetIDHelper for SubnetID;
-    using EnumerableMap for EnumerableMap.Bytes32ToBytes32Map;
-    using EnumerableSet for EnumerableSet.AddressSet;
-
-    // =========== External library functions =============
-
-    function initNewDistribution(
-        SubnetID calldata subnetId,
-        uint64 checkpointHeight,
-        Consensus.MerkleHash commitment,
-        uint64 totalActiveValidators
-    ) internal {
-        ValidatorRewardStorage storage ds = facetStorage();
-
-        bytes32 subnetKey = subnetId.toHash();
-
-        if (ds.distributions[subnetKey][checkpointHeight].totalValidators != 0) {
-            revert CommitmentAlreadyInitialized();
-        }
-
-        ds.commitments[subnetKey].set(bytes32(uint256(checkpointHeight)), Consensus.MerkleHash.unwrap(commitment));
-        ds.distributions[subnetKey][checkpointHeight].totalValidators = totalActiveValidators;
-    }
-
-    function listCommitments(
-        SubnetID calldata subnetId
-    ) internal view returns (ListCommitmentDetail[] memory listDetails) {
-        ValidatorRewardStorage storage ds = facetStorage();
-
-        bytes32 subnetKey = subnetId.toHash();
-
-        uint256 size = ds.commitments[subnetKey].length();
-        listDetails = new ListCommitmentDetail[](size);
-
-        for (uint256 i = 0; i < size; ) {
-            (bytes32 heightBytes32, bytes32 commitment) = ds.commitments[subnetKey].at(i);
-
-            listDetails[i] = ListCommitmentDetail({
-                checkpointHeight: uint64(uint256(heightBytes32)),
-                commitment: commitment
-            });
-
-            unchecked {
-                i++;
-            }
-        }
-
-        return listDetails;
-    }
-
-    function setRewarder(address rewarder) internal {
-        ValidatorRewardStorage storage ds = facetStorage();
-        ds.validatorRewarder = rewarder;
-    }
-
-    // ============ Internal library functions ============
-
-    function facetStorage() internal pure returns (ValidatorRewardStorage storage ds) {
-        bytes32 position = NAMESPACE;
-        assembly {
-            ds.slot := position
-        }
-        return ds;
-    }
-
-    function handleDistribution(
-        SubnetID calldata subnetId,
-        uint64 checkpointHeight,
-        Consensus.ValidatorData calldata detail,
-        Consensus.MerkleHash[] calldata proof
-    ) internal {
-        ValidatorRewardStorage storage s = LibValidatorReward.facetStorage();
-
-        bytes32 subnetKey = subnetId.toHash();
-
-        bytes32 commitment = ensureValidCommitment(s, subnetKey, checkpointHeight);
-        LibActivityMerkleVerifier.ensureValidProof(commitment, detail, proof);
-
-        validatorTryClaim(s, subnetKey, checkpointHeight, detail.validator);
-        IValidatorRewarder(s.validatorRewarder).disburseRewards(subnetId, detail);
-    }
-
-    function ensureValidCommitment(
-        ValidatorRewardStorage storage ds,
-        bytes32 subnetKey,
-        uint64 checkpointHeight
-    ) internal view returns (bytes32) {
-        (bool exists, bytes32 commitment) = ds.commitments[subnetKey].tryGet(bytes32(uint256(checkpointHeight)));
-        if (!exists) {
-            revert SubnetNoTargetCommitment();
-        }
-
-        // Note: ideally we should check the commitment actually exists, but we dont have to as
-        // Note: the code will ensure if commitments contains the commitment,
-        // Note: the commitment will have distribution
-        // if (ds.distributions[commitment].checkpointHeight == 0) {
-        //     revert CommitmentNotFound();
-        // }
-
-        return commitment;
-    }
-
-    /// Validator tries to claim the reward. The validator can only claim the reward if the validator
-    /// has not claimed before
-    function validatorTryClaim(
-        ValidatorRewardStorage storage ds,
-        bytes32 subnetKey,
-        uint64 checkpointHeight,
-        address validator
-    ) internal {
-        if (ds.distributions[subnetKey][checkpointHeight].claimed.contains(validator)) {
-            revert ValidatorAlreadyClaimed();
-        }
-
-        ds.distributions[subnetKey][checkpointHeight].claimed.add(validator);
-    }
-
-    /// Try to remove the distribution in the target subnet when ALL VALIDATORS HAVE CLAIMED.
-    function tryPurgeDistribution(
-        ValidatorRewardStorage storage ds,
-        SubnetID calldata subnetId,
-        uint64 checkpointHeight
-    ) internal {
-        bytes32 subnetKey = subnetId.toHash();
-
-        uint256 total = uint256(ds.distributions[subnetKey][checkpointHeight].totalValidators);
-        uint256 claimed = ds.distributions[subnetKey][checkpointHeight].claimed.length();
-
-        if (claimed < total) {
-            return;
-        }
-
-        delete ds.commitments[subnetKey];
-        delete ds.distributions[subnetKey][checkpointHeight];
+        LibActivity.processConsensusClaim(subnetId, checkpointHeight, detail, proof);
     }
 }
diff --git a/contracts/contracts/errors/IPCErrors.sol b/contracts/contracts/errors/IPCErrors.sol
@@ -80,8 +80,8 @@ error InvalidFederationPayload();
 error DuplicatedGenesisValidator();
 error NotEnoughGenesisValidators();
 error ValidatorPowerChangeDenied();
-error CommitmentAlreadyInitialized();
-error SubnetNoTargetCommitment();
+error DuplicateCommitment();
+error MissingActivityCommitment();
 error ValidatorAlreadyClaimed();
 error InvalidActivityProof();
 error NotOwner();