Skip to content

fix(archiver): compare fee modifier when checking a checkpoint against L1 - #136

Merged
rkarabut merged 2 commits into
mainfrom
rk/fix-a1783-feemodifier-equivocation
Sep 15, 2026
Merged

rkarabut merged 2 commits into
mainfrom
rk/fix-a1783-feemodifier-equivocation

Conversation

@rkarabut

@rkarabut rkarabut commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

The archiver decides whether a local checkpoint matches the one on L1 by comparing the header and the archive root, while the committee actually signs feeAssetPriceModifier along with them. The mismatch won't be detected if the fee modifier differs.

I don't think this could be exploited in a meaningful way (besides for some purpose making previous committee signatures invalid) but for correctness we should check all the fields we sign on.

@rkarabut
rkarabut requested a review from spalladino September 9, 2026 11:05
@labsbox
labsbox Bot force-pushed the rk/fix-a1783-feemodifier-equivocation branch from be726c3 to 42e078b Compare September 9, 2026 11:06
@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge, with the added comparison correctly routing modifier-only divergence through the existing equivocation recovery path.

Summary

  • Adds feeAssetPriceModifier to the checkpoint matching condition.
  • Adds a regression test confirming modifier-only divergence emits an equivocation event and still ingests the authoritative L1 checkpoint.

@spalladino spalladino left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Can you add the feeAssetPriceModifier to the attributes logged in the log.warn below for debugging, and then merge?

@rkarabut
rkarabut enabled auto-merge (squash) September 9, 2026 16:49
@rkarabut
rkarabut force-pushed the rk/fix-a1783-feemodifier-equivocation branch from 0215159 to 0ca7b90 Compare September 14, 2026 08:57
rkarabut and others added 2 commits September 15, 2026 08:39
…t L1

The L1-sync divergence check compared the header and archive root but not
feeAssetPriceModifier, the third field the committee signs (it is not part of
the header). A checkpoint differing only in the fee modifier was treated as
identical, so the mismatch branch was skipped. Compare it too, so the check
covers the full signed payload. Adds a regression test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…view nit)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rkarabut
rkarabut force-pushed the rk/fix-a1783-feemodifier-equivocation branch from 0ca7b90 to c506d1d Compare September 15, 2026 08:40
@rkarabut
rkarabut merged commit f697144 into main Sep 15, 2026
4 checks passed
@rkarabut
rkarabut deleted the rk/fix-a1783-feemodifier-equivocation branch September 15, 2026 08:40
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.

2 participants