Skip to content

feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0026) - #22127

Draft
davidtaikocha wants to merge 26 commits into
mainfrom
claude/proposal0024-basefee-sharing-100
Draft

davidtaikocha wants to merge 26 commits into
mainfrom
claude/proposal0024-basefee-sharing-100

Conversation

@davidtaikocha

@davidtaikocha davidtaikocha commented Sep 12, 2026

Copy link
Copy Markdown
Member

Stacked on #22155, which renumbers the existing bridge / ERC20 vault proposal from Proposal0023 to Proposal0024.

What

Proposal0026 raises the mainnet Shasta inbox's basefeeSharingPctg from 75 to 100, so the whole L2 basefee of every block in a proposal made after execution goes to that block's coinbase and nothing is retained by the L2 fee treasury, the Anchor contract 0x1670000000000000000000000000000000010001. The percentage is a constructor immutable of the inbox implementation, so the proposal is one upgradeTo on the inbox proxy 0x6f21C543a4aF5189eBdb0723827577e1EF57ef1f, executed by the DAO controller that owns it. 1 L1 action, no L2 leg, no initializer, immutables only.

The implementation is deployed and verified, and Proposal0026.action.md carries the executable calldata. Deployed on 2026-09-12 in L1 block 25,961,745 by DeployInboxUpgradeL1 (deployer 0x56706f118e42ae069f20c5636141b844d1324ae1, sources at 9deb5b590b4bf303ff161f0b8b14a49ab518a312); the address was checked on-chain before being written in.

Address
MainnetInbox impl 0xA18431d42C8dF9778905fBEa912aCF1881b49D2e codediff
LibInboxSetup (linked) 0x526957d1a25E9D3F5ab5a4926d07eEE5d612ED42 CREATE2 by the same run
LibForcedInclusion (linked) 0x511e1E5D9b9E23958076ccF1dD0033237a8cE4f8 CREATE2 by the same run

The Rationale section of Proposal0026.md is now written: it is what TAIKO holders read during the veto window, so it states where the non-coinbase share actually goes (an un-swept ETH balance in the Anchor, withdrawable by the DAO through the L2 delegate controller), the asymmetry the change trades on, the two limits of the refund, and a trade-offs section covering the forgone revenue and the fact that a proposer's own L2 gas round-trips fully at 100.

Under contracts/ three things move:

  • MainnetInbox.sol: basefeeSharingPctg: 75100. This is the change the proposal ships.
  • DevnetInbox.sol: the same flip, so local and devnet deployments match mainnet. See below.
  • LibL1Addrs.ZK_REQUIRED_VERIFIER: the live proof verifier 0x7284aaC0… (Proposal0019's ZkRequiredVerifier), which was only a Proposal0019 constant until now, so the deploy script reproduces all five live immutables from the address library.

The rest is the deploy script, the proposal, the action file, the runbook and tests. BuildProposal.sol is untouched.

Why this PR touches DevnetInbox and syncer_test.go

DevnetInbox.sol moves to basefeeSharingPctg: 100 as well, so local and devnet deployments behave like mainnet after execution. It is not in MainnetInbox's dependency tree and no deployed contract reads it; its only consumer is DeployProtocolOnL1.

That is what pulls packages/taiko-client/driver/chain_syncer/event/syncer_test.go in. The taiko-client integration tests deploy the protocol through DeployProtocolOnL1, and two of them — TestTreasuryIncome and TestTreasuryIncomeAllAnchors — asserted the treasury balance strictly grows. That only holds below 100: at 100 the coinbase takes the whole basefee and the treasury receives nothing. Both now derive the expectation from the inbox's own basefeeSharingPctg, so they are correct at any setting, and the per-transaction reconciliation in TestTreasuryIncome still pins the split exactly.

Kept here rather than split out, because it is load-bearing for the devnet flip in this PR: without it, merging this breaks main. Splitting them would only create a merge-order dependency — the test change would have to land first or main goes red — for no benefit, since the change is a no-op at 75.

Note that this PR's own integration lanes do not currently exercise the flip. taiko-client--test.yml checks the protocol out from main rather than from the PR, so they deploy DevnetInbox at 75 and only ever take the < 100 branch of the new assertions. #22142 fixes that; once it lands, merging main in here makes these lanes deploy this PR's contracts at 100.

The new implementation is the live one with one field changed

DeployInboxUpgradeL1 read the live proxy's getConfig() before broadcasting and would have aborted unless its five addresses were the LibL1Addrs constants it compiles in and its percentage still exactly 75 (OLD_BASEFEE_SHARING_PCTG); after deploying it compared the new implementation's getConfig() with the live one and would have aborted unless the percentage was the only difference. Read back on-chain at block 25,961,770: the new implementation's getConfig() equals the live proxy's in every field but basefeeSharingPctg (100 vs 75), it has 23,058 bytes of code, and proxiableUUID() returns the EIP-1967 implementation slot, which is what upgradeTo checks. test_mainnetInbox_MatchesTheLiveConfigExceptForBasefeeSharing pins every field of the live configuration as a literal against a MainnetInbox built from this tree.

Diffing the implementation's dependency tree (MainnetInbox.sol and its 23 imports under contracts/) from the commit the live implementation was built from (9078278909a43a83fc5bb2664f30b81eb5c967f6, Proposal0019) to main changes two files: MainnetInbox.sol (the literal, and the LibFasterReentryLock overrides that #22058 moved into the base contract) and EssentialContract.sol (#22058: the reentry lock in transient storage at the byte-identical _REENTRY_SLOT 0xa5054f72…; __reentry becomes private and keeps its slot). Nothing under contracts/layer1/core/ changed. MainnetInbox_Layout.sol is identical at both commits, _initialized is 3 and Inbox has no init4, the ABI and the Proposed event are unchanged, and LibInboxSetup.validateConfig allows <= 100, so 100 is the maximum rather than an edge case; both execution clients compute fee × pctg / 100, so the treasury remainder is exactly 0.

Evidence

  • The code is authenticated, not just its getters. Etherscan holds the verified source (MainnetInbox, solc 0.8.30, osaka, optimizer 200 runs). Independently, the creation code is reproduced byte for byte from this branch: forge build, patch the two link references with the deployed library addresses, append the constructor arguments, and the result equals the deployment transaction's input (24,947 bytes). forge verify-bytecode cannot do this for a library-linked contract ("Unlinked bytecode is not supported"), and compiling with --libraries changes the metadata hash, so the runbook documents the manual comparison instead.
  • Fork rehearsal against live mainnet state (Proposal0026Fork.t.sol, gated on L1_FORK_URL, passing on 2026-09-12 through a public RPC after the deployment): asserts the proxy still runs the Unzen implementation 0x5253D4C9…, executes the committed calldata from the DAO controller against the deployed implementation, and asserts the proxy answers 100 with every other configuration field, the core state, the last and last-finalized proposal hashes, the forced-inclusion queue and fee, the owner, the activation timestamp and the initializer version unchanged. Nothing is deployed by the test. A second test runs Controller.dryrun on the fork and expects DryrunSucceeded().
  • P=0026 pnpm proposal:dryrun:l1 simulated against the deployment RPC reverts with DryrunSucceeded(), the success signal.
  • Unit tests pin the encoding of the single upgradeTo, the revert on a missing implementation, the absence of an L2 leg, that the no-argument builder forwards the deployed address (a DEPLOYED_INBOX_IMPL literal, so a library edit cannot be mirrored silently), the full live configuration as literals, and that Proposal0026.action.md matches what the code builds.

Client rollout (runbook, Client rollout)

The drivers derive the byte from the Proposed event and provers take it from the event they verify, so neither needs a change. The whitelisted preconfer (Catalyst) caches getConfig().basefeeSharingPctg at startup, so it must be restarted right after execution: until then every block it preconfirms is re-derived with 100 once proposed (same transactions, different extraData and state root), i.e. one preconfirmation reorg per block. Blocks preconfirmed under 75 but carried by the first post-execution proposal are re-derived the same way, once; executing right after a proposal lands bounds that to a few blocks.

Review notes

  • Error declarations sit at the top of the script and proposal files, matching Proposal0024.s.sol and DeployERC20VaultUpgradeL1.s.sol.
  • MainnetInbox links two libraries (LibForcedInclusion and LibInboxSetup both have public functions), so the deployment was three creates, all in one block with status 1; the receipts were checked rather than forge's summary line.
  • Contract logs are not touched here. They follow the docs(protocol): update mainnet contract logs for Proposal0023 #22120 pattern: a separate PR with the post-execution entries pre-written as TBD, filled after execution.
  • DeployInboxUpgradeL1 must not be re-run; it also refuses to run once the live proxy answers 100.

Next steps

  1. Create the DAO proposal from Proposal0026.action.md (target: DAO controller, execute(bytes)).
  2. After execution: restart the whitelisted preconfer nodes, and record the upgrade in the contract logs.

🤖 Generated with Claude Code

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

@dantaik
dantaik self-requested a review September 16, 2026 08:21
…sal IDs (#22154)

Co-authored-by: Claude <noreply@anthropic.com>
dantaik
dantaik previously approved these changes Sep 21, 2026
@dantaik dantaik changed the title feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0024) feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0025) Sep 21, 2026
@dantaik
dantaik changed the base branch from main to refactor/proposal0023-to-0024 September 21, 2026 09:42
auto-merge was automatically disabled September 21, 2026 09:42

Merge commits are not allowed on this repository

Base automatically changed from refactor/proposal0023-to-0024 to main September 21, 2026 09:59
@dantaik
dantaik dismissed their stale review September 21, 2026 09:59

The base branch was changed.

@dantaik
dantaik self-requested a review September 21, 2026 10:00
@dantaik
dantaik enabled auto-merge September 21, 2026 10:00
@github-actions

Copy link
Copy Markdown
Contributor

🐋 DeepSeek Code Review

🟡 Warnings

  • Proposal0025ForkTest can underflow on a fork with no proposals.
    _snapshot() and _assertOnlyTheSharingPercentageChanged() compute _before.coreState.nextProposalId - 1 without checking nextProposalId > 0. The test is meant for live mainnet, but if anyone runs the fork test against an archive fork after the Unzen implementation but before the first inbox proposal, it will revert with an underflow. Add a guard/assert or document the requirement.

  • DeployInboxUpgradeL1 broadcasts before validating the new implementation’s full config.
    The script sends the deployment transaction and only then calls _checkConfig; if that validation reverts, the broadcasted implementation remains on-chain and the script fails. This is operationally recoverable, but it would be safer to validate the configuration in a non-broadcast simulation or use vm.etch/state override before spending mainnet gas.

🔵 Suggestions

  • Compare config structs field-by-field instead of abi.encode + keccak256.
    _checkConfig currently uses keccak256(abi.encode(_new)) == keccak256(abi.encode(_live)). It works for the current all-fixed-value Config, but direct field comparisons would be clearer and less brittle if the struct ever gains dynamic fields.

🟢 What Looks Good

  • The PR is very well verified: bytecode reconstruction, fork rehearsal against live state, dry-run, action-file generation pinned by tests, and no storage/ABI changes.
  • basefeeSharingPctg = 100 is explicitly allowed by validateConfig, and the change is immutable-only with no initializer, so the upgrade surface is minimal.

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

@dantaik
dantaik marked this pull request as draft September 22, 2026 08:00
auto-merge was automatically disabled September 22, 2026 08:00

Pull request was converted to draft

Renames the basefee-sharing proposal from 0025 to 0026 across its script,
proposal doc, action file, tests and harness, and updates the references in
`MainnetInbox.sol`, `DeployInboxUpgradeL1.s.sol` and the layer1 gas report.

Rename only: no calldata, address or configuration value changes. The action
file's calldata is unchanged, and the gas report was regenerated with
`forge snapshot` rather than hand-edited.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AhZUYdta3M1mhVs7VzVFyZ
@dantaik dantaik changed the title feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0025) feat(protocol): raise the inbox basefee sharing percentage to 100% (Proposal0026) Sep 22, 2026

This branch has not been deployed

No deployments
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.

3 participants