Skip to content

fix(protocol): fix _getOperatorForEpoch in PreconfWhitelist.sol #19329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed

fix(protocol): fix _getOperatorForEpoch in PreconfWhitelist.sol #19329

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fc8cc56
Update PreconfWhitelist.sol
dantaik Apr 24, 2025
65df2ad
fix
dantaik Apr 24, 2025
81206b1
Update LibPreconfUtils.t.sol
dantaik Apr 24, 2025
bdad9ad
fix tests
dantaik Apr 24, 2025
541f285
forge fmt & update contract layout tables
dantaik Apr 24, 2025
18a0b1a
more changes
dantaik Apr 24, 2025
3290b9a
Merge branch 'additional_whitelist_epoch_fix-' of https://github.com/…
dantaik Apr 24, 2025
4a984b3
more
dantaik Apr 24, 2025
87387c3
Update PreconfWhitelist.sol
dantaik Apr 24, 2025
be2a275
rename
dantaik Apr 24, 2025
2612b1e
Update PreconfWhitelist.sol
dantaik Apr 24, 2025
b78f33c
forge fmt & update contract layout tables
dantaik Apr 24, 2025
d4c5334
Merge branch 'main' into additional_whitelist_epoch_fix-
dantaik Apr 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions packages/protocol/.gas-snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ InboxTest_ProposeAndProve:test_inbox_reprove_the_same_batch_with_same_transition
InboxTest_ProposeAndProve:test_inbox_ring_buffer_will_be_reused() (gas: 14705853)
InboxTest_ProposeAndProve:test_proposeBatch_reverts_for_invalid_proposer_and_operator() (gas: 165337)
InboxTest_StopBatch:test_inbox_num_batches_verified() (gas: 8162653)
PreconfRouterTest:test_preconfRouter_proposeBatch() (gas: 374389)
PreconfRouterTest:test_preconfRouter_proposeBatch_notOperator() (gas: 325555)
PreconfRouterTest:test_preconfRouter_proposeBatch_proposerNotSender() (gas: 373120)
PreconfRouterTest:test_preconfRouter_proposeBatch() (gas: 375140)
PreconfRouterTest:test_preconfRouter_proposeBatch_notOperator() (gas: 326307)
PreconfRouterTest:test_preconfRouter_proposeBatch_proposerNotSender() (gas: 373872)
SendMessageToDelegateOwner:testAddress1() (gas: 2391)
TestAutomataDcapV3Attestation:testAttestation() (gas: 4848498)
TestAutomataDcapV3Attestation:testParsedCustomQuoteBinAttestation() (gas: 4553350)
Expand Down Expand Up @@ -107,16 +107,16 @@ TestMinimalOwner:test_minimalOwner_InitialOwner() (gas: 13083)
TestMinimalOwner:test_minimalOwner_TransferOwnership() (gas: 21879)
TestMinimalOwner:test_minimalOwner_TransferOwnershipToSameAddress() (gas: 13832)
TestMinimalOwner:test_minimalOwner_TransferOwnershipToZeroAddress() (gas: 13493)
TestPreconfWhitelist:test_whitelistNoDelay_consolidationPreservesOrder() (gas: 252745)
TestPreconfWhitelist:test_whitelistNoDelay_consolidationWillNotChangeCurrentEpochOperator() (gas: 299356)
TestPreconfWhitelist:test_whitelist_addBackRemovedOperator() (gas: 106230)
TestPreconfWhitelist:test_whitelist_addOrRemoveTheSameOperatorTwiceWillRevert() (gas: 83296)
TestPreconfWhitelist:test_whitelist_consolidate_whenEmpty_not_revert() (gas: 19756)
TestPreconfWhitelist:test_whitelist_delay2epoch_addThenRemoveOneOperator() (gas: 257049)
TestPreconfWhitelist:test_whitelist_delay2epoch_addThenRemoveTwoOperators() (gas: 305557)
TestPreconfWhitelist:test_whitelist_noDelay_addThenRemoveOneOperator() (gas: 154524)
TestPreconfWhitelist:test_whitelist_removeNonExistingOperatorWillRevert() (gas: 25210)
TestPreconfWhitelist:test_whitelist_selfRemoval() (gas: 174701)
TestPreconfWhitelist:test_whitelistNoDelay_consolidationPreservesOrder() (gas: 251985)
TestPreconfWhitelist:test_whitelistNoDelay_consolidationWillNotChangeCurrentEpochOperator() (gas: 291066)
TestPreconfWhitelist:test_whitelist_addBackRemovedOperator() (gas: 105759)
TestPreconfWhitelist:test_whitelist_addOrRemoveTheSameOperatorTwiceWillRevert() (gas: 82692)
TestPreconfWhitelist:test_whitelist_consolidate_whenEmpty_not_revert() (gas: 19712)
TestPreconfWhitelist:test_whitelist_delay2epoch_addThenRemoveOneOperator() (gas: 245550)
TestPreconfWhitelist:test_whitelist_delay2epoch_addThenRemoveTwoOperators() (gas: 294371)
TestPreconfWhitelist:test_whitelist_noDelay_addThenRemoveOneOperator() (gas: 158506)
TestPreconfWhitelist:test_whitelist_removeNonExistingOperatorWillRevert() (gas: 23065)
TestPreconfWhitelist:test_whitelist_selfRemoval() (gas: 139154)
TestTaikoTreasuryVault:testForwardCallFailsOnError() (gas: 79082)
TestTaikoTreasuryVault:testForwardCallToSelfShouldRevert() (gas: 26006)
TestTaikoTreasuryVault:testNonOwnerCannotForwardCall() (gas: 28382)
Expand Down
1 change: 1 addition & 0 deletions packages/protocol/contracts/layer1/preconf/iface/IPreconfWhitelist.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface IPreconfWhitelist {
error InvalidOperatorIndex();
error InvalidOperatorCount();
error InvalidOperatorAddress();
error SelectorEpochOffsetTooSmall();
error OperatorAlreadyExists();
error OperatorAlreadyRemoved();
error OperatorNotAvailableYet();
Expand Down
54 changes: 32 additions & 22 deletions packages/protocol/contracts/layer1/preconf/impl/PreconfWhitelist.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,38 @@ contract PreconfWhitelist is EssentialContract, IPreconfWhitelist {
}

event Consolidated(uint8 previousCount, uint8 newCount, bool havingPerfectOperators);
event OperatorChangeDelaySet(uint8 delay);

uint256 public immutable selectorEpochOffset; // in epochs
uint256 public immutable operatorChangeDelay; // in epochs

// Slot 1
mapping(address operator => OperatorInfo info) public operators;

// Slot 2
mapping(uint256 index => address operator) public operatorMapping;

// Slot 3
uint8 public operatorCount;
uint8 public operatorChangeDelay; // in epochs

// all operators in operatorMapping are active and none of them is to be deactivated.
bool public havingPerfectOperators;

uint256[47] private __gap;

constructor() EssentialContract(address(0)) { }

function init(address _owner, uint8 _operatorChangeDelay) external initializer {
__Essential_init(_owner);
constructor(
uint256 _operatorChangeDelay,
uint256 _selectorEpochOffset
)
EssentialContract(address(0))
{
require(_selectorEpochOffset > 1, SelectorEpochOffsetTooSmall());
selectorEpochOffset = _selectorEpochOffset;
operatorChangeDelay = _operatorChangeDelay;
havingPerfectOperators = true;
}

function setOperatorChangeDelay(uint8 _operatorChangeDelay) external onlyOwner {
operatorChangeDelay = _operatorChangeDelay;
emit OperatorChangeDelaySet(_operatorChangeDelay);
function init(address _owner) external initializer {
__Essential_init(_owner);
havingPerfectOperators = true;
}

/// @inheritdoc IPreconfWhitelist
Expand Down Expand Up @@ -161,7 +170,7 @@ contract PreconfWhitelist is EssentialContract, IPreconfWhitelist {
);
}

function _addOperator(address _operator, uint8 _operatorChangeDelay) internal {
function _addOperator(address _operator, uint256 _operatorChangeDelay) internal {
require(_operator != address(0), InvalidOperatorAddress());
require(operators[_operator].activeSince == 0, OperatorAlreadyExists());

Expand All @@ -184,7 +193,7 @@ contract PreconfWhitelist is EssentialContract, IPreconfWhitelist {
emit OperatorAdded(_operator, activeSince);
}

function _removeOperator(address _operator, uint8 _operatorChangeDelay) internal {
function _removeOperator(address _operator, uint256 _operatorChangeDelay) internal {
require(_operator != address(0), InvalidOperatorAddress());
OperatorInfo memory info = operators[_operator];
require(info.inactiveSince == 0, OperatorAlreadyRemoved());
Expand All @@ -209,17 +218,18 @@ contract PreconfWhitelist is EssentialContract, IPreconfWhitelist {

/// @dev The cost of this function is primarily linear with respect to operatorCount.
function _getOperatorForEpoch(uint64 _epochTimestamp) internal view returns (address) {
if (_epochTimestamp < LibPreconfConstants.SECONDS_IN_EPOCH) {
return address(0);
}
uint256 timeShift = selectorEpochOffset * LibPreconfConstants.SECONDS_IN_EPOCH;
// We use the beacon root at or after ` _epochTimestamp - timeShift` as the random number to
// select an operator.
// selectorEpochOffset must be big enough to ensure a non-zero beacon root is
// available and immutable.
Comment on lines +224 to +225
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does having big enough selectorEpochOffset ensure non-zero beacon root? In other words, isn't something like following enough?

Suggested change
// selectorEpochOffset must be big enough to ensure a non-zero beacon root is
// available and immutable.
// selectorEpochOffset must be big enough to ensure that the beacon root is finalized and
// cannot change due to reorg.


// Use the previous epoch's start timestamp as the random number, if it is not available
// (zero), return address(0) directly.
uint256 rand = uint256(
LibPreconfUtils.getBeaconBlockRoot(
_epochTimestamp - LibPreconfConstants.SECONDS_IN_SLOT
)
);
if (_epochTimestamp < timeShift) return address(0);

uint256 rand;
unchecked {
rand = uint256(LibPreconfUtils.getBeaconBlockRoot(_epochTimestamp - timeShift));
}

if (rand == 0) return address(0);

Expand Down
4 changes: 1 addition & 3 deletions packages/protocol/layout/layer1-contracts.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,9 +1346,7 @@
|
| operatorCount | uint8 | 253 | 0 | 1 |
|
| operatorChangeDelay | uint8 | 253 | 1 | 1 |
|
| havingPerfectOperators | bool | 253 | 2 | 1 |
| havingPerfectOperators | bool | 253 | 1 | 1 |
|
| __gap | uint256[47] | 254 | 0 | 1504 |
╰-----------------------------+----------------------------------------------------------+------+--------+-------+---------------------------------------------------------------------╯
Expand Down
4 changes: 2 additions & 2 deletions packages/protocol/script/layer1/based/DeployProtocolOnL1.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,8 @@ contract DeployProtocolOnL1 is DeployCapability {
{
whitelist = deployProxy({
name: "preconf_whitelist",
impl: address(new PreconfWhitelist()),
data: abi.encodeCall(PreconfWhitelist.init, (owner, 2)),
impl: address(new PreconfWhitelist(2, 3)),
data: abi.encodeCall(PreconfWhitelist.init, (owner)),
registerTo: rollupResolver
});

Expand Down
4 changes: 2 additions & 2 deletions packages/protocol/script/layer1/preconf/DeployPreconfContracts.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ contract DeployPreconfContracts is BaseScript {
// Deploy PreconfWhitelist
deploy(
LibStrings.B_PRECONF_WHITELIST,
address(new PreconfWhitelist()),
abi.encodeCall(PreconfWhitelist.init, (contractOwner, 2))
address(new PreconfWhitelist(2, 3)),
abi.encodeCall(PreconfWhitelist.init, (contractOwner))
);

// Deploy PreconfRouter
Expand Down
27 changes: 15 additions & 12 deletions packages/protocol/test/layer1/preconf/router/PreconfRouter.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@ contract PreconfRouterTest is PreconfRouterTestBase {

// Setup mock beacon for operator selection
vm.chainId(1);
uint256 epochOneStart = LibPreconfConstants.getGenesisTimestamp(block.chainid);
uint256 epoch1Start = LibPreconfConstants.getGenesisTimestamp(block.chainid);
// Current epoch
uint256 epochTwoStart = epochOneStart + LibPreconfConstants.SECONDS_IN_EPOCH;
uint256 epoch4Start =
epoch1Start + whitelist.selectorEpochOffset() * LibPreconfConstants.SECONDS_IN_EPOCH;

MockBeaconBlockRoot mockBeacon = new MockBeaconBlockRoot();
bytes32 mockRoot = bytes32(uint256(1)); // This will select Carol

address beaconBlockRootContract = LibPreconfConstants.getBeaconBlockRootContract();
vm.etch(beaconBlockRootContract, address(mockBeacon).code);
MockBeaconBlockRoot(payable(beaconBlockRootContract)).set(
epochOneStart + LibPreconfConstants.SECONDS_IN_SLOT, mockRoot
epoch1Start + LibPreconfConstants.SECONDS_IN_SLOT, mockRoot
);

// Setup block params
Expand All @@ -51,7 +52,7 @@ contract PreconfRouterTest is PreconfRouterTestBase {
});

// Warp to arbitrary slot in epoch 2
vm.warp(epochTwoStart + 2 * LibPreconfConstants.SECONDS_IN_SLOT);
vm.warp(epoch4Start + 2 * LibPreconfConstants.SECONDS_IN_SLOT);

// Prank as Carol (selected operator) and propose blocks
vm.prank(Carol);
Expand All @@ -70,21 +71,22 @@ contract PreconfRouterTest is PreconfRouterTestBase {

// Setup mock beacon for operator selection
vm.chainId(1);
uint256 epochOneStart = LibPreconfConstants.getGenesisTimestamp(block.chainid);
uint256 epoch1Start = LibPreconfConstants.getGenesisTimestamp(block.chainid);
MockBeaconBlockRoot mockBeacon = new MockBeaconBlockRoot();
// Current epoch
uint256 epochTwoStart = epochOneStart + LibPreconfConstants.SECONDS_IN_EPOCH;
uint256 epoch4Start =
epoch1Start + whitelist.selectorEpochOffset() * LibPreconfConstants.SECONDS_IN_EPOCH;

bytes32 mockRoot = bytes32(uint256(1)); // This will select Carol

address beaconBlockRootContract = LibPreconfConstants.getBeaconBlockRootContract();
vm.etch(beaconBlockRootContract, address(mockBeacon).code);
MockBeaconBlockRoot(payable(beaconBlockRootContract)).set(
epochOneStart + LibPreconfConstants.SECONDS_IN_SLOT, mockRoot
epoch1Start + LibPreconfConstants.SECONDS_IN_SLOT, mockRoot
);

// Warp to arbitrary slot in epoch 2
vm.warp(epochTwoStart + 2 * LibPreconfConstants.SECONDS_IN_SLOT);
vm.warp(epoch4Start + 2 * LibPreconfConstants.SECONDS_IN_SLOT);

// Prank as David (not the selected operator) and propose blocks
vm.prank(David);
Expand All @@ -101,17 +103,18 @@ contract PreconfRouterTest is PreconfRouterTestBase {

// Setup mock beacon for operator selection
vm.chainId(1);
uint256 epochOneStart = LibPreconfConstants.getGenesisTimestamp(block.chainid);
uint256 epoch1Start = LibPreconfConstants.getGenesisTimestamp(block.chainid);
// Current epoch
uint256 epochTwoStart = epochOneStart + LibPreconfConstants.SECONDS_IN_EPOCH;
uint256 epoch4Start =
epoch1Start + whitelist.selectorEpochOffset() * LibPreconfConstants.SECONDS_IN_EPOCH;

MockBeaconBlockRoot mockBeacon = new MockBeaconBlockRoot();
bytes32 mockRoot = bytes32(uint256(1)); // This will select Carol

address beaconBlockRootContract = LibPreconfConstants.getBeaconBlockRootContract();
vm.etch(beaconBlockRootContract, address(mockBeacon).code);
MockBeaconBlockRoot(payable(beaconBlockRootContract)).set(
epochOneStart + LibPreconfConstants.SECONDS_IN_SLOT, mockRoot
epoch1Start + LibPreconfConstants.SECONDS_IN_SLOT, mockRoot
);

// Setup block params
Expand All @@ -137,7 +140,7 @@ contract PreconfRouterTest is PreconfRouterTestBase {
});

// Warp to arbitrary slot in epoch 2
vm.warp(epochTwoStart + 2 * LibPreconfConstants.SECONDS_IN_SLOT);
vm.warp(epoch4Start + 2 * LibPreconfConstants.SECONDS_IN_SLOT);

// Prank as Carol (selected operator) and propose blocks
vm.prank(Carol);
Expand Down
4 changes: 2 additions & 2 deletions packages/protocol/test/layer1/preconf/router/PreconfRouterTestBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ abstract contract PreconfRouterTestBase is Layer1Test {
whitelist = PreconfWhitelist(
deploy({
name: "preconf_whitelist",
impl: address(new PreconfWhitelist()),
data: abi.encodeCall(PreconfWhitelist.init, (whitelistOwner, 2))
impl: address(new PreconfWhitelist(2, 3)),
data: abi.encodeCall(PreconfWhitelist.init, (whitelistOwner))
})
);

Expand Down
11 changes: 7 additions & 4 deletions packages/protocol/test/layer1/preconf/whitelist/PeconfWhitelist.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@ contract TestPreconfWhitelist is Layer1Test {
whitelist = PreconfWhitelist(
deploy({
name: "preconf_whitelist",
impl: address(new PreconfWhitelist()),
data: abi.encodeCall(PreconfWhitelist.init, (whitelistOwner, 2))
impl: address(new PreconfWhitelist(2, 3)),
data: abi.encodeCall(PreconfWhitelist.init, (whitelistOwner))
})
);

whitelistNoDelay = PreconfWhitelist(
deploy({
name: "preconf_whitelist_nodelay",
impl: address(new PreconfWhitelist()),
data: abi.encodeCall(PreconfWhitelist.init, (whitelistOwner, 0))
impl: address(new PreconfWhitelist(0, 3)),
data: abi.encodeCall(PreconfWhitelist.init, (whitelistOwner))
})
);

Expand Down Expand Up @@ -276,6 +276,9 @@ contract TestPreconfWhitelist is Layer1Test {
assertEq(inactiveSince, 0);
assertEq(index, 0);

assertEq(whitelistNoDelay.getOperatorForCurrentEpoch(), address(0));

vm.warp(whitelistNoDelay.selectorEpochOffset() * LibPreconfConstants.SECONDS_IN_EPOCH);
assertEq(whitelistNoDelay.getOperatorForCurrentEpoch(), Bob);
assertEq(whitelistNoDelay.getOperatorForNextEpoch(), Bob);

Expand Down