From ff8690e52f1d4e5e506b156f7e4042cf13d8d858 Mon Sep 17 00:00:00 2001 From: D <51912515+adaki2004@users.noreply.github.com> Date: Fri, 19 Jan 2024 14:40:27 +0530 Subject: [PATCH] fix(protocol): fix guardian prover bug (#15528) Co-authored-by: Daniel Wang --- .../protocol/contracts/L1/libs/LibProving.sol | 27 +++++--------- .../test/L1/TaikoL1LibProvingWithTiers.t.sol | 37 ++++++++++++------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/packages/protocol/contracts/L1/libs/LibProving.sol b/packages/protocol/contracts/L1/libs/LibProving.sol index 7c8dd7c4f3b..363543eb29a 100644 --- a/packages/protocol/contracts/L1/libs/LibProving.sol +++ b/packages/protocol/contracts/L1/libs/LibProving.sol @@ -224,12 +224,9 @@ library LibProving { if (tier.contestBond == 0) { assert(tier.validityBond == 0); - // When contestBond is zero for the current tier, it signifies - // it's the top tier. In this case, it can overwrite existing - // transitions without contestation. - if (tran.blockHash == ts.blockHash && tran.signalRoot == ts.signalRoot) { - revert L1_ALREADY_PROVED(); - } + + // It means prover is right (not the contester) + bool sameTransition = tran.blockHash == ts.blockHash && tran.signalRoot == ts.signalRoot; // We should outright prohibit the use of zero values for both // blockHash and signalRoot since, when we initialize a new // transition, we set both blockHash and signalRoot to 0. @@ -252,8 +249,10 @@ library LibProving { ts.prover = msg.sender; if (ts.contester != address(0)) { - // At this point we know that the contester was right - tko.transfer(ts.contester, ts.validityBond >> 2 + ts.contestBond); + if (!sameTransition) { + // At this point we know that the contester was right + tko.transfer(ts.contester, ts.validityBond >> 2 + ts.contestBond); + } ts.contester = address(0); ts.validityBond = 0; } @@ -345,20 +344,12 @@ library LibProving { revert L1_ALREADY_PROVED(); } - if (tid == 1 && ts.prover == blk.assignedProver) { + if (tid == 1 && ts.tier == 0 && block.timestamp <= ts.timestamp + tier.provingWindow) { // For the first transition, (1) if the previous prover is // still the assigned prover, we exclusively grant permission to // the assigned approver to re-prove the block, (2) unless the // proof window has elapsed. - if ( - block.timestamp <= ts.timestamp + tier.provingWindow - && msg.sender != blk.assignedProver - ) revert L1_NOT_ASSIGNED_PROVER(); - - if ( - block.timestamp > ts.timestamp + tier.provingWindow - && msg.sender == blk.assignedProver - ) revert L1_ASSIGNED_PROVER_NOT_ALLOWED(); + if (msg.sender != blk.assignedProver) revert L1_NOT_ASSIGNED_PROVER(); } else if (msg.sender == blk.assignedProver) { // However, if the previous prover of the first transition is // not the block's assigned prover, or for any other diff --git a/packages/protocol/test/L1/TaikoL1LibProvingWithTiers.t.sol b/packages/protocol/test/L1/TaikoL1LibProvingWithTiers.t.sol index cd29e0d5e34..1bdd01613bf 100644 --- a/packages/protocol/test/L1/TaikoL1LibProvingWithTiers.t.sol +++ b/packages/protocol/test/L1/TaikoL1LibProvingWithTiers.t.sol @@ -405,7 +405,7 @@ contract TaikoL1LibProvingWithTiers is TaikoL1TestBase { printVariables(""); } - function test_L1_GuardianProverCannotOverwriteIfSameProof() external { + function test_L1_GuardianProverCanAlwaysOverwriteTheProof() external { giveEthAndTko(Alice, 1e7 ether, 1000 ether); giveEthAndTko(Carol, 1e7 ether, 1000 ether); console2.log("Alice balance:", tko.balanceOf(Alice)); @@ -429,20 +429,13 @@ contract TaikoL1LibProvingWithTiers is TaikoL1TestBase { bytes32 signalRoot = bytes32(1e9 + blockId); // This proof cannot be verified obviously because of // blockhash:blockId - proveBlock(Bob, Bob, meta, parentHash, blockHash, signalRoot, meta.minTier, ""); - - // Try to contest - but should revert with L1_ALREADY_PROVED - proveBlock( - Carol, - Carol, - meta, - parentHash, - blockHash, - signalRoot, - LibTiers.TIER_GUARDIAN, - TaikoErrors.L1_ALREADY_PROVED.selector - ); + (, TaikoData.SlotB memory b) = L1.getStateVariables(); + uint64 lastVerifiedBlockBefore = b.lastVerifiedBlockId; + proveBlock(Bob, Bob, meta, parentHash, blockHash, signalRoot, meta.minTier, ""); + console2.log("mintTier is:", meta.minTier); + // Try to contest + proveBlock(Carol, Carol, meta, parentHash, 0, 0, meta.minTier, ""); vm.roll(block.number + 15 * 12); uint16 minTier = meta.minTier; @@ -450,6 +443,22 @@ contract TaikoL1LibProvingWithTiers is TaikoL1TestBase { verifyBlock(Carol, 1); + (, b) = L1.getStateVariables(); + uint64 lastVerifiedBlockAfter = b.lastVerifiedBlockId; + + console.log(lastVerifiedBlockAfter, lastVerifiedBlockBefore); + // So it is contested - because last verified not changd + assertEq(lastVerifiedBlockAfter, lastVerifiedBlockBefore); + + // Guardian can prove with the original (good) hashes. + proveBlock( + Carol, Carol, meta, parentHash, blockHash, signalRoot, LibTiers.TIER_GUARDIAN, "" + ); + + vm.roll(block.number + 15 * 12); + vm.warp(block.timestamp + L1.getTier(LibTiers.TIER_GUARDIAN).cooldownWindow + 1); + + verifyBlock(Carol, 1); parentHash = blockHash; } printVariables("");