diff --git a/protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol b/protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol index 8769d5e01..a096ea93e 100644 --- a/protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol +++ b/protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol @@ -48,7 +48,7 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { /// @notice Sets the minimum time that must pass before a commitment can be postconfirmed /// @param _minCommitmentAgeForPostconfirmation New minimum commitment age // TODO we also require a check when setting the epoch length that it is larger than the min commitment age - function setMinCommitmentAge(uint256 _minCommitmentAgeForPostconfirmation) public onlyRole(COMMITMENT_ADMIN) { + function setMinCommitmentAgeForPostconfirmation(uint256 _minCommitmentAgeForPostconfirmation) public onlyRole(COMMITMENT_ADMIN) { // Ensure min age is less than epoch duration to allow postconfirmation within same epoch if (_minCommitmentAgeForPostconfirmation >= stakingContract.getEpochDuration(address(this))) { revert MinCommitmentAgeTooLong(); @@ -338,7 +338,7 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { } } - function currentAcceptorIsLive() public pure returns (bool) { + function isAcceptorLive() public pure returns (bool) { // TODO check if current acceptor has been live sufficiently recently // use getAcceptorStartTime, and the mappings return true; // dummy implementation @@ -432,7 +432,7 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { // TODO: for rewards we have to run through all the attesters, as we need to acknowledge that they get rewards. // TODO: if the attester is the current acceptor, we need to record that the acceptor has shown liveness. - // TODO: this liveness needs to be discoverable by isCurrentAcceptorLive() + // TODO: this liveness needs to be discoverable by isAcceptorLive() return true; } @@ -507,6 +507,23 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { } } + // Award points to postconfirmer + // if the acceptor has not been live award anyone who postconfirms + if (!isAcceptorLive()) { + console.log("[postconfirmSuperBlockCommitment] currentAcceptor is not live"); + postconfirmerRewardPoints[currentAcceptingEpoch][attester] += 1; + } else { + // if the acceptor has been live, only award points to the acceptor + // TODO optimization: even if the height has been volunteer postconfirmed we need to allow that that acceptor gets rewards, + // TODO otherwise weak acceptors may could get played (rich volunteer acceptors pay the fees and poor acceptors never get any reward) + // TODO but check if this is really required game theoretically. + console.log("[postconfirmSuperBlockCommitment] currentAcceptor is %s", getAcceptor()); + console.log("[postconfirmSuperBlockCommitment] attester is %s", attester); + if (getAcceptor() == attester) { + postconfirmerRewardPoints[currentAcceptingEpoch][attester] += 1; + } + } + versionedPostconfirmedSuperBlocks[postconfirmedSuperBlocksVersion][superBlockCommitment.height] = superBlockCommitment; lastPostconfirmedSuperBlockHeight = superBlockCommitment.height; postconfirmedBy[superBlockCommitment.height] = attester; @@ -543,8 +560,8 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { for (uint256 i = 0; i < attesters.length; i++) { if (attesterRewardPoints[acceptingEpoch][attesters[i]] > 0) { // TODO: make this configurable and set it on instance creation - uint256 rewardPerPoint = 1; - uint256 reward = attesterRewardPoints[acceptingEpoch][attesters[i]] * rewardPerPoint * getAttesterStakeForAcceptingEpoch(attesters[i]); + uint256 rewardPerAttestationPoint = 1; + uint256 reward = attesterRewardPoints[acceptingEpoch][attesters[i]] * rewardPerAttestationPoint * getAttesterStakeForAcceptingEpoch(attesters[i]); // the staking contract is the custodian console.log("[rollOverEpoch] Rewarding attester %s with %s", attesters[i], reward); console.log("[rollOverEpoch] Staking contract is %s", address(stakingContract)); @@ -552,7 +569,21 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { console.log("[rollOverEpoch] msg.sender is %s", msg.sender); // rewards are currently paid out from the mcr domain stakingContract.rewardFromDomain(attesters[i], reward, moveTokenAddress); - delete attesterRewardPoints[acceptingEpoch][attesters[i]]; + // TODO : check if we really have to keep attesterRewardPoints per epoch, or whether we could simply delete the points here for a given attester. + } + + // Add postconfirmation rewards + if (postconfirmerRewardPoints[acceptingEpoch][attesters[i]] > 0) { + uint256 rewardPerPostconfirmationPoint = 1; // Can be different from attester reward + uint256 reward = postconfirmerRewardPoints[acceptingEpoch][attesters[i]] * rewardPerPostconfirmationPoint * getAttesterStakeForAcceptingEpoch(attesters[i]); + console.log("[rollOverEpoch] Rewarding postconfirmer %s with %s", attesters[i], reward); + console.log("[rollOverEpoch] Staking contract is %s", address(stakingContract)); + console.log("[rollOverEpoch] Move token address is %s", moveTokenAddress); + console.log("[rollOverEpoch] msg.sender is %s", msg.sender); + stakingContract.rewardFromDomain(attesters[i], reward, moveTokenAddress); + // TODO : check if we really have to keep postconfirmerRewardPoints per epoch, or whether we could simply delete the points here for a given postconfirmer. + // TODO also the postconfirmer list is super short. typically for a given height only the acceptor and at most the acceptor and a volunteer acceptor. + // TODO So this can be heavily optimized. } } @@ -596,4 +627,12 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { function getAttesterRewardPoints(uint256 epoch, address attester) public view returns (uint256) { return attesterRewardPoints[epoch][attester]; } + + /// @notice Gets the reward points for a postconfirmer in a given epoch + /// @param epoch The epoch to get the reward points for + /// @param postconfirmer The postconfirmer to get the reward points for + /// @return The reward points for the postconfirmer in the given epoch + function getPostconfirmerRewardPoints(uint256 epoch, address postconfirmer) public view returns (uint256) { + return postconfirmerRewardPoints[epoch][postconfirmer]; + } } diff --git a/protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol b/protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol index 19a23ff0b..4c787e1cf 100644 --- a/protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol +++ b/protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol @@ -93,6 +93,9 @@ contract MCRStorage { // track reward points for attesters mapping(uint256 epoch => mapping(address attester => uint256 points)) public attesterRewardPoints; - uint256[47] internal __gap; + // track reward points for postconfirmers + mapping(uint256 epoch => mapping(address postconfirmer => uint256 points)) public postconfirmerRewardPoints; + + uint256[45] internal __gap; // Reduced by 1 for new mapping } \ No newline at end of file diff --git a/protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol b/protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol index 8c8777c22..65237a53c 100644 --- a/protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol +++ b/protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol @@ -662,7 +662,7 @@ contract MCRTest is Test, IMCR { assert(bobCommitment.commitment == commitment.commitment); // Verify acceptor state - assert(mcr.currentAcceptorIsLive()); + assert(mcr.isAcceptorLive()); assertEq(mcr.getSuperBlockHeightAssignedEpoch(targetHeight), mcr.getAcceptingEpoch()); // Attempt postconfirmation @@ -698,7 +698,7 @@ contract MCRTest is Test, IMCR { assert(bobCommitment.commitment == commitment.commitment); // Verify acceptor state - assert(mcr.currentAcceptorIsLive()); + assert(mcr.isAcceptorLive()); assertEq(mcr.getSuperBlockHeightAssignedEpoch(targetHeight), mcr.getAcceptingEpoch()); // Attempt postconfirmation - this should fail because there's no supermajority @@ -772,11 +772,11 @@ contract MCRTest is Test, IMCR { function testSetMinCommitmentAge() public { // Set min commitment age to a too long value vm.expectRevert(MCR.MinCommitmentAgeTooLong.selector); - mcr.setMinCommitmentAge(epochDuration); + mcr.setMinCommitmentAgeForPostconfirmation(epochDuration); // Set min commitment age to 1/10 of epochDuration uint256 minAge = epochDuration/10; - mcr.setMinCommitmentAge(minAge); + mcr.setMinCommitmentAgeForPostconfirmation(minAge); assertEq(mcr.minCommitmentAgeForPostconfirmation(), minAge, "Min commitment age should be updated to 1/10 of epochDuration"); } @@ -785,7 +785,7 @@ contract MCRTest is Test, IMCR { address alice = setupGenesisWithOneAttester(1); assertEq(mcr.getMinCommitmentAgeForPostconfirmation(), 0, "The unset min commitment age should be 0"); uint256 minAge = 1 minutes; - mcr.setMinCommitmentAge(minAge); + mcr.setMinCommitmentAgeForPostconfirmation(minAge); assertEq(mcr.getMinCommitmentAgeForPostconfirmation(), minAge, "Min commitment age should be updated to 1 minutes"); vm.prank(alice); @@ -943,29 +943,39 @@ contract MCRTest is Test, IMCR { function testPostconfirmationRewards() public { - // Setup with Alice having supermajority-enabling stake - address alice = setupGenesisWithOneAttester(1); - assertEq(moveToken.balanceOf(alice), 0, "Alice should have 0 tokens"); - - // Attester attests to height 1 + uint256 stake = 7; + address alice = setupGenesisWithOneAttester(stake); + uint256 aliceInitialBalance = moveToken.balanceOf(alice); + assertEq(aliceInitialBalance, 0, "Alice should have 0 tokens"); + + // submit commitment vm.prank(alice); mcr.submitSuperBlockCommitment(makeHonestCommitment(1)); - // get out of genesis epoch + // attempt postconfirmation + vm.prank(alice); + mcr.postconfirmSuperBlocksAndRollover(); + // balance of alice should have not increased yet + assertEq(moveToken.balanceOf(alice), aliceInitialBalance, "Alice should have not received any rewards yet"); + assertEq(mcr.getLastPostconfirmedSuperBlockHeight(), 1, "Last postconfirmed superblock height should be 1"); + assertEq(mcr.getAttesterRewardPoints(mcr.getAcceptingEpoch(), alice), 1, "Alice should have 1 attester points"); + assertEq(mcr.getPostconfirmerRewardPoints(mcr.getAcceptingEpoch(), alice), 1, "Alice should have 1 postconfirmer points"); + assertEq(mcr.getAcceptingEpoch(), 0, "Should be in epoch 0"); + + // warp to next epoch vm.warp(block.timestamp + epochDuration); - - // Attester postconfirms and gets a reward vm.prank(alice); mcr.postconfirmSuperBlocksAndRollover(); assertEq(mcr.getLastPostconfirmedSuperBlockHeight(), 1); assertEq(mcr.getAcceptingEpoch(), 1, "Should be in epoch 1"); - assertEq(mcr.getStakeForAcceptingEpoch(address(moveToken), alice), 1, "Alice should have 1 token on stake"); - assertEq(moveToken.balanceOf(alice), 1, "Alice should have 1 token on balance"); + // Verify rewards: + // 1. Attestation reward: stake * rewardPerPoint * points (7 * 1 * 1) + // 2. Postconfirmation reward: stake * rewardPerPoint * points (7 * 1 * 1) + assertEq(moveToken.balanceOf(alice), aliceInitialBalance + stake + stake, "Alice should have received the rewards"); } - // ---------------------------------------------------------------- // -------- Acceptor rewards -------------------------------------- // ----------------------------------------------------------------