Skip to content

refactor(protocol): renumber Proposal0023/0024 to match the DAO proposal IDs - #22154

Merged
dantaik merged 1 commit into
claude/proposal0024-basefee-sharing-100from
claude/great-einstein-0dcbuo
Sep 21, 2026
Merged

refactor(protocol): renumber Proposal0023/0024 to match the DAO proposal IDs#22154
dantaik merged 1 commit into
claude/proposal0024-basefee-sharing-100from
claude/great-einstein-0dcbuo

Conversation

@dantaik

@dantaik dantaik commented Sep 21, 2026

Copy link
Copy Markdown
Member

Summary

The repository's proposal numbering had drifted one behind the IDs the proposals actually carry on dao.taiko.xyz. This shifts the two affected proposals up by one so file names, contract names and runbooks line up with the DAO:

Was Now Proposal
Proposal0024 Proposal0025 Raise the inbox basefee sharing percentage to 100%
Proposal0023 Proposal0024 Upgrade the L1 and L2 bridges and ERC20 vaults

The renames are applied in that order (24 → 25 first) so the two never collide.

Stacking

Proposal0024 exists only on #22127, not on main, so this PR is based on claude/proposal0024-basefee-sharing-100 rather than main and carries both renames together. The diff against that base is the single rename commit. It needs #22127 to merge first; if #22127 is instead rebased or retargeted, this branch should follow it.

What changed

Renamed per proposal — the .s.sol builder, the .md runbook, the .action.md payload, and the .t.sol, Fork.t.sol and Harness.sol tests, along with their contract names.

References updated in place:

  • the mainnet deploy scripts for both legs (DeployBridgeUpgradeL1/L2, DeployBridgedERC20V2L1/L2, DeployERC20VaultUpgradeL1/L2, DeployInboxUpgradeL1)
  • the historical comments in LibL1Addrs.sol and LibL2Addrs.sol
  • the basefee sharing comment in MainnetInbox.sol
  • the test entries in gas-reports/layer1-contracts.txt (which remains sorted)

This is a name-only change. No calldata, address or gas value is touched — the committed .action.md payloads are byte-identical, since the DAO calldata does not encode the proposal name.

Verification

Foundry and the Solidity dependencies could not be installed in this environment (the egress policy blocks github.com / codeload.github.com), so the contracts were not compiled and the tests were not run here — CI is the first real check. What was verified statically:

  • Each of the 12 renamed files is byte-for-byte its original with only the Proposal00NN, PROPOSAL-00NN and P=00NN tokens substituted (diffed against the base branch programmatically).
  • Every changed line outside the renamed files is a name-only comment or gas-report edit.
  • No Proposal0023 / PROPOSAL-0023 / P=0023 token remains anywhere in the monorepo.
  • Bare 0023 / 0024 also occur inside hex addresses, so substitution was anchored on the prefixed forms only; no address bytes changed.
  • Every import path in the renamed files resolves; every contract name matches its filename; both vm.readFile("…action.md") targets exist; every P=00NN comment agrees with its own file.
  • The gas report is still fully sorted after renumbering.

Note

This leaves a gap at 0022/0023 in script/layer1/proposals/, which is inherent to the shift (0022 was already unused on main — it is claimed by draft #21964). Worth a sanity check that the draft's intended number is still right.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QPnP4qc8wE7YeBXWvZJzgW

Generated by Claude Code

@claude
refactor(protocol): renumber Proposal0023/0024 to match the DAO propo…
abc40e5 
…sal IDs

The repository's proposal numbering had drifted one behind the IDs the
proposals actually carry on dao.taiko.xyz. Shift the two affected proposals up
by one so the file names, contract names and runbooks line up with the DAO:

- Proposal0024 (raise the inbox basefee sharing percentage to 100%)
  becomes Proposal0025.
- Proposal0023 (upgrade the L1 and L2 bridges and ERC20 vaults)
  becomes Proposal0024.

The renames are applied in that order so the two never collide. This is a
name-only change: every renamed file is byte-for-byte its original with only
the `Proposal00NN`, `PROPOSAL-00NN` and `P=00NN` tokens substituted. No
calldata, address or gas value is touched -- the committed `.action.md`
payloads are unchanged, since the DAO calldata does not encode the proposal
name.

Renamed, per proposal: the `.s.sol` builder, the `.md` runbook, the
`.action.md` payload, and the `.t.sol`, `Fork.t.sol` and `Harness.sol` tests,
along with their contract names.

References updated in place: the mainnet deploy scripts for both legs, the
historical comments in LibL1Addrs.sol and LibL2Addrs.sol, the basefee sharing
comment in MainnetInbox.sol, and the test entries in the layer1 gas report
(which remains sorted).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QPnP4qc8wE7YeBXWvZJzgW
smtmfft
smtmfft approved these changes Sep 21, 2026
View reviewed changes

@smtmfft smtmfft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed exact head abc40e5e5ac518a364d272ef5313c8c69457ba58 against the declared stacked base 8e6942d13fbd4bdcb56715a5f62bfb1ae8e46efa (the current head of #22127). No blocking correctness, regression, or compatibility findings.

Verified that all 12 moved proposal artifacts are byte-identical to their base versions after only the anchored Proposal00NN, PROPOSAL-00NN, and P=00NN substitutions. Both .action.md files retain identical DAO controller, value, and calldata bytes. Proposal imports, vm.readFile targets, filenames, and contract names agree; the gas report remains sorted; git diff --check and forge fmt --check pass; and a focused forge build of both renamed unit-test files succeeds.

This approval covers only the renumbering delta. The underlying basefee-sharing semantic change belongs to #22127, not this PR. Because this is a draft stacked PR, its protocol/test CI jobs are currently skipped; #22127 must merge first, and this PR should be rebased or retargeted and rechecked if that base head changes.

@dantaik
dantaik marked this pull request as ready for review September 21, 2026 08:26
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-21T08:28:37.697362Z abc40e5 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

🐋 DeepSeek Code Review

🟡 Warnings

  1. CI has not been run; compilation and tests unverified locally.
    The PR description explicitly states that Foundry and dependencies could not be installed, so contracts were not compiled and tests were not run. This is a large rename touching 12+ Solidity files, contracts, tests, harnesses, and deploy scripts. Even though static checks were done, missed references (e.g., in package.json scripts, CI workflows, or other files not shown in the diff) would only surface in CI. This PR should not be merged until CI passes.

  2. PR depends on feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0026) #22127 (claude/proposal0024-basefee-sharing-100).
    The PR is not based on main; it carries both renames together with the basefee-sharing proposal. If feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0026) #22127 is rebased or retargeted, this branch must follow. Merge order must be coordinated to avoid broken references or duplicate proposal numbers.

  3. Potential missed references outside the diff.
    The diff only shows files changed in this PR. While the description claims no Proposal0023 / PROPOSAL-0023 / P=0023 tokens remain, this could not be verified without a full repository scan. References in scripts, Makefiles, GitHub Actions, or documentation may still point to the old numbers. A grep across the monorepo should be run before merge.

  4. Gas report entries for fork tests show gas: 0 because they are skipped when RPC URLs are not set.
    This is expected and not a bug, but reviewers should ensure that fork tests are run in CI with proper RPC env vars to validate the re-numbered tests actually execute.

🔵 Suggestions

  • Run pnpm proposal for both P=0024 and P=0025 after the rename and check git diff --exit-code on the .action.md files to confirm calldata is unchanged and file paths are correct.
  • Verify that all deploy scripts (DeployBridgeUpgradeL1, DeployInboxUpgradeL1, etc.) now log/reference the correct proposal numbers and that no comments remain stale.
  • Consider adding a CI check that fails on any remaining Proposal0023 tokens in the repository to prevent future drift.

🟢 What Looks Good

  • The rename order (24 → 25 first) avoids file collisions.
  • Comments in LibL1Addrs, LibL2Addrs, MainnetInbox, and all deploy scripts have been updated consistently with the new numbering.
  • Gas report is correctly sorted and updated.
  • The new .action.md files are byte-identical to their pre-rename counterparts, preserving DAO calldata.
  • Contract names, filenames, and import paths are internally consistent within each proposal set.
  • The PR description is thorough and transparent about verification limitations.

Automatically triggered on PR update • model: deepseek-v4-pro

@dantaik Claude

dantaik commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

Checked each DeepSeek point. No code change is needed; details so reviewers don't have to re-derive them.

W1 — CI not run. Accurate when written, now resolved: the PR was a draft, and protocol.yml gates build-protocol and genesis-docker on github.event.pull_request.draft == false, so those jobs reported skipped rather than green. It is now marked ready and the full suite is running on abc40e5. Agreed it should not merge before that is green.

W3 — possible missed references outside the diff. This one was already verified, and I re-ran it exhaustively just now. Zero occurrences of Proposal0023 / PROPOSAL-0023 / P=0023 anywhere in the monorepo — case-insensitive, all file types including binary, tracked and untracked, node_modules and .git excluded — via both grep -rni and git grep -i. Separately, the only non-.sol/.md file in the repo that mentions any Proposal00NN token at all is packages/protocol/gas-reports/layer1-contracts.txt, which this PR updates. No workflow, JSON, shell script or Makefile references proposal numbers, so there is nothing of that kind left to miss.

S1 — regenerate the .action.md files and diff them. Provably a no-op here rather than an unchecked assumption. BuildProposal.logProposalAction writes "# Proposal" + proposalId followed by the DAO controller, function, value and abi.encode(_buildAllActions()). The heading is therefore exactly # Proposal0024 / # Proposal0025 for P=0024 / P=0025, which is what the renamed files contain, and the calldata derives only from _buildAllActions(), which a rename does not touch. Both committed payloads are byte-identical to their pre-rename counterparts. test_actionFileMatchesTheBuiltCalldata enforces this in CI for both proposals, so CI is already running the check the suggestion asks for.

W2 — dependency on #22127: intended and documented under Stacking; this branch is pinned to base head 8e6942d13 and needs a recheck if that moves. W4 — fork tests at gas: 0: pre-existing behaviour for RPC-gated fork tests, not introduced here. S2 — deploy script references: verified; every changed line outside the renamed files is a name-only comment or gas-report edit.

S3 — add a CI check that fails on leftover Proposal0023 tokens. Declining in this PR. It would widen a pure rename into tooling changes, and a token-specific guard goes stale at the next renumbering. Reasonable as a follow-up if you want it.

Generated by Claude Code

@dantaik
dantaik merged commit 2447a95 into claude/proposal0024-basefee-sharing-100 Sep 21, 2026
25 of 27 checks passed
@dantaik
dantaik deleted the claude/great-einstein-0dcbuo branch September 21, 2026 08:39
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.18%. Comparing base (8e6942d) to head (abc40e5).
⚠️ Report is 4 commits behind head on claude/proposal0024-basefee-sharing-100.

Additional details and impacted files

see 1 file with indirect coverage changes

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e6942d...abc40e5. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dantaik added a commit that referenced this pull request Sep 21, 2026
@dantaik
Revert "refactor(protocol): renumber Proposal0023/0024 to match the D…
9e32a29 
…AO proposal IDs (#22154)"

This reverts commit 2447a95.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smtmfft smtmfft smtmfft approved these changes
@davidtaikocha davidtaikocha Awaiting requested review from davidtaikocha davidtaikocha is a code owner

Assignees

No one assigned

Labels

area.protocol

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dantaik @smtmfft @davidtaikocha @claude