Skip to content

feat(protocol,taiko-client): raise inbox basefee sharing to 100% and rotate raiko2 to v0.9.0 (Proposal0026) - #22127

Merged
davidtaikocha merged 37 commits into
mainfrom
claude/proposal0024-basefee-sharing-100
Sep 30, 2026
Merged

davidtaikocha merged 37 commits into
mainfrom
claude/proposal0024-basefee-sharing-100

Conversation

@davidtaikocha

@davidtaikocha davidtaikocha commented Sep 12, 2026 •

Copy link
Copy Markdown
Member

What

Proposal0026 is one atomic DAO batch of 21 L1 actions (no L2 leg), executed by the DAO controller 0x75Ba76403b13b26AD1beC70D6eE937314eeaCD0a. It does two things:

  1. Raises the Shasta inbox's basefeeSharingPctg from 75 to 100. Every L2 block in a proposal made after execution pays its whole basefee to the coinbase (the proposing preconfer). Nothing then accrues in the L2 fee treasury (the Anchor 0x1670000000000000000000000000000000010001). The percentage is a constructor immutable, so this is an upgradeTo on the inbox proxy to a new MainnetInbox implementation. That implementation is the live one with this single field changed: same five address immutables, same numeric config, same storage, no initializer.
  2. Rotates the proving identifiers from raiko2 v0.8.0-rc1 to v0.9.0-rc1: RISC0 image IDs, SP1 program vkeys and SGX MRENCLAVEs. It also deletes the active v0.8.0-rc1 SGX instance (ID 2) from both SGX verifiers. This part came in through feat(protocol): rotate raiko2 artifacts to v0.9.0-rc1 (Proposal0026) #22176.
# Target Call
0 Inbox proxy 0x6f21C543a4aF5189eBdb0723827577e1EF57ef1f upgradeTo(0xA18431d42C8dF9778905fBEa912aCF1881b49D2e) (codediff)
1–4 RISC0 verifier 0x059dAF31F571da48Ab4e74Ae12F64f907681Cd8b setImageIdTrusted: untrust 2 v0.8.0-rc1 image IDs, trust 2 v0.9.0-rc1 image IDs
5–12 SP1 verifier 0x73A0Db393ef87ce781ac7957bE10D6628432100F setProgramTrusted: untrust 4 v0.8.0-rc1 vkeys, trust 4 v0.9.0-rc1 vkeys (BN254 + hash-bytes, proposal + aggregation)
13–18 SGX-geth attester 0x0ffa4A625ED9DB32B70F99180FD00759fc3e9261, SGX-reth attester 0x8d7C954960a36a7596d7eA4945dDf891967ca8A3 setMrEnclave: untrust 3 v0.8.0-rc1 MRENCLAVEs, trust 3 v0.9.0-rc1 MRENCLAVEs (SGX-geth, SGX-reth, SGX-reth EDMM). MRSIGNER and attribute policy unchanged
19–20 SGX-geth verifier 0x41e79EB4F03aBB5DF8716B759528dc5d8f6a84Ee, SGX-reth verifier 0x9D3C595BFf6Ff7D2b2CbdEcF94aD917eB2fCFFd8 deleteInstances([2]). v0.9.0-rc1 instances register separately after execution (expected ID 3)

The v0.9.0-rc1 values come from the release's guest-digests-summary.json and tee-attestation-manifest-v0.9.0-rc1.json. The v0.8.0-rc1 values are the ones Proposal0021 enabled. The full tables and rationale are in Proposal0026.md.

Why 100%. At 75, a preconfer that sponsors its users' L2 gas loses a quarter of every basefee. At 100 the round trip is exact, so sponsoring costs only the L1 data. The DAO gives up the 25% the Anchor currently accrues; that amount is small and has never been swept. See Rationale in Proposal0026.md.

Code changes

  • MainnetInbox.sol: basefeeSharingPctg 75 → 100. This is the source of the deployed implementation.
  • DevnetInbox.sol: the same flip, so devnet and local deployments match mainnet. Because of it, packages/taiko-client/driver/chain_syncer/event/syncer_test.go now derives the treasury-income expectation from the inbox's basefeeSharingPctg: at 100 the treasury gains nothing.
  • LibL1Addrs.ZK_REQUIRED_VERIFIER = 0x7284aaC05555Ae6559bdAd8B4221eC9584254Eec: the live proof verifier. Until now it was only a Proposal0019 constant.
  • LibRisc0Constants / LibSP1Constants: add the v0.9.0-rc1 IDs. The new LibSGXConstants holds the v0.8.0-rc1 and v0.9.0-rc1 SGX MRENCLAVEs.
  • script/layer1/mainnet/DeployInboxUpgradeL1.s.sol deploys the implementation only. It has already run and must not be re-run; it refuses once the proxy answers 100.
  • Proposal0026.{s.sol,md,action.md}, Proposal0026.t.sol, Proposal0026Fork.t.sol, Proposal0026Harness.sol, gas report.

Deployed implementation

Deployed 2026-09-12 in L1 block 25,961,745 by 0x56706f118e42ae069f20c5636141b844d1324ae1, from source commit 9deb5b590b4bf303ff161f0b8b14a49ab518a312. All three contracts are verified on Etherscan.

Contract Address Tx
MainnetInbox impl 0xA18431d42C8dF9778905fBEa912aCF1881b49D2e 0x16532ab9b11251324578fd2f1d42b0dac2986523a19771365cefd9f9af42b533
LibInboxSetup (linked) 0x526957d1a25E9D3F5ab5a4926d07eEE5d612ED42 0xbbf8ec0cee3151e09b6286e19d629d4e648de60d3b17824f939dc3ff6310ce40
LibForcedInclusion (linked) 0x511e1E5D9b9E23958076ccF1dD0033237a8cE4f8 0xf9a89d6ae2feff8a6f9f6339473fb51731eb621a6ade6052da8238b6302f3d1c

Constructor args are the live immutables: ZK_REQUIRED_VERIFIER, PRECONF_WHITELIST 0xFD01…b2ac, PROVER_WHITELIST 0xEa79…12Ae, SIGNAL_SERVICE 0x9e0a…C77C and TAIKO_TOKEN 0x10de…d800. The storage layout is unchanged from the live implementation 0x5253D4C91e80b880DdB54B78E74082Abe066F6b9.

Verification

  • Proposal0026.action.md is what P=0026 pnpm proposal generates, and test_actionFileMatchesTheBuiltCalldata pins it.
  • The fork rehearsal executes the 21-action batch from the DAO controller at L1 block 26,075,649. It asserts the inbox answers 100 with all other config and state unchanged, that every RISC0/SP1/SGX identifier rotated, and that SGX instance 2 is deleted on both verifiers. Run it with L1_FORK_URL=<archive rpc> FOUNDRY_PROFILE=layer1 forge test --match-contract Proposal0026ForkTest -vv.
  • P=0026 pnpm proposal:dryrun:l1 reverts with DryrunSucceeded().
  • The runbook reproduces the implementation's creation code byte for byte from 9deb5b5.

Rollout

  • Provers must run raiko2 v0.9.0-rc1 once the batch executes. RISC0 and SP1 can prove immediately. SGX proving is unavailable until the registrar 0x9CBeE534B5D8a6280e01a14844Ee8aF350399C7F registers the v0.9.0-rc1 SGX-geth and SGX-reth instances, which is a separate post-execution step. Until then, ZkRequiredVerifier still accepts RISC0 + SP1.
  • The whitelisted preconfer (Catalyst) must be restarted right after execution. It caches getConfig().basefeeSharingPctg at startup. Drivers and provers read the value from the Proposed event and need no change.
  • The contract logs are updated in a separate PR after execution.

🤖 Generated with Claude Code

https://claude.ai/code/session_018jqMkXxqFDncNk8KyJhY99

davidtaikocha and others added 3 commits September 12, 2026 22:19
…age to 100%

Deploys a MainnetInbox whose basefeeSharingPctg is 100 instead of 75 and
upgrades the mainnet inbox proxy to it: one upgradeTo from the DAO
controller, no L2 leg, no initializer, immutables only.

- MainnetInbox.sol: basefeeSharingPctg 75 -> 100.
- LibL1Addrs.ZK_REQUIRED_VERIFIER: the live proof verifier (Proposal0019),
  so the deploy script reproduces the live immutables from the library.
- DeployInboxUpgradeL1: reads the live proxy's getConfig() before
  broadcasting and aborts unless the new implementation differs from it
  only in the percentage.
- Proposal0024: MAINNET_INBOX_NEW_IMPL is a placeholder until the
  implementation is deployed; the print mode reverts until then and no
  action file exists.
- Tests pin the encoding, the placeholder phase, the full live
  configuration as literals, and rehearse the upgrade on a mainnet fork.
- Runbook with a TODO for @dantaik on the rationale.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ementation

MainnetInbox 0xA18431d42C8dF9778905fBEa912aCF1881b49D2e was deployed on
2026-09-12 in L1 block 25,961,745 by DeployInboxUpgradeL1, with
LibForcedInclusion and LibInboxSetup linked (three creates, all with
status 1). Its getConfig() equals the live proxy's except the sharing
percentage, Etherscan verified it, and its creation code is reproduced
byte for byte from this branch.

- Proposal0024.MAINNET_INBOX_NEW_IMPL and the test's DEPLOYED_INBOX_IMPL
  literal are filled in; the placeholder-phase branches are gone.
- Proposal0024.action.md generated with `P=0024 pnpm proposal` and pinned
  by test_actionFileMatchesTheBuiltCalldata; the L1 dry run reverts
  DryrunSucceeded(); the fork rehearsal executes the committed calldata
  against the deployed implementation and deploys nothing.
- Runbook: deployment facts, addresses, codediff link, and a
  creation-code comparison in place of forge verify-bytecode, which
  refuses library-linked contracts.
- Deploy script doc: three creates, not two.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
davidtaikocha and others added 3 commits September 12, 2026 23:12
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…runbook

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@davidtaikocha
davidtaikocha marked this pull request as ready for review September 12, 2026 14:17
davidtaikocha and others added 2 commits September 12, 2026 23:32
… refresh fix

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ences

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@davidtaikocha davidtaikocha changed the title feat(protocol): Proposal0024 raises the inbox basefee sharing percentage to 100% feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0024) Sep 12, 2026
… stack limit

genesis-docker compiles test/layer1 under the via-IR layer1o profile and
failed with "Variable size_1 is 1 too deep in the stack". The action loop
in Proposal0024Fork.t.sol::_executeAs is identical to Proposal0023's, but
it has a single caller, so the IR inliner folds it into the test, whose
live variables push the loop's call temporaries and the concatenated
assertion message one slot over the limit. The failure is now a custom
error carrying the action index, which needs no string.concat or
vm.toString temporaries; the layer1o build passes and the rehearsal
still passes against live mainnet state.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread packages/protocol/contracts/layer1/mainnet/LibL1Addrs.sol
Comment thread packages/protocol/script/layer1/mainnet/DeployInboxUpgradeL1.s.sol
Comment thread packages/protocol/contracts/layer1/mainnet/MainnetInbox.sol Outdated
Replace the rationale TODO with the case for moving the inbox basefee
share from 75% to 100%, and make the runbook precise about where the
non-coinbase share actually goes.

- Name the two recipients exactly: the coinbase, which both drivers
  overwrite with the proposal's `proposer` at derivation, so it is always
  the whitelisted preconfer that proposed the block; and the Anchor
  contract, which holds its share as a plain ETH balance until the DAO
  sweeps it with `Anchor.withdraw` through the L2 delegate controller.
  The runbook previously called the Anchor "the L2 treasury" without
  saying that nothing forwards the balance anywhere.
- State the argument as the asymmetry it is: at current L2 volume the 25%
  is immaterial to the DAO, while for a proposer it is a per-transaction
  loss on every sponsored transaction that makes fee sponsorship a
  business nobody would enter.
- Record the two limits of the refund: it is exact only within a
  preconfer's own proposals, and it covers the L2 fee only.
- Add a trade-offs section covering the forgone revenue and its
  reversibility, and the fact that a proposer's own L2 gas round-trips
  fully at 100 so L1 data cost becomes its only floor.
- Note that the already-accrued Anchor balance is untouched and stays
  withdrawable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UhqmeYDadT95CmcUhWzc2B
…100%

Match `MainnetInbox` so local and devnet deployments behave like mainnet
after Proposal0024 executes. `DevnetInbox` is not in `MainnetInbox`'s
dependency tree and no deployed contract reads it; its only consumer is
`DeployProtocolOnL1`.

The taiko-client integration tests deploy through that script, and two of
them asserted the treasury balance strictly grows, which only holds while
the percentage is below 100. Derive the expectation from the inbox's own
`basefeeSharingPctg` instead, so both tests are correct at any setting:
at 100 the treasury gains nothing and the per-transaction reconciliation
in `TestTreasuryIncome` still pins the split exactly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UhqmeYDadT95CmcUhWzc2B
`LibL1Addrs.ZK_REQUIRED_VERIFIER` became `ZKEVM_VERIFIER`, but two call
sites still referenced the old name and no longer compiled:
`DeployInboxUpgradeL1._checkLiveProxy` and the Proposal0024 test that
builds `MainnetInbox` with the live address immutables. The address is
unchanged. Also update the name in the Proposal0024 config table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UhqmeYDadT95CmcUhWzc2B
Restore `LibL1Addrs.ZK_REQUIRED_VERIFIER` and its three call sites in
`DeployInboxUpgradeL1`, the Proposal0024 test and the Proposal0024 config
table. The address `0x7284aaC05555Ae6559bdAd8B4221eC9584254Eec` is
unchanged, and the name now matches the deployed contract
(`ZkRequiredVerifier`), Proposal0019's own constant and the L1 deployment
log, so nothing in the tree still says `ZKEVM_VERIFIER`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UhqmeYDadT95CmcUhWzc2B
…UpgradeL1

`_checkLiveProxy` only rejected a live percentage already equal to the new
value, so any other non-100 reading passed the guard — and `_checkConfig`
cannot catch it either, because it normalises `basefeeSharingPctg` away
before comparing the new implementation with the live one. The NatSpec and
the Proposal0024 runbook both claimed the script aborts unless the live
value is still 75, which was not what the code did.

Add `OLD_BASEFEE_SHARING_PCTG = 75` and require the live proxy to equal it,
after the existing `AlreadyUpgraded` check so a re-run following execution
still reports itself rather than as a generic mismatch.

Reported by the deepseek-review bot on the pull request.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UhqmeYDadT95CmcUhWzc2B

dantaik commented Sep 16, 2026

Copy link
Copy Markdown
Member

Went through the deepseek-review findings. Took the warning, declined both suggestions.

Warning — _checkLiveProxy does not pin 75. Correct, fixed in e319834.

The guard only rejected a live percentage already equal to NEW_BASEFEE_SHARING_PCTG, and _checkConfig cannot catch the gap either: it assigns _new.basefeeSharingPctg = _live.basefeeSharingPctg before the keccak comparison, so any live value other than 100 passed both checks. The NatSpec and the runbook both claimed the script aborts unless the live value is still 75, which is not what the code did. Added OLD_BASEFEE_SHARING_PCTG = 75 and a require against it, ordered after the existing AlreadyUpgraded check so a re-run after execution still reports itself rather than as a generic LiveProxyMismatch.

Suggestion — named constants for 75 in the fork tests. Declining.

Proposal0024Fork.t.sol asserts the percentage the live proxy reports (assertEq(before.config.basefeeSharingPctg, 75, ...)). That is pinning observed on-chain state, which is where a literal is the right thing: importing the deploy script's constant would make the assertion self-referential and it would keep passing if both drifted together. The script itself now has the named constant, since the guard fix needs it.

Suggestion — pin the fork block in _forkOrSkip. Declining.

This is the repo's existing convention rather than an oversight. Proposal0023Fork.t.sol does the same thing — _forkOrSkip("L1_FORK_URL") with the pre-upgrade assertion carrying the block number in its failure message (pin --fork-block-number 25888605 against an archive node); Proposal0024 follows it with 25961507. A fork taken after execution fails on the first assertion with the exact block to pin, so reproducibility is already handled, and changing it here would make this one rehearsal diverge from every other.


Generated by Claude Code

dantaik commented Sep 16, 2026

Copy link
Copy Markdown
Member

Second deepseek-review pass. Checked all four against the code; none needs a change, and one is factually wrong about what the script does.

Warning 1 — "broadcasts before validating the full config", leaving an orphaned implementation. Not how forge script works.

forge script executes the whole function in simulation first and only submits the collected broadcast transactions afterwards, so a revert in _checkConfig — which runs after vm.stopBroadcast() but still inside the same script body — aborts the run during simulation and nothing is ever sent. There is no path where an implementation lands on-chain and the config check then fails. The ordering is also forced: _checkConfig compares the new implementation's getConfig(), which cannot be read before the contract exists. The fork simulation the finding suggests as the remedy is what forge script already does by default.

Warning 2 — library deployment is implicit. Correct, and deliberately left alone.

Forge's implicit linked-library deployment is exactly what ran on 2026-09-12, and the runbook documents the resulting three creates with cast codesize checks for all three addresses. Changing the script to deploy the libraries explicitly would make it no longer describe the deployment that actually happened, which is the one thing this script is now good for — the implementation is already deployed and the script must not be re-run. The verification that matters is stronger than the script anyway: the creation code is reproduced byte for byte against the deployment transaction's input, with the deployed library addresses patched into the link references.

Suggestion 1 — split the address check from AlreadyUpgraded. They are already separate requires as of e319834; the description does not match the current code. On the substance, the ordering is intentional: a drifted address is a more serious condition than "already upgraded", so it should be what gets reported. After a normal execution the addresses are unchanged and the re-run case reports AlreadyUpgraded as intended; the combination the finding describes can only mean something went badly wrong, and LiveProxyMismatch is the right alarm for that.

Suggestion 2 — exposedBuildAllActions reimplements _buildAllActions. BuildProposal._buildAllActions is private, so a harness cannot call it; Proposal0023Harness reproduces it the same way. Making it internal would change shared governance code every proposal inherits for the benefit of one test harness. The drift the finding worries about is already covered and documented in the harness NatSpec: Proposal0024.action.md is generated by the real _buildAllActions via P=0024 pnpm proposal, and test_actionFileMatchesTheBuiltCalldata compares it against the harness's reproduction — so the two diverging fails that test rather than going unnoticed.


Generated by Claude Code

Comment thread packages/protocol/script/layer1/proposals/Proposal0026.s.sol Outdated
Comment thread packages/taiko-client/driver/chain_syncer/event/syncer_test.go
Comment thread packages/protocol/script/layer1/proposals/Proposal0026.md Outdated
@dantaik
dantaik self-requested a review September 29, 2026 04:09
…onstants

Add contracts/layer1/verifiers/LibSGXConstants.sol next to LibRisc0Constants
and LibSP1Constants. It holds the raiko2 v0.8.0-rc1 and v0.9.0-rc1 SGX-geth and
SGX-reth (non-EDMM and EDMM) MRENCLAVE values under version-prefixed names, and
Proposal0026 now reads them from there instead of declaring its own OLD_/NEW_
constants. The values are unchanged: Proposal0026.action.md regenerates
byte-identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

dantaik commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Review of Proposal0026 at 1d155df, re-checked at d4bec46: calldata, deployed code and values check out. Three items are open.

Verified:

  • Proposal0026.action.md is byte-identical to the P=0026 pnpm proposal output, and the 21 actions decode as documented. Unit tests, the fork rehearsal at block 26,075,649 and the dry run at the current head all pass. d4bec46 (moving the constants into LibSGXConstants) leaves the calldata unchanged.
  • Inbox upgrade 0x5253… → 0xA184…:
  • All 24 addresses are verified on Etherscan, and the controller owns every target.
  • All 18 RISC0/SP1/SGX values match the v0.8.0-rc1 and v0.9.0-rc1 release assets and recompute from the ELFs and enclave SIGSTRUCTs. The on-chain pre-state is as expected.

Open:

  1. @smtmfft: besides the inline comment on verifiability, raiko2 fe646ca rebuilt the guest ELFs after rc1. It bumps to protocol v2.3.0, which includes the chore(taiko-client,taiko-client-rs): align manifest decoding #22173 manifest-decoding change, so rc1 predates it. Is rc1 what we want to trust, or should we wait for an rc2?
  2. The SGX targets of actions 13–20 don't match main. The attesters run AutomataDcapV3Attestation, which was deleted in feat(protocol): use audited upstream automata code with DEBUG enclaves rejection #21827, and the verifiers are the pre-redesign SecureSgxVerifier from feat(protocol): disable forced inclusion submission && add hack recovery deploy script #21847. The calls work against the deployed code, but the runbook should say so.
  3. From execution until registrar 0x9CBe…9C7F registers the v0.9.0-rc1 SGX instances, only RISC0+SP1 proofs are accepted. Have that tx ready. Register no new v0.8 instance before execution, since only ID 2 is deleted.

Nits:

  • Proposal0026.md:234 says 18 layout entries; there are 17. Fixed in 1e6a874.
  • 9deb5b5 is not a main commit, which conflicts with note 1 in mainnet-contract-logs-L1.md.

Generated by Claude Code

@dantaik dantaik changed the title feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0026) feat(protocol): raise inbox basefee sharing to 100% and rotate raiko2 to v0.9.0-rc1 (Proposal0026) Sep 29, 2026
claude and others added 2 commits September 29, 2026 05:34
…al0026.md

MainnetInbox_Layout.sol lists 17 storage entries, not 18. The file is
byte-identical at 9078278, the PR head and main, so only the count in
the runbook was wrong.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018jqMkXxqFDncNk8KyJhY99
@dantaik dantaik changed the title feat(protocol): raise inbox basefee sharing to 100% and rotate raiko2 to v0.9.0-rc1 (Proposal0026) feat(protocol,taiko-client): raise inbox basefee sharing to 100% and rotate raiko2 to v0.9.0-rc1 (Proposal0026) Sep 29, 2026
@davidtaikocha davidtaikocha changed the title feat(protocol,taiko-client): raise inbox basefee sharing to 100% and rotate raiko2 to v0.9.0-rc1 (Proposal0026) feat(protocol,taiko-client): raise inbox basefee sharing to 100% and rotate raiko2 to v0.9.0 (Proposal0026) Sep 30, 2026
#22178 moved Proposal0026 to the final raiko2 v0.9.0 identifiers but left
the nine v0.9.0-rc1 constants in LibRisc0Constants, LibSP1Constants and
LibSGXConstants. Nothing references them, and none was ever trusted on
mainnet, so the libraries keep only the versions the proposal uses. The
calldata in Proposal0026.action.md is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dantaik
dantaik self-requested a review September 30, 2026 06:02
@github-actions

Copy link
Copy Markdown
Contributor

🐋 DeepSeek Code Review

🟡 Warnings

  • DeployInboxUpgradeL1 does not check the live proxy’s implementation slot against the expected live implementation (0x5253...).
    It validates getConfig() and the five address immutables only. If the proxy were upgraded by another proposal before this script is re-run (but while still showing basefeeSharingPctg == 75), the script would deploy a new implementation from the current source without verifying the storage/behavior baseline it claims to preserve. The fork test does check this, but the deployment script itself should also pin the implementation slot.

  • Proposal0026Fork.t.sol does not assert that nextInstanceId() remains 3 after deleting SGX instance 2.
    The rollout document and post-execution steps depend on the next registered instance receiving ID 3. _assertRotationState only checks whether instance 2 is active/zero, not that the registry’s nextInstanceId was left unchanged. Add explicit assertions so a verifier change that reused/decremented instance IDs would fail the rehearsal.

  • Only one Go test was updated for the DevnetInbox 75→100 change.
    syncer_test.go is adjusted, but other local/devnet tests or integration paths may still assume a 75/25 treasury split or a positive Anchor balance. This is not necessarily a bug, but the full taiko-client test suite should be run to confirm there are no other stale expectations.

🔵 Suggestions

  • Add release-source/provenance NatSpec to the new identifier constants in LibRisc0Constants, LibSP1Constants, and LibSGXConstants. The values are opaque bytes32s; a short comment linking each to the v0.9.0 release artifact would reduce future review risk.

  • Guard _snapshot() against very early fork blocks.
    _snapshot() computes nextProposalId - 1 unconditionally. If L1_FORK_BLOCK is set to a pre-upgrade block with nextProposalId == 0, it will revert with an underflow/out-of-bounds rather than a clear precondition failure. Since the doc says other fork blocks must have the same preconditions, an explicit check with a useful message would be better.

  • Consider pinning the full execute calldata selector in test_actionFileMatchesTheBuiltCalldata.
    The test compares only the ABI-encoded argument array (abi.encode(actions)), while Proposal0026.action.md also declares Function: Execute. If the tooling ever prepended the wrong selector, this test would not catch it. Pinning the complete calldata would close that gap.

🟢 What Looks Good

  • Strong fork rehearsal: executes the exact 21 actions against a live mainnet fork and verifies the inbox config/storage remains unchanged except for basefeeSharingPctg.
  • Good independent test literals for verifier IDs, MRENCLAVEs, vkeys, and deployed implementation address, reducing mirroring mistakes.
  • The deployment script compares the new implementation’s getConfig() against the live proxy and normalizes out only basefeeSharingPctg.
  • No initializer on upgradeTo; storage layout is explicitly tested in the fork rehearsal.
  • Action file is pinned against the generated proposal calldata, so a stale committed action file cannot silently diverge from the proposal code.

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

@davidtaikocha
davidtaikocha added this pull request to the merge queue Sep 30, 2026
Merged via the queue into main with commit 6d1d627 Sep 30, 2026
15 checks passed
@davidtaikocha
davidtaikocha deleted the claude/proposal0024-basefee-sharing-100 branch September 30, 2026 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants