Skip to content

Commit

Permalink
block.number starts at 1
Browse files Browse the repository at this point in the history
  • Loading branch information
@apenzk
apenzk committed Feb 24, 2025
1 parent 2d2c5dc commit e315a31
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 54 deletions.
36 changes: 14 additions & 22 deletions protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,12 +309,6 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR {
/// @notice If the current acceptor is not live, we should accept postconfirmations from any attester
// TODO: this will be improved, such that the first voluntary attester to do sowill be rewarded
function postconfirmAndRolloverWithAttester(address /* attester */) internal {
// if the current acceptor is live we should not accept postconfirmations from voluntary attesters
// TODO: we probably have to apply this check somewhere else as (volunteer) attesters can only postconfirm and rollover an epoch in which they are staked.
if (currentAcceptorIsLive()) {
// TODO: for now everyone can postconfirm, but change this later
// if (attester != getAcceptor()) revert("NotAcceptorAndAcceptorIsLive");
}

// keep ticking through postconfirmations and rollovers as long as the acceptor is permitted to do
// ! rewards need to be
Expand All @@ -331,31 +325,29 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR {
}

/// @notice Gets the block height at which the current acceptor's term started
function getAcceptorStartL1BlockHeight() public view returns (uint256) {
uint256 currentBlock = block.number;
return currentBlock - (currentBlock % acceptorTerm);
}

// TODO we need to think of a solution that is not dependent on the block time as finding the correct block is expensive. We need something simple and cheap.
// TODO For example think in terms of L1 block heights, rather than timestamps both for epochs and acceptor terms.
/// @notice Gets the L1 block number that is closest to but not exceeding the given timestamp
function getClosestL1BlockToTime(uint256 targetTime) public view returns (uint256) {
// TODO: implement this
return 0; // dummy implementation
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 Determines the acceptor in the accepting epoch using L1 block hash as a source of randomness
// TODO at the border between epochs this is currently not ideal as getAcceptor works on blocks and epochs works with thime.
// TODO consider using block numbers instead of timestamps for epochs, and have epochs as multiple of acceptorTerm
function getAcceptor() public view returns (address) {
bytes32 randomness = blockhash(getAcceptorStartL1BlockHeight());
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");
address[] memory attesters = stakingContract.getStakedAttestersForAcceptingEpoch(address(this));
uint256 acceptorIndex = uint256(randomness) % attesters.length;
return attesters[acceptorIndex];
}

// TODO : liveness. if the accepting epoch is behind the presentEpoch and does not have enough votes for a given block height
// TODO : but the current epoch has enough votes, what should we do??
// TODO : Should we move to the next epoch and ignore all votes on blocks of that epoch?
// TODO : What if none of the epochs have enough votes for a given block height.
// TODO : liveness. if the accepting epoch is behind the presentEpoch and does not have enough votes for a given block height.
// TODO : Suggestion: move to the next epoch and counts votes there
function attemptPostconfirmOrRollover(uint256 superBlockHeight) internal returns (bool) {
uint256 superBlockEpoch = superBlockHeightAssignedEpoch[superBlockHeight];
// ensure that the superBlock height is equal or above the lastPostconfirmedSuperBlockHeight
Expand Down
69 changes: 37 additions & 32 deletions protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -610,21 +610,25 @@ contract MCRTest is Test, IMCR {

/// @notice Test that getAcceptorStartTime correctly calculates term start times
function testAcceptorStartL1BlockHeight() public {
// Test at time 0
console.log("Current L1Block time:", block.timestamp);
assertEq(mcr.getAcceptorStartL1BlockHeight(), 0, "Acceptor term should start at time 0");
// Test at block 0
uint256 currentL1Block = block.number;
assertEq(mcr.getAcceptorStartL1BlockHeight(block.number), 1, "Acceptor term should start at L1Block 1");

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

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

// 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");

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

/// @notice Test setting acceptor term with validation
Expand Down Expand Up @@ -656,28 +660,34 @@ contract MCRTest is Test, IMCR {
/// @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 alice, address bob, address carol) = setupGenesisWithThreeAttesters(1, 1, 1);

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

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
assertEq(mcr.getAcceptor(), initialAcceptor, "Acceptor should not change within term");

// Move to next acceptor Term
vm.roll(acceptorTerm);
vm.roll(myAcceptorTerm+1); // L1blocks started at 1, not 0
address newAcceptor = mcr.getAcceptor();
assertTrue( newAcceptor == alice, "New acceptor should be Alice");
}

// An acceptor that is in place for acceptorTerm time should be replaced by a new acceptor after their term ended.
// TODO reward logic is not yet implemented
function testAcceptorRewards() public {
// Setup, with carol having no stake
(address alice, address bob, ) = setupGenesisWithThreeAttesters(50, 50, 0);
(address alice, address bob, address carol) = setupGenesisWithThreeAttesters(1, 1, 0);
assertEq(mcr.getAcceptor(), bob, "Bob should be the acceptor");

// TODO why do we need to whitelist the address?
staking.whitelistAddress(alice);
staking.whitelistAddress(bob);
// staking.whitelistAddress(alice);
// staking.whitelistAddress(bob);

// make superBlock commitments
MCRStorage.SuperBlockCommitment memory initCommitment = newHonestCommitment(1);
Expand All @@ -686,35 +696,30 @@ contract MCRTest is Test, IMCR {
vm.prank(bob);
mcr.submitSuperBlockCommitment(initCommitment);

// check that alice is the current acceptor
// TODO: getAcceptor does not yet work.
// assertEq(mcr.getAcceptor(), alice);
console.log("WARNING: Test not correct yet, as getAcceptor does not work");

// TODO : here we should check that the reward goes only to alice
// alice can confirm the block comittment and get a reward
// TODO check that bob did not get the reward
// bob postconfirms and gets a reward
vm.prank(bob);
mcr.postconfirmSuperBlocksAndRollover();
assertEq(mcr.getLastPostconfirmedSuperBlockHeight(), 1);

// Alice tries to postconfirm
// TODO: Alice should still get the reward
vm.prank(alice);
mcr.postconfirmSuperBlocksAndRollover();
assertEq(mcr.getLastPostconfirmedSuperBlockHeight(), 1);

// make second superblock commitment
MCRStorage.SuperBlockCommitment memory secondCommitment = newHonestCommitment(2);
vm.prank(alice);
mcr.submitSuperBlockCommitment(secondCommitment);
vm.prank(bob);
mcr.submitSuperBlockCommitment(secondCommitment);

// alice can confirm the block comittment and get a reward

// alice can postconfirm, but does not get the reward
// TODO check that bob did not get the reward
vm.prank(alice);
mcr.postconfirmSuperBlocksAndRollover();
assertEq(mcr.getLastPostconfirmedSuperBlockHeight(), 2);

// bob tries to postconfirm, but already done by alice
// TODO: bob should still get the reward
vm.prank(bob);
mcr.postconfirmSuperBlocksAndRollover();
assertEq(mcr.getLastPostconfirmedSuperBlockHeight(), 2);

}

}

0 comments on commit e315a31

Please sign in to comment.