feat(firewood/syncer): implement change proof support#5385
Draft
rkuris wants to merge 1 commit into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Implements real change-proof support in the firewood syncer backend, replacing the previous struct{} stub with a *ChangeProof type that wraps *ffi.ChangeProof and the verification *ffi.Proposal. Bumps the firewood FFI dependency to v0.5.0 which provides the required APIs.
Changes:
- Add
ChangeProofwrapper type with real marshal/unmarshal and implementGetChangeProof/VerifyChangeProof/CommitChangeProofon*database. - Switch the type parameter from
struct{}to*ChangeProofacrossdatabase,evmDB, handler constructors, and external callers; implementevmDB.CommitChangeProofto enqueue code hashes before commit. - Bump
github.com/ava-labs/firewood-go-ethhash/ffitov0.5.0.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Bumps firewood FFI to v0.5.0 (required for change-proof APIs). |
| database/merkle/firewood/syncer/proof.go | Adds ChangeProof wrapper and real marshaler implementation. |
| database/merkle/firewood/syncer/syncer.go | Implements GetChangeProof/VerifyChangeProof/CommitChangeProof; switches generics to *ChangeProof. |
| database/merkle/firewood/syncer/evm_syncer.go | Implements evmDB.CommitChangeProof (code-hash enqueue + commit) and updates generics. |
| database/merkle/firewood/syncer/handler.go | Updates handler constructors to new type parameter. |
| database/merkle/firewood/syncer/syncer_test.go | Updates syncer type parameter in test. |
| graft/evm/sync/evmstate/firewood_syncer.go | Updates FirewoodSyncer.s type parameter to *ChangeProof. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
## Why this should be merged
The firewood syncer backend previously stubbed out the change-proof side of
the `sync.DB` interface (placeholder type `struct{}`, marshaler/methods
returned `"not implemented"`). Range proofs were the only working path, so
the syncer fell back to full range proofs whenever a change proof would
have been more efficient. Firewood v0.5.0 exposes the full change-proof
API (`(*Database).ChangeProof`, `.VerifyChangeProof` → `*Proposal`,
`(*Proposal).CommitWithRebase`, `(*ChangeProof).FindNextKey`), so this
change lights up the real implementation end-to-end.
## How this works
- `proof.go`: add a `ChangeProof` wrapper around `*ffi.ChangeProof` that
holds the `*ffi.Proposal` produced by `VerifyChangeProof` until commit
time; replace the stub marshaler with the real one.
- `syncer.go`: switch the type parameter from `struct{}` to `*ChangeProof`
on `*database` and the `New`/`newWithDB` constructors; implement
`GetChangeProof` (server side), `VerifyChangeProof` (client side, returns
a Proposal), and `CommitChangeProof` (`Proposal.CommitWithRebase` +
`FindNextKey` for the next-key continuation, matching the existing
range-proof pattern).
- `evm_syncer.go`: implement `(*evmDB).CommitChangeProof` to enqueue code
hashes from the proof before committing, mirroring the range-proof flow.
`(*ffi.ChangeProof).CodeHashes()` is currently a no-op in firewood; the
iterator loop is kept so we pick up real hashes automatically once it
is implemented.
- `handler.go`: change-proof handler type params.
- External callers (`graft/evm/sync/evmstate/firewood_syncer.go`,
`syncer_test.go`) updated to the new `*ChangeProof` type parameter.
Firewood version bumped to v0.5.0 in \`go.mod\`. v0.5.0 contains several
correctness fixes that this code depends on (no-op proposal handling,
\`FindNextKey\` returning nil at trie right edge, range-proof boundary
checks).
## How this was tested
Ran the firewood syncer test suite against a local firewood build at the
v0.5.0 source. All 11 sub-tests pass:
- \`Test_Firewood_Sync\` (7 cases — range-proof sync from empty/non-empty
client to various server sizes up to 100k keys)
- \`Test_Firewood_Sync_UpdateSyncTarget\` (4 cases — sync target update
mid-flight, exercising the real change-proof path including
\`partial_sync_*_then_update\` scenarios)
CI will fail until firewood v0.5.0 is published; reviewers can validate
locally by pointing the firewood dependency at a v0.5.0 build via
\`go mod edit -replace\`.
## Need to be documented in RELEASES.md?
No.
3c58984 to
9148af6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this should be merged
The firewood syncer backend previously stubbed out the change-proof side of the
sync.DBinterface (placeholder typestruct{}, marshaler/methods returned"not implemented"). Range proofs were the only working path, so the syncer fell back to full range proofs whenever a change proof would have been more efficient. Firewood v0.5.0 exposes the full change-proof API ((*Database).ChangeProof,.VerifyChangeProof→*Proposal,(*Proposal).CommitWithRebase,(*ChangeProof).FindNextKey), so this change lights up the real implementation end-to-end.How this works
proof.go: add aChangeProofwrapper around*ffi.ChangeProofthat holds the*ffi.Proposalproduced byVerifyChangeProofuntil commit time; replace the stub marshaler with the real one.syncer.go: switch the type parameter fromstruct{}to*ChangeProofon*databaseand theNew/newWithDBconstructors; implementGetChangeProof(server side),VerifyChangeProof(client side, returns a Proposal), andCommitChangeProof(Proposal.CommitWithRebase+FindNextKeyfor the next-key continuation, matching the existing range-proof pattern).evm_syncer.go: implement(*evmDB).CommitChangeProofto enqueue code hashes from the proof before committing, mirroring the range-proof flow.(*ffi.ChangeProof).CodeHashes()is currently a no-op in firewood; the iterator loop is kept so we pick up real hashes automatically once it is implemented.handler.go: change-proof handler type params.graft/evm/sync/evmstate/firewood_syncer.go,syncer_test.go) updated to the new*ChangeProoftype parameter.Firewood version bumped to v0.5.0 in `go.mod`. v0.5.0 contains several correctness fixes that this code depends on (no-op proposal handling, `FindNextKey` returning nil at trie right edge, range-proof boundary checks).
How this was tested
Ran the firewood syncer test suite against a local firewood build at the v0.5.0 source. All 11 sub-tests pass:
CI will fail until firewood v0.5.0 is published; reviewers can validate locally by pointing the firewood dependency at a v0.5.0 build via `go mod edit -replace`.
Need to be documented in RELEASES.md?
No.
Why this should be merged
How this works
How this was tested
Need to be documented in RELEASES.md?