Skip to content

Commit

Permalink
use time for acceptor terms, not blocks
Browse files Browse the repository at this point in the history
  • Loading branch information
@apenzk
apenzk committed Mar 2, 2025
1 parent 7239055 commit 4f15c69
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 103 deletions.
59 changes: 30 additions & 29 deletions protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR {
bytes32 public constant TRUSTED_ATTESTER = keccak256("TRUSTED_ATTESTER");

/// @notice Error thrown when acceptor term is greater than 256 blocks
error AcceptorTermTooLong();
error AcceptorDurationTooLong();

/// @notice Error thrown when acceptor term is too large for epoch duration
error AcceptorTermTooLongForEpoch();
error AcceptorDurationTooLongForEpoch();

/// @notice Error thrown when minimum commitment age is greater than epoch duration
error MinCommitmentAgeTooLong();
Expand All @@ -31,25 +31,20 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR {
error MaxAcceptorNonReactivityTimeTooLong();

/// @notice Sets the acceptor term duration, must be less than epoch duration
/// @param _acceptorTerm New acceptor term duration in time units
function setAcceptorTerm(uint256 _acceptorTerm) public onlyRole(COMMITMENT_ADMIN) {
// Ensure acceptor term is not longer than 256 blocks
if (_acceptorTerm > 256) {
revert AcceptorTermTooLong();
}
/// @param _acceptorDuration New acceptor term duration in time units
function setAcceptorDuration(uint256 _acceptorDuration) public onlyRole(COMMITMENT_ADMIN) {
// Ensure acceptor term is sufficiently small compared to epoch duration
uint256 epochDuration = stakingContract.getEpochDuration(address(this));

// TODO If we would use block heights instead of timestamps we could handle everything much smoother.
uint256 estimatedL1BlockDelta = 12 seconds;
if (2 * _acceptorTerm >= epochDuration / estimatedL1BlockDelta) {
revert AcceptorTermTooLongForEpoch();
if (2 * _acceptorDuration >= epochDuration ) {
revert AcceptorDurationTooLongForEpoch();
}
acceptorTerm = _acceptorTerm;
acceptorDuration = _acceptorDuration;
}

function getAcceptorTerm() public view returns (uint256) {
return acceptorTerm;
function getAcceptorDuration() public view returns (uint256) {
return acceptorDuration;
}

/// @notice Sets the minimum time that must pass before a commitment can be postconfirmed
Expand Down Expand Up @@ -91,7 +86,7 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR {
uint256 _leadingSuperBlockTolerance,
uint256 _epochDuration, // in time units
address[] memory _custodians,
uint256 _acceptorTerm, // in time units
uint256 _acceptorDuration, // in time units
address _moveTokenAddress // the primary custodian for rewards in the staking contract
) public initializer {
__BaseSettlement_init_unchained();
Expand All @@ -101,7 +96,7 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR {
stakingContract.registerDomain(_epochDuration, _custodians);
grantCommitmentAdmin(msg.sender);
grantTrustedAttester(msg.sender);
acceptorTerm = _acceptorTerm;
acceptorDuration = _acceptorDuration;
moveTokenAddress = _moveTokenAddress;

// Set default values to 1/10th of epoch duration
Expand Down Expand Up @@ -391,26 +386,32 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR {
return true;
}

/// @notice Gets the block height at which the current acceptor's term started
function getAcceptorStartL1BlockHeight(uint256 currentL1Block) public view returns (uint256) {
uint256 currentL1BlockCorrected = currentL1Block - 1; // The first block is 1, not 0
return currentL1BlockCorrected - (currentL1BlockCorrected % acceptorTerm) + 1;
/// @notice Gets the time at which the current epoch started
function getEpochStartTime() public view returns (uint256) {
uint256 currentTime = block.timestamp;
return currentTime - (currentTime % stakingContract.getEpochDuration(address(this)));
}

/// @notice Gets the time at which the current acceptor's term started
function getAcceptorStartTime() public view returns (uint256) {
uint256 currentTime = block.timestamp;
// We reset the times to match the start of epochs. This ensures every epoch has the same setup.
uint256 currentTimeCorrected = currentTime % stakingContract.getEpochDuration(address(this));
return currentTimeCorrected - (currentTimeCorrected % acceptorDuration);
}

/// @notice Determines the acceptor in the accepting epoch using L1 block hash as a source of randomness
// At the border between epochs this is not ideal as getAcceptor works on blocks and epochs works with time.
// Thus we must consider the edge cases where the acceptor is only active for a short time.
function getAcceptor() public view returns (address) {
uint256 currentL1Block = block.number;
uint256 acceptorStartL1Block = getAcceptorStartL1BlockHeight(currentL1Block);
require(acceptorStartL1Block > 0, "Acceptor start block should not be 0");
require(acceptorStartL1Block <= currentL1Block, "Acceptor start block is in the future");
require(currentL1Block - acceptorStartL1Block <= 256, "Acceptor start block is too old, as data is not available for more than 256 blocks");
bytes32 randomness = blockhash(acceptorStartL1Block-1);
require(randomness != 0, "Block too old for randomness");
// TODO unless we swap with everything, including epochs, we must use block.timestamp.
// However, to get easy access to L1 randomness we need to know the block at which the acceptor started, which is expensive (unless we count in blocks instead of time)
// TODO as long as we do not swap to block.number, the randomness below is precictable.
uint256 randSeed1 = getAcceptorStartTime();
uint256 randSeed2 = getEpochStartTime();
address[] memory attesters = stakingContract.getStakedAttestersForAcceptingEpoch(address(this));
uint256 acceptorIndex = uint256(randomness) % attesters.length;
return attesters[acceptorIndex];
uint256 acceptorIndex = uint256(keccak256(abi.encodePacked(randSeed1, randSeed2))) % attesters.length; // randomize the order of the attesters
return attesters[acceptorIndex];
}

/// @dev it is possible if the accepting epoch is behind the presentEpoch that heights dont obtain enough votes in the assigned epoch.
Expand Down
9 changes: 3 additions & 6 deletions protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,10 @@ contract MCRStorage {
// track the last postconfirmed superBlock height, so that we can require superBlocks are submitted in order and handle staking effectively
uint256 public lastPostconfirmedSuperBlockHeight;

/// Acceptor term time in seconds (determined by L1 blocks). The confimer remains the same for acceptorTerm period.
// This means we accept that if the acceptor is not active the postconfirmations will be delayed.
// TODO permit that anyone can confirm but only the Acceptor gets rewarded.
// TODO The Acceptor should also get rewarded even if another attestor confirmed the postconfirmation.
// The Acceptor term can be minimal, but it should not be O(1) as the acceptor should have some time
/// Acceptor term time in seconds. The acceptor remains the same for acceptorDuration period.
// The Acceptor term can be minimal, but it should not be too small as the acceptor should have some time
// to prepare and post L1-transactions that will start the validation of attestations.
uint256 public acceptorTerm;
uint256 public acceptorDuration;

/// @notice Minimum time that must pass before a commitment can be postconfirmed
uint256 public minCommitmentAgeForPostconfirmation;
Expand Down
122 changes: 54 additions & 68 deletions protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ contract MCRTest is Test, IMCR {
string public stakingSignature = "initialize(address)";
string public mcrSignature = "initialize(address,uint256,uint256,uint256,address[],uint256,address)";
uint256 epochDuration = 7200 seconds;
uint256 acceptorTerm = epochDuration/12 seconds/4;
uint256 acceptorDuration = epochDuration/12 seconds/4;
bytes32 honestCommitmentTemplate = keccak256(abi.encodePacked(uint256(1), uint256(2), uint256(3)));
bytes32 honestBlockIdTemplate = keccak256(abi.encodePacked(uint256(1), uint256(2), uint256(3)));
bytes32 dishonestCommitmentTemplate = keccak256(abi.encodePacked(uint256(3), uint256(2), uint256(1)));
Expand Down Expand Up @@ -88,7 +88,7 @@ contract MCRTest is Test, IMCR {
5, // _leadingSuperBlockTolerance, max blocks ahead of last confirmed
epochDuration, // _epochDuration, how long an epoch lasts, constant stakes in that time
custodians, // _custodians, array with moveProxy address
acceptorTerm, // _acceptorTerm, how long an acceptor serves
acceptorDuration, // _acceptorDuration, how long an acceptor serves
// TODO can we replace the following line with the moveToken address?
address(moveProxy) // _moveTokenAddress, the primary custodian for rewards in the staking contract
);
Expand Down Expand Up @@ -321,7 +321,7 @@ contract MCRTest is Test, IMCR {
}

// ----------------------------------------------------------------
// -------- Test functions ----------------------------------------
// -------- General tests ----------------------------------------
// ----------------------------------------------------------------

function testCannotInitializeTwice() public {
Expand Down Expand Up @@ -771,8 +771,6 @@ contract MCRTest is Test, IMCR {
assertEq(mcr.getLastPostconfirmedSuperBlockHeight(), 1, "Last postconfirmed superblock height should be 1, as supermajority was reached (2/2 > threshold)");
}



function testSetMinCommitmentAge() public {
// Set min commitment age to a too long value
vm.expectRevert(MCR.MinCommitmentAgeTooLong.selector);
Expand Down Expand Up @@ -808,82 +806,81 @@ contract MCRTest is Test, IMCR {


// ----------------------------------------------------------------
// -------- Acceptor --------------------------------------
// -------- Acceptor tests --------------------------------------
// ----------------------------------------------------------------

/// @notice Test that getAcceptorStartTime correctly calculates term start times
function testAcceptorStartL1BlockHeight() public {
function testAcceptorStartTime() public {
// Test at block 0
assertEq(mcr.getAcceptorStartL1BlockHeight(block.number), 1, "Acceptor term should start at L1Block 1");
assertEq(block.timestamp, 1, "Current time should be 1"); // TODO why is it 1? and not 0?
assertEq(acceptorDuration, mcr.getAcceptorDuration(), "Acceptor term should be correctly set");
assertEq(mcr.getAcceptorStartTime(), 0, "Acceptor term should start at (1) time 0");

// Test at half an acceptor term
vm.roll(acceptorTerm/2);
assertEq(mcr.getAcceptorStartL1BlockHeight(block.number), 1, "Acceptor term should start at L1Block 1");
vm.warp(acceptorDuration-1);
assertEq(mcr.getAcceptorStartTime(), 0, "Acceptor term should start at (2) time 0");

// Test at an acceptor term boundary
vm.roll(acceptorTerm);
assertEq(mcr.getAcceptorStartL1BlockHeight(block.number), 1, "Acceptor term should start at L1Block 1");
vm.warp(acceptorDuration);
console.log("current time", block.timestamp);
console.log("acceptorDuration", acceptorDuration);
console.log("epochTime", epochDuration);
assertEq(mcr.getAcceptorStartTime(), acceptorDuration, "Acceptor term should start at (3) time acceptorDuration");

// Test at an acceptor term boundary
vm.roll(acceptorTerm+1);
assertEq(mcr.getAcceptorStartL1BlockHeight(block.number), acceptorTerm+1, "Acceptor term should start at L1Block acceptorTerm+1");
vm.warp(acceptorDuration+1);
assertEq(mcr.getAcceptorStartTime(), acceptorDuration, "Acceptor term should start at (4) time acceptorDuration");

// Test at 1.5 acceptor terms
vm.warp(3 * acceptorTerm / 2);
assertEq(mcr.getAcceptorStartL1BlockHeight(block.number), acceptorTerm+1, "Acceptor term should start at L1Block acceptorTerm+1");
vm.warp(2 * acceptorDuration );
assertEq(mcr.getAcceptorStartTime(), 2 * acceptorDuration, "Acceptor term should start at (5) time 2 * acceptorDuration");
}

/// @notice Test setting acceptor term with validation
function testSetAcceptorTerm() public {
// Ensure we can retrieve the epoch duration correct
assertEq(epochDuration, staking.getEpochDuration(address(mcr)));

// Set acceptor term to 256 blocks (should succeed)
mcr.setAcceptorTerm(256);
assertEq(mcr.acceptorTerm(), 256, "Term should be updated to 256");

// Try setting acceptor term to over 256 blocks (should fail)
vm.expectRevert(MCR.AcceptorTermTooLong.selector);
mcr.setAcceptorTerm(257);
assertEq(mcr.acceptorTerm(), 256, "Term should remain at 256");

// Check validity with respect to epoch duration
uint256 validTerm = epochDuration/12 seconds/4;
mcr.setAcceptorTerm(validTerm);
assertEq(mcr.acceptorTerm(), validTerm, "Term should be updated to epoch related value");

// Try setting acceptor term to epoch duration
uint256 invalidTerm = epochDuration/12 seconds;
vm.expectRevert(MCR.AcceptorTermTooLong.selector);
mcr.setAcceptorTerm(invalidTerm);
assertEq(mcr.acceptorTerm(), validTerm, "Term should remain at epoch related value");
/// @notice Test setting acceptor duration with validation
function testSetAcceptorDuration() public {
// Check the epoch duration is set correctly
assertEq(epochDuration, staking.getEpochDuration(address(mcr)));
// Test valid duration (less than half epoch duration)
uint256 validDuration = epochDuration / 2 - 1;
mcr.setAcceptorDuration(validDuration);
assertEq(mcr.getAcceptorDuration(), validDuration, "Duration should be updated to valid value");

// Test duration too long compared to epoch (>= epochDuration/2)
uint256 invalidDuration = epochDuration / 2;
vm.expectRevert(MCR.AcceptorDurationTooLongForEpoch.selector);
mcr.setAcceptorDuration(invalidDuration);
assertEq(mcr.getAcceptorDuration(), validDuration, "Duration should remain at previous valid value");

// Test duration equal to epoch duration (should fail)
vm.expectRevert(MCR.AcceptorDurationTooLongForEpoch.selector);
mcr.setAcceptorDuration(epochDuration);
assertEq(mcr.getAcceptorDuration(), validDuration, "Duration should remain at previous valid value");
}

/// @notice Test that getAcceptor correctly selects an acceptor based on block hash
function testGetAcceptor() public {
// Setup with three attesters with equal stakes
(address alice, , address carol) = setupGenesisWithThreeAttesters(1, 1, 1);
(, address bob, address carol) = setupGenesisWithThreeAttesters(1, 1, 1);
uint256 myAcceptorDuration = 13;
mcr.setAcceptorDuration(myAcceptorDuration);
assertEq(myAcceptorDuration,mcr.getAcceptorDuration(),"Acceptor duration not set correctly");

uint256 myAcceptorTerm = 4;
mcr.setAcceptorTerm(myAcceptorTerm);
address initialAcceptor = mcr.getAcceptor();
assertTrue( initialAcceptor == carol, "Acceptor should be Carol");
assertEq(initialAcceptor, bob, "Acceptor should be bob");

vm.roll(2); // we started at block 1
assertEq(mcr.getAcceptor(), initialAcceptor, "Acceptor should not change within term");

vm.roll(myAcceptorTerm); // L1blocks started at 1, not 0
vm.warp(myAcceptorDuration-1);
assertEq(mcr.getAcceptor(), initialAcceptor, "Acceptor should not change within term");

// Move to next acceptor Term
vm.roll(myAcceptorTerm+1); // L1blocks started at 1, not 0
// Move two acceptor terms (moving one resulted still in bob as acceptor with current randomness)
vm.warp(2*myAcceptorDuration);
address newAcceptor = mcr.getAcceptor();
assertTrue( newAcceptor == alice, "New acceptor should be Alice");
assertEq(mcr.getAcceptorStartTime(),2*myAcceptorDuration,"Acceptor start time should be myAcceptorDuration");
assertEq(newAcceptor, carol, "New acceptor should be Carol");
}


// ----------------------------------------------------------------
// -------- Attester rewards --------------------------------------
// -------- Attester reward tests --------------------------------------
// ----------------------------------------------------------------

function testAttesterRewardPoints() public {
Expand Down Expand Up @@ -945,17 +942,6 @@ contract MCRTest is Test, IMCR {
assertEq(moveToken.balanceOf(carol), carolInitialBalance + mcr.getStakeForAcceptingEpoch(address(moveToken), carol), "Carol reward not correct.");
}

/// @notice Test that the acceptor privilege window works correctly
function testAcceptorPrivilegeWindow() public {
address alice = setupGenesisWithOneAttester(1);

// set the max acceptor non-reactivity time to 1/4 epochDuration
mcr.setAcceptorPrivilegeWindow(epochDuration/4);
assertEq(mcr.getMaxAcceptorNonReactivityTime(), epochDuration/4, "Max acceptor non-reactivity time should be 1/4 epochDuration");


}

/// @notice Test that postconfirmation rewards are distributed correctly when the acceptor is live
function testPostconfirmationRewardsLiveAcceptor() public {
uint256 stake = 7;
Expand Down Expand Up @@ -1050,16 +1036,16 @@ contract MCRTest is Test, IMCR {
(address alice, address bob, ) = setupGenesisWithThreeAttesters(aliceStake, bobStake, 0);
uint256 aliceInitialBalance = moveToken.balanceOf(alice);
uint256 bobInitialBalance = moveToken.balanceOf(bob);
uint256 thisAcceptorTerm = mcr.getAcceptorTerm();
uint256 thisAcceptorDuration = mcr.getAcceptorDuration();

// set the time windows
assertEq(mcr.getMinCommitmentAgeForPostconfirmation(), 0, "Min commitment age should be 0");
uint256 thisAcceptorPriviledgeWindow = epochDuration/100;
mcr.setAcceptorPrivilegeWindow(thisAcceptorPriviledgeWindow);
assertEq(mcr.getMaxAcceptorNonReactivityTime(), thisAcceptorPriviledgeWindow, "Max acceptor non-reactivity time should be 1/100 epochDuration");
console.log("getMaxAcceptorNonReactivityTime", mcr.getMaxAcceptorNonReactivityTime());
console.log("thisAcceptorTerm", thisAcceptorTerm);
assertGt(thisAcceptorTerm, thisAcceptorPriviledgeWindow, "Acceptor term should be greater than thisAcceptorPriviledgeWindow");
console.log("thisAcceptorDuration", thisAcceptorDuration);
assertGt(thisAcceptorDuration, thisAcceptorPriviledgeWindow, "Acceptor term should be greater than thisAcceptorPriviledgeWindow");

vm.prank(alice);
mcr.submitSuperBlockCommitment(makeHonestCommitment(1));
Expand Down Expand Up @@ -1088,11 +1074,11 @@ contract MCRTest is Test, IMCR {
}

// ----------------------------------------------------------------
// -------- Acceptor rewards --------------------------------------
// -------- Acceptor reward tests --------------------------------------
// ----------------------------------------------------------------


// An acceptor that is in place for acceptorTerm time should be replaced by a new acceptor after their term ended.
// An acceptor that is in place for acceptorDuration time should be replaced by a new acceptor after their term ended.
// TODO reward logic is not yet implemented
function testAcceptorRewards() public {
(address alice, address bob, ) = setupGenesisWithThreeAttesters(1, 1, 0);
Expand Down

0 comments on commit 4f15c69

Please sign in to comment.