Skip to content

Re-order check #153

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

Merged
nikeshnazareth merged 1 commit into from
Aug 14, 2025
Merged

Re-order check #153

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions src/protocol/taiko_alethia/TaikoInbox.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ contract TaikoInbox is IInbox, DelayedInclusionStore, Ownable {
require(lookahead.isCurrentPreconfer(msg.sender), NotCurrentPreconfer());
}

// Build the attribute for the anchor transaction inputs
require(anchorBlockId >= block.number - maxAnchorBlockIdOffset, AnchorBlockTooOld());
require(anchorBlockId + maxAnchorBlockIdOffset >= block.number, AnchorBlockTooOld());
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 8, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Add boundary tests to lock in semantics and prevent regressions

Please add tests covering:

  • Line 69 boundary: anchorBlockId == block.number - maxAnchorBlockIdOffset (passes) and anchorBlockId == block.number - maxAnchorBlockIdOffset - 1 (reverts AnchorBlockTooOld()).
  • Early-chain scenario: maxAnchorBlockIdOffset > block.number (should not revert due to underflow; still fails later with BlockhashUnavailable() if out of 256-block window).
  • Future/current block IDs: anchorBlockId >= block.number (should revert BlockhashUnavailable()).
  • Very large maxAnchorBlockIdOffset (if allowed by config), to ensure no unexpected addition overflow behavior.

You can quickly check for existing tests and anchor usages with:

I can draft Foundry/Hardhat tests for these cases if helpful.

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Search for AnchorBlockTooOld usage/tests:"
rg -n --hidden --glob '!*node_modules*' 'AnchorBlockTooOld' -A 3 -B 3 || true

echo "Search for TaikoInbox.publish tests or calls:"
rg -n --hidden --glob '!*node_modules*' 'publish\(' -A 3 -B 3 || true

echo "Search for anchorBlockId boundary conditions in tests:"
rg -n --hidden --glob '!*node_modules*' 'anchorBlockId' -A 5 -B 5 || true

Length of output: 14681

Add comprehensive boundary tests for publish in TaikoInbox

We need to lock in the semantics of the require(anchorBlockId + maxAnchorBlockIdOffset >= block.number, AnchorBlockTooOld()) check and the subsequent BlockhashUnavailable() guard. No existing unit tests cover these edge cases, so please add tests (e.g. in test/InboxBoundary.t.sol) for:

  • Exact lower boundary:
    • anchorBlockId == block.number − maxAnchorBlockIdOffset → should succeed
    • anchorBlockId == block.number − maxAnchorBlockIdOffset − 1 → should revert AnchorBlockTooOld()

  • Underflow “early‐chain” scenario (when maxAnchorBlockIdOffset > block.number):
    • Ensure the underflowed sum still passes the first require, but then reverts BlockhashUnavailable() if the block is out of the 256‐block hash window.

  • Future and current block IDs:
    • anchorBlockId ≥ block.number → should revert BlockhashUnavailable() (because blockhash(anchorBlockId) returns 0)

  • Large maxAnchorBlockIdOffset values:
    • If your config allows very large offsets, verify no overflow in anchorBlockId + maxAnchorBlockIdOffset and correct revert paths.

Pinpoint locations:

  • File: src/protocol/taiko_alethia/TaikoInbox.sol, line 69 (the AnchorBlockTooOld() check)
  • Immediately after: the blockhash lookup and its BlockhashUnavailable() require

Feel free to use Foundry’s vm.roll(...) to manipulate block.number and vm.expectRevert(...) for these scenarios.

🤖 Prompt for AI Agents 
In src/protocol/taiko_alethia/TaikoInbox.sol at line 69 and immediately after,
add comprehensive unit tests in test/InboxBoundary.t.sol to cover boundary
conditions of the require check and blockhash validation in the publish
function. Specifically, write tests for the exact lower boundary cases where
anchorBlockId equals block.number minus maxAnchorBlockIdOffset (should succeed)
and one less than that (should revert AnchorBlockTooOld). Also, test the
underflow scenario when maxAnchorBlockIdOffset is greater than block.number to
ensure the require passes but blockhash lookup reverts with
BlockhashUnavailable. Include tests for anchorBlockId greater or equal to
block.number to confirm it reverts BlockhashUnavailable due to blockhash
returning zero. Finally, if large maxAnchorBlockIdOffset values are allowed,
verify no overflow occurs in the sum and that revert paths behave correctly. Use
Foundry’s vm.roll to manipulate block.number and vm.expectRevert to assert the
expected reverts.


Metadata memory metadata = Metadata({
anchorBlockId: anchorBlockId,
Expand Down