spv: sync compact filters and match watched scripts (BIP157, stacked on #384) - #386
Open
xanimo wants to merge 10 commits into
Open
spv: sync compact filters and match watched scripts (BIP157, stacked on #384)#386xanimo wants to merge 10 commits into
xanimo wants to merge 10 commits into
Conversation
xanimo
force-pushed
the
0.1.5-dev-cf-spv
branch
2 times, most recently
from
August 3, 2026 03:01
d207f69 to
5ab5a00
Compare
Golomb-Coded Set construction and matching, the encoding BIP158 compact block filters are built from: Golomb-Rice encode/decode over a bitstream, the SipHash-2-4 based mapping of an element into the [0, N*M) range, and set construction and membership queries over a sorted delta list. This is the arithmetic layer only. It has no dependency on any P2P message type, on filter headers, or on the network at all -- it takes byte strings in and answers membership questions -- so it builds and tests standalone, and does so with WITH_NET=OFF. Placed in the unconditional source lists of both build systems rather than under WITH_NET, which is where filter code would otherwise land. Guarding it on net would have made the two build systems disagree (Makefile.am lists core sources unconditionally), and that disagreement is not theoretical: a source present in one build system and absent from the other is invisible until the other platform links. Verified src/*.c membership matches between Makefile.am and CMakeLists.txt after the change. 79/79 with test_golomb registered and running; CMake build clean with WITH_NET=OFF.
One filter header per CFCHECKPT_INTERVAL (1000) blocks, used to validate a peer's cfcheckpt response before any filter header it serves is trusted. Mainnet carries 6239 entries; testnet and regtest are empty for now. Kept out of chainparams.c on purpose. It is 6200 lines of generated data, which would bury the chain parameter definitions it would otherwise sit between, and it means adding block checkpoints and adding filter checkpoints touch different files -- so work on header sync and work on compact filters stop colliding in the same region of the same file. The regtest array carries a single zero entry with a count of 0: regtest chains are generated per test run, so there is no stable header to pin, and the entry exists only because a zero-length array is not valid C.
The BIP157 wire layer on top of the GCS encoding: getcfilters, getcfheaders and getcfcheckpt requests, the cfilter, cfheaders and cfcheckpt responses, filter header chaining, and validation of a served filter against the header committed to by that chain. Two things worth a reviewer's attention. The cfilter message follows BIP157 field order -- filter_type first, then block_hash, then the filter bytes. This is worth stating because it has been the other way round here before, and because Dogecoin Core's own compact filter work has been observed serialising block_hash first, which is not the specified order. A peer pairing this code with an implementation that swaps those two fields will parse filter_type out of block_hash[0], take the next 32 bytes as the hash, and then hit a bogus varint -- deserialisation failure at best, a silent header mismatch and a ban at worst. That failure mode does not show up in a round-trip test, because a swapped serialiser and a swapped deserialiser agree with each other perfectly. It needs a test against a fixed BIP157 payload with known bytes. Note also that the on-disk cfilters format is framed separately from the wire format, so a client reading filters from a bootstrap or cache exercises none of this. The counts in cfheaders and cfcheckpt are bounded before they size an allocation. Both are compact_size values straight off the wire, and vector_new() rounds its reservation up to a power of two, so 0xFFFFFFFF became a ~34 GB allocation before a single hash was read -- a memory exhaustion DoS from a ten byte message, the same class as the getheaders locator count. Each entry is a 32 byte hash, so any count the remaining buffer cannot hold is rejected, and cfheaders additionally takes BIP157's 2000 hash cap. Tests cover the wire format, header chaining, checkpoint validation, both unbounded-count regressions, and payload tampering: every single-bit flip of a filter payload, plus truncation and extension, must fail validation against the header computed from the original. That last one is the attack the validation exists to stop, since a peer able to strip a script from a filter makes the client skip the block that pays it. 80/80 with test_compact_filter registered and running.
The Windows build failed to link every executable with six unresolved
externals -- the three CF checkpoint arrays and their three counts:
spvnode.vcxproj : error LNK2019: unresolved external symbol
dogecoin_mainnet_cf_checkpoint_array
dogecoin.dll : fatal error LNK1120: 6 unresolved externals
The file was committed, listed in both build systems, and compiled
cleanly. The tell was in the build log: chainparams.c was compiled nine
times and cf_checkpoints.c once.
chainparams.c is declared TARGET_SOURCES(... PUBLIC ...), so CMake
compiles it into every consumer target as well as the library.
cf_checkpoints.c was PRIVATE, so only the library got it. That is
invisible on Linux, where a shared object resolves the data symbol for
the executables anyway, and fatal on a Windows DLL build, where data
symbols do not cross the boundary without dllimport -- and
LIBDOGECOIN_API expands to nothing for consumers.
So the existing PUBLIC on chainparams is deliberate rather than
incidental: it is what lets the CLI tools reference chain data arrays at
all on Windows. Filter checkpoints are the same kind of thing and need
the same treatment.
Verified by reproducing the Windows configuration locally
(-DBUILD_SHARED_LIBS=1): cf_checkpoints.c now compiles into 9 targets,
matching chainparams.c exactly, and the shared build links clean.
… does The two build systems disagreed. Makefile.am lists src/compact_filter.c in the unconditional sources, CMake had it inside IF(WITH_NET). Since test/compact_filter_tests.c is registered unconditionally in both, a CMake build configured with -DWITH_NET=OFF compiled the tests but not the code under test and failed to link with 55 undefined references. compact_filter.c has no net dependency to justify the guard: it includes only cf_checkpoints, chainparams, hash, mem, serialize and utils, and references no node, nodegroup or event_base symbol. It is message serialization and filter header arithmetic, in the same category as golomb.c, which is already unconditional. Aligning CMake with Makefile.am rather than gating the test, so the two build systems describe the same library. WITH_NET=OFF: 74/74, was failing to link. WITH_NET=ON with -DBUILD_SHARED_LIBS=1: 80/80, unchanged.
compact_filter.h referenced nothing from either: no dogecoin_headers_db, no dogecoin_headers_db_interface, no dogecoin_blockindex. headersdb.h in particular includes logdb/logdb.h, so anything including compact_filter.h also needed -I src/logdb/include on its command line, even when it never touched logdb. That is invisible inside the library, whose CPPFLAGS already carry that path, and only shows up when something outside it -- a fuzz harness, a consumer -- includes the header directly and fails with include/dogecoin/headersdb.h:37:10: fatal error: 'logdb/logdb.h' file not found The header now pulls only what it uses. Removing the two lines leaves it self-contained given the include path for dogecoin's own headers. WITH_NET=ON with -DBUILD_SHARED_LIBS=1: 80/80. WITH_NET=OFF: 74/74.
Adds the file-backed stores behind the BIP157 filter header chain: cfheaders.dat holding height+filter_header records, and cfilters.dat holding the raw filter payloads. The on-disk layout is segregated per network, <datadir>/filter/basic/<chainname>/. Sharing one directory across chains is not merely untidy: a regtest or testnet client loads the mainnet cache at startup, rescans it, matches mainnet blocks its peer has never heard of, and then blocks forever waiting for them -- while writing its own filters back into the mainnet files. A one-time rename migrates a pre-segregation mainnet cache into the new location, and only mainnet, because only mainnet data is worth keeping. Bound the cfilters record length before allocating. data_len is read straight off disk and was passed to dogecoin_malloc unchecked, so a corrupt or tampered file could request up to 4 GiB (CWE-400). A cfilter cannot legitimately exceed the P2P message that delivered it, so DOGECOIN_MAX_P2P_MSG_SIZE is the ceiling. The short read that follows would have failed the call anyway, but only after the allocation had already been attempted. Tests cover the round trip, the genesis header at offset 8, reset(), in-memory mode, a trailing partial record (dropped rather than misparsed), and the length bound. The bound test asserts on the allocator rather than the return value. An earlier version of it passed with the bound removed: malloc(4 GiB) succeeds under overcommit and the oversized read then fails, so iterate() returns false either way and the return value cannot tell the two apart. It now installs a recording dogecoin_mem_mapper and asserts no allocation exceeded the bound, which was confirmed by removing the bound and watching the test fail. 81/81, and verified against a -DBUILD_SHARED_LIBS=1 configure.
The test hardcoded /tmp/libdogecoin_cfheaders_test.dat. Neither Windows nor Android has /tmp, so x86_64-win-native and aarch64-android failed: cfheadersdb: cannot open /tmp/libdogecoin_cfheaders_test.dat: No such file or directory Use plain relative filenames in the working directory, as the wallet and spv tests already do. Both files are removed at the end of the test. Same portability class as the truncate()/_chsize() call this test already avoids -- caught there, missed here.
Wires the BIP157 message flow into the SPV client: getcfcheckpt to pin
the peer's checkpoints, getcfheaders to build and validate the filter
header chain, getcfilters to fetch filters, GCS matching against watched
scripts, and a getdata for the blocks that matched.
Filter headers are persisted through the cfheaders/cfilters DBs added in
the previous layer, so a restart resumes from the stored tip rather than
re-downloading. Cached filters are rescanned at startup, which covers
scripts registered after the filters were stored.
BIP37 and BIP157 are mutually exclusive here, by design and for privacy:
a bloom filter tells the peer which scripts are being watched, which is
the disclosure compact filters exist to avoid. Compact filters are on by
default and dogecoin_spv_client_filterload fails closed while they are,
so a BIP37 consumer opts out explicitly via
dogecoin_spv_enable_compact_filters(client, false). The test asserts both
halves of that.
headersdb gains a block-hash-by-height lookup with a resume cursor.
cfheaders batches request ascending heights, and rescanning the whole
file per lookup is O(N^2) over a 6.2M-record file; resuming from the last
match keeps it linear.
Deliberately not included, to keep this reviewable and because each
belongs elsewhere:
- parallel cfilter/cfheaders download. The state fields live in the
compact_filter layer; the logic lands in its own PR. Every path here
takes the sequential branch.
- the par_hdr parallel genesis header download, which is dogecoinfoundation#378.
- skip_pow and the stored-hash-on-load shortcut. Both were in the
prototype branch, both weaken what header loading verifies, and
neither is needed by filter sync. They belong with the bulk-load work
where their trust assumptions can be reviewed as a unit. Header
loading here is unchanged from 0.1.5-dev.
Report the height range actually covered on completion. cf_scan_start_height
was only set on one of the two scan-start paths, and the log falls back to 1
when it is unset, so a run that covered the checkpoint tail reported
"scanned heights 1..tip" -- overstating coverage in exactly the way that
hides a gap.
Verified against a local 1.14.99 node serving BIP157 with the corrected
cfilter wire order, run with an isolated HOME so the real filter cache was
untouched: 1442 cfilters parsed off the wire, full
getcfcheckpt/getcfheaders/getcfilters round trip, zero validation errors or
misbehaviour, and the completion line now reports 6314395..6315836 rather
than 1..6315836.
That run does not exercise GCS matching: -a is not wired to the filter
state until the CLI layer, so no scripts were watched. The matching block
is byte-identical to the prototype's sequential arm, which was separately
confirmed to match at a predicted height over the wire, and the unit tests
cover GCS matching directly -- but this layer's own end-to-end run proves
the wire path, not the match path.
WITH_NET=ON with -DBUILD_SHARED_LIBS=1: 81/81. WITH_NET=OFF: 74/74.
xanimo
force-pushed
the
0.1.5-dev-cf-spv
branch
from
August 5, 2026 21:04
5ab5a00 to
8408269
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| cstring *old_obj = NULL; | ||
| legacy_filter_path(&old_obj, filename); | ||
| if (stat(old_obj->str, &sb) == 0 && sb.st_size > 0) { | ||
| if (rename(old_obj->str, new_path) == 0) { |
| dogecoin_bool create = (stat(path, &sb) != 0) || (sb.st_size < (long)CF_HEADERS_FILE_HDR_LEN); | ||
|
|
||
| if (!create) { | ||
| db->file = fopen(path, "r+b"); |
| fclose(db->file); | ||
| db->file = NULL; | ||
| } | ||
| remove(path); |
| } | ||
|
|
||
| /* Create fresh v2 file */ | ||
| db->file = fopen(path, "w+b"); |
CodeQL cpp/world-writable-file-creation on test/cfheadersdb_tests.c. fopen(path, "wb") leaves the mode to the process umask, so the file can land world-writable and another local user could rewrite it between creation and read. Creates it 0600 via open()+fdopen() on POSIX; Windows keeps fopen, which does not have the same exposure.
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
2 participants
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.
Layer 4 of the BIP157/158 stack — stacked on #384. Review only the top
commit; the ones below it are #380, #381 and #384.
Wires the BIP157 message flow into the SPV client:
getcfcheckptto pin thepeer's checkpoints,
getcfheadersto build and validate the filter headerchain,
getcfiltersto fetch filters, GCS matching against watched scripts,and a
getdatafor blocks that matched. Filter headers persist through the DBsfrom #384, so a restart resumes from the stored tip. Cached filters are
rescanned at startup, covering scripts registered after the filters were
stored.
BIP37 and BIP157 are mutually exclusive
By design, and for privacy: a bloom filter tells the peer which scripts are
being watched, which is exactly the disclosure compact filters exist to avoid.
Compact filters are on by default and
dogecoin_spv_client_filterloadfailsclosed while they are. A BIP37 consumer opts out explicitly:
The test asserts both halves — that
filterloadfails with CF on, and succeedsafter opting out.
headersdb lookup
Adds block-hash-by-height with a resume cursor.
cfheadersbatches requestascending heights, and rescanning the whole file per lookup is O(N²) over a
6.2M-record file; resuming from the last match keeps it linear.
Deliberately excluded
Each belongs elsewhere, and saying so explicitly because all three were present
in the prototype branch this was extracted from:
lands in its own PR. Every path here takes the sequential branch.
par_hdrparallel genesis header download — that is spv: parallel genesis header download over checkpoint segments #378.skip_powand the stored-hash-on-load shortcut. Both weaken what headerloading verifies (
skip_powskips scrypt PoW re-verification on load; theother trusts the hash stored in the file rather than recomputing it from the
header bytes, so a tampered
headers.dbcould assert a hash inconsistentwith its own header). Neither is needed by filter sync. They belong with the
bulk-load work where their trust assumptions get reviewed as a unit. Header
loading in this PR is unchanged from
0.1.5-dev.Integration note for #378
dogecoin_spv_client_enable_genesis_headersis the seam between #378 and thiswork: in the prototype it both initialises
par_hdrand resetscf_start_heightand clears CF DB state. #378 cannot reference the CF half andthis PR has no
par_hdr, so whichever merges second needs to reconcile the twohalves. Flagging it so it does not merge as a silent no-op.
Verification
Run against a local
1.14.99node serving BIP157 with the corrected cfilterwire order, with an isolated
HOMEso the real filter cache was untouched:getcfcheckpt/getcfheaders/getcfiltersround trip6314395..6315836That last point is also a fix in this PR.
cf_scan_start_heightwas set on onlyone of the two scan-start paths, and the completion log falls back to
1whenunset, so a run covering just the checkpoint tail reported
scanned heights 1..tip— overstating coverage in exactly the way that hides a gap.What that run does not prove: GCS matching.
-ais not wired to the filterstate until the CLI layer, so no scripts were watched and
gcs_filter_match_anywas never called. The matching block is byte-identicalto the prototype's sequential arm (verified by diff), that arm was separately
confirmed to match at a predicted height over the wire, and the unit tests
cover GCS matching directly — but this layer's own end-to-end run exercises the
wire path, not the match path. The end-to-end match returns at the CLI layer.
WITH_NET=ONwith-DBUILD_SHARED_LIBS=1: 81/81.WITH_NET=OFF: 74/74.