Skip to content

feat(bridge): record the treasury fee divisor in force at reveal - #23

Merged
mswilkison merged 3 commits into
masterfrom
feat/record-treasury-fee-divisor-at-reveal
Oct 4, 2026
Merged

mswilkison merged 3 commits into
masterfrom
feat/record-treasury-fee-divisor-at-reveal

Conversation

@mswilkison

@mswilkison mswilkison commented Sep 13, 2026 •

Copy link
Copy Markdown

Deploy deferred by request. Bundle deployment and the historical re-sync with the next release rather than tagging a release for this PR.

Problem

treasuryFee: 0 cannot distinguish a deposit made while protocol fees were disabled from a deposit whose fee was waived. Consumers need the treasury fee divisor that applied when the deposit was revealed.

A contract call in the reveal handler returns end-of-block state. If a reveal is followed by a governance update from 0 to 500 in the same block, that call records divisor 500 for the earlier no-fee deposit and incorrectly makes it look fully waived.

Change

  • Add nullable Deposit.treasuryFeeDivisorAtReveal and snapshot the tracked divisor at each reveal.
  • Seed BridgeState.depositTreasuryFeeDivisor to the initializer's value of 2000 on Initialized(1), then apply DepositParametersUpdated in event order. Both configured start blocks include initialization; commit-pinned deployment receipts and initializer sources are documented in docs/treasury-fee-divisor.md.
  • Preserve existing state across reinitialization. If initialization history is unavailable, leave the snapshot null until a parameter update supplies the value.

A reveal, update from 0 to 500, and second reveal in one block now retain divisors 0 and 500 respectively. Later updates cannot change either snapshot. This removes the per-reveal parameter call and does not hardcode a fee activation date.

Consumers can distinguish a zero divisor from a positive divisor when interpreting the recorded fee. Null means the historical divisor is unknown. Historical deposits require a re-sync, and consumers must query the new field after deployment.

Validation

  • yarn install --frozen-lockfile, yarn codegen, yarn build-mainnet, and yarn build-sepolia pass.
  • Nine handler regression tests pass on Node 22 and 26. They cover initialization, both zero/nonzero transitions within one block, successive updates, missing history, reinitialization, unrelated singleton state, and unreadable legacy deposit records. CI runs these alongside the existing builds.
  • The ordering regression detects the original bug: the previous mapping records 500 for both deposits where the expected snapshots are 0 and 500.
  • All 58 existing toolchain and cutover tests pass; git diff --check passes.

The handler tests execute the mapping with mocked Graph dependencies; code generation and both network builds check schema, ABI, and AssemblyScript compatibility.

`treasuryFee: 0` is ambiguous. It means either "the protocol charged no
fee at the time" or "this depositor's fee was waived", and nothing in
the indexed data distinguishes them. The explorer currently resolves
that by showing no waiver badge at all, so a genuine full waiver renders
as a blank fee.

Mainnet makes the ambiguity concrete: the earliest deposit carrying a
fee is 2026-04-15, none of the ~17,600 before it has one, and since then
the only zero-fee deposits belong to a single address whose fee is
waived. Both cases index as 0.

Records `Bridge.depositParameters().depositTreasuryFeeDivisor` as it
stood in the reveal block, so a consumer can tell them apart:

  treasuryFee 0, divisor 0    -> no fee regime; nothing waived
  treasuryFee 0, divisor > 0  -> genuine full waiver
  treasuryFee below expected  -> partial waiver (already detected)

The field is nullable and left null when the call reverts, so a failed
read stays distinct from a real zero. The call is guarded for the same
reason as the `deposits` read beside it: an unguarded one would halt
indexing, as it did at block 16523905.

Costs one extra eth_call per DepositRevealed, roughly 17,600 across all
of history, which is negligible against 12.9M blocks.

Deliberately avoids exposing the 2026-04-15 boundary as a constant, so
consumers keep working when governance next changes the divisor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FvT6VkqyTSCcs89LBem9Z8
@coderabbitai

coderabbitai Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a3342615-ee1e-4a8a-9931-3a57c7b16491
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mswilkison
mswilkison merged commit 7e445c6 into master Oct 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant