feat(p2p/sensor): rewrite the ClickHouse writer for the redesigned schema by minhd-vu · Pull Request #975 · 0xPolygon/polygon-cli

minhd-vu · 2026-07-29T18:15:07Z

Description

Rewrites the ClickHouse sensor backend (merged in #961) for the redesigned schema in
0xPolygon/polygon-infrastructure#1856, and documents the data model of both
backends.

Rebased onto main now that #961 has landed, so this PR is just the rewrite —
three commits.

The design

The schema splits content-addressed facts from observations, and the writer
is structured to match. Every row it puts into a fact table (blocks,
block_bodies, block_txs, transactions) is complete and a pure function of the
hash it is keyed by, so two sensors that see the same block emit byte-identical rows
and no write can partially overwrite another. Everything situational — which sensor,
which peer, when, and how the block was learned — goes into the event streams.

The defect that motivated it

A header carries no transaction count, so the header path wrote tx_count = 0 onto
the block row. Because the engine kept the row with the newest version column, a
header arriving after the full block — routine during parent backfill —
permanently replaced the real counts with zeros.

Body facts now live in block_bodies, which the header path never writes. It is
keyed by hash alone: a body can arrive before, or without, its header (the sensor
requests the two separately and they race), so the height is not reliably known
there, and carrying number would mean writing 0 when unknown — reintroducing the
exact problem. Regression test: TestClickHouseHeaderDoesNotClobberBody.

Other changes

Observation attributes leave the fact rows. sensor_id, ingested_at and
is_parent move to block_events, with is_parent becoming
source='header_backfill'. Announced total_difficulty moves too — it comes from
the NewBlock announcement, so it describes the announcement, not the block.
Block heights reach the backend. WriteBlockEvents takes
[]database.BlockAnnouncement, and NewBlockHashesPacket is redefined as a slice
of that type so a decoded packet passes straight through with no copy or
conversion. RLP is unaffected — the wire format follows field order, not the
element type name, and the existing announce tests round-trip through the real
codec. block_events needs the height because block_number leads its sort key.
block_txs records the block → transaction mapping, previously thrown away
entirely — "which txs were in block X" was unanswerable in either direction.
Peer identity is the devp2p node id on both event streams, not the enode URL.
peers and events were previously in different key spaces and could not be joined
at all; they now join at a 100% match rate.
--peer-snapshot-interval (default 30s) for the database write, leaving the 2s
tick to drive the Prometheus gauge and local peer file. Writing up to --max-peers
rows every 2s was a lot of near-duplicate data to answer one question ("who is
connected now"), and reads go through peers_current.
NodeList reads the narrow peers_current rollup instead of grouping over the
event firehose.
Wider types: base_fee → UInt256, post-Shanghai/Cancun header fields, and per
transaction the chain id, calldata selector and size, and access-list / blob /
auth-list counts.

Docs

Four documents under cmd/p2p/sensor/, linked from the embedded usage text: schema
and write-path diagrams for both backends, so the ClickHouse and Datastore shapes
can be compared directly. The erDiagram column lists were mechanically diffed
against system.columns on a live server — no drift.

Jira / Linear Tickets

DVT-000

Testing

Validated against clickhouse/clickhouse-server:25.3 (the deployed version) and a
live sensor against Polygon mainnet.

go build ./..., go vet ./p2p/... ./cmd/p2p/..., go test ./p2p/... ./cmd/... — no failures, re-run after rebasing onto main
TestClickHouseWrites — every table the writer targets receives its row, with
round-tripped values asserted (base_fee as UInt256, recovered signer,
total_difficulty and node id on the event, the block_txs mapping, calldata
selector/size)
TestClickHouseHeaderDoesNotClobberBody — writes a full block then the same
header a second later (the ordering that used to lose the counts) and asserts
tx_count survives, with both events distinguishable by source
TestBlockEventFullVsFirst extended to assert the announced height reaches the
backend — a zero there would silently mis-key every row
Live sensor against mainnet for ~5 min: all tables and all four MV rollups
populated; zero dropped rows, zero insert failures; clean batcher drain on
shutdown
Docs regenerated (go run docutil/*.go); all 5 mermaid diagrams parsed with the
real mermaid parser

Notes

Ordering: the schema (0xPolygon/polygon-infrastructure#1856) must be applied
before this ships; the reader side is 0xPolygon/panoptichain#97.
The Datastore and JSON backends are unchanged in behaviour — they take the new
announcement type and ignore the heights.
Worth knowing when reading the data: latency_ms is legitimately negative for
many Bor blocks, because the header timestamp is the proposer's slot time and can
be ahead of actual propagation.

🤖 Generated with Claude Code

Follows the schema redesign in polygon-infrastructure, which splits content-addressed facts from observations. Every row this writer puts into a fact table (blocks, block_bodies, block_txs, transactions) is now complete and a pure function of the hash it is keyed by, so two sensors seeing the same block emit byte-identical rows and no write can partially overwrite another. The defect that motivated it: a header carries no transaction count, so the header path wrote tx_count=0 onto the block row, and because the engine kept the row with the newest version column a header arriving after the full block -- routine during parent backfill -- permanently replaced the real counts with zeros. Body facts now live in block_bodies, keyed by hash alone (a body can arrive before, or without, its header, so the height is not reliably known there), which the header path never writes. Covered by TestClickHouseHeaderDoesNotClobberBody. Other changes: - Move sensor_id, ingested_at and is_parent off the fact rows into block_sightings, with is_parent becoming source='header_backfill'. Announced total_difficulty moves there too: it comes from the NewBlock announcement, so it describes the announcement rather than the block. - Thread announced block heights through the Database interface as []BlockAnnouncement, and redefine NewBlockHashesPacket as a slice of that type so a decoded packet reaches the backend with no copy or conversion. RLP is unaffected -- the wire format follows field order, not the element type name. block_sightings needs the height because block_number leads its sort key. - Record the block -> transaction mapping in block_txs, which was previously thrown away entirely. - Use the devp2p node id, not the enode URL, on both sighting streams. peers and events were previously in different key spaces and could not be joined; they now join on node_id at a 100% match rate. - Add --peer-snapshot-interval (default 30s) for the database write, leaving the 2s tick to drive the Prometheus gauge and local peer file. Persisting up to --max-peers rows every 2s was a large amount of near-duplicate data to answer one question, and reads go through peers_current. - NodeList reads the narrow peers_current rollup instead of grouping over the sighting firehose. - Widen base_fee to UInt256, add the post-Shanghai/Cancun header fields, and record each transaction's chain id, calldata selector and size, access-list, blob and auth-list counts. Also documents both backends' data models and write paths under cmd/p2p/sensor/, linked from the embedded usage text.

Follows the schema rename: block_sightings becomes block_events and tx_sightings becomes tx_events, with the row types (chBlockEvent, chTxEvent) and the event_count column following. The Datastore-era names were already the vocabulary for this data. Also condense the comments throughout, and in the data-model docs annotate self-references (blocks.parent_hash, Datastore's ParentHash and Uncles) on their columns rather than drawing them as relationships -- a self-reference renders as an easily-missed loop, or nothing at all, depending on the mermaid version.

srcHeaderBackfil was missing its second l, and the typos dictionary rejects a bare "mis-" prefix, so reword the comment.

Merge the schema and write-path docs into clickhouse.md and datastore.md. The two halves were always read together -- the write paths only make sense against the table shapes -- and splitting them meant duplicated intros and four-way cross-links. Each file is now schema, then write paths, under one heading tree. Also point at the schema's new home (sensor-network-tools clickhouse/schema.sql) and run prettier over the markdown.

…used fields Follows the schema: peer_snapshots becomes peers, matching the Datastore kind it replaces the way block_events and tx_events already do. Stops writing withdrawals_root, blob_gas_used, excess_blob_gas and parent_beacon_root. Bor sets none of them -- verified absent from mainnet and amoy headers over RPC -- and clique's encodeSigHeader panics if any is non-nil, so they can never take part in the seal hash and nothing can reconstruct a header from them. That also removes the nil-pointer unpacking they needed. mix_digest and nonce stay despite being all-zero on Bor: encodeSigHeader includes them unconditionally, so a consumer re-running ecrecover needs them, as it needs base_fee. Store what the seal hash consumes, not every field types.Header can hold.

size_bytes was the one column this writer put in a fact table that is not a pure function of the hash: block.Size() from WriteBlock, hardcoded 0 from WriteBlockBody. Two sensors that learned the same block by different routes wrote disagreeing rows for one hash, and blocks/block_bodies are ReplacingMergeTree with no version column, so the survivor is arbitrary. Running two concurrent sensors showed the rows collapsing on insert with the 0 winning, discarding the real size. It cannot be fixed by computing the size in the body path: block.Size() is the RLP size of header plus body and that path has no header. So the column goes, and writeBlockBody no longer takes a size. Verified with two sensors and 32 blocks seen by both: blocks, block_bodies, block_txs and transactions hold zero keys with conflicting rows. Duplicate rows still occur and are byte-identical, which is the invariant the schema relies on.

Body arrival was not tracked at all: WriteBlockBody wrote no event and block_bodies has no timestamp, so "which sensor got the body, and when" could not be answered. It now writes a source='body' event. The timestamp goes on the event rather than on block_bodies, which keeps that table content-addressed -- a seen_at column there would recreate the size_bytes bug. Observations are keyed by block number and the eth block body carries neither the hash nor the height, so the height has to be threaded through: c.requests now holds a database.BlockAnnouncement rather than a bare hash, getBlockData takes the announcement, and WriteBlockBody receives it. Both call sites already knew the height -- an announcement carries it, and a parent is header.Number - 1 -- so nothing has to be looked up. Without this the event would be written at block number 0 and mis-keyed. Datastore and JSON ignore the height, as they key events by entity.

sensor_first_seen / sensor_last_seen are one sensor's; plain first_seen / last_seen are across every sensor; first_seen_latency is how far behind the earliest sensor a given sensor was.

new_block, header, header_backfill and body events were gated on shouldWriteBlockEvents, the flag that controls the full per-peer announcement stream. Production runs write_block_events=false with write_first_block_event=true, so all four were silently dropped: no total_difficulty (only new_block carries it), no header timing, and no header_backfill marker -- the replacement for Datastore's IsParent. Only hash_announce survived, because that path is gated at the call site by eventAnnouncements rather than inside the backend. The flag is a volume control and these are not volume. Measured per (block, sensor): hash_announce 52.3 rows, header 7.8, body 1.2, new_block 1, header_backfill 1. The four provenance sources come to 2.8 MB/day across the five mainnet sensors, 0.04 GB at the 14-day TTL, against a hash_announce stream that scales with peer count. They now follow recordsBlockEvents(), which is true when either block-event flag is set. Verified by running a sensor with the exact terraform defaults: all five sources recorded, new_block carrying a real total_difficulty, and the rollup picking it up. Before this, that same configuration produced none of them.

Documents the rule and the resulting growth picture: everything that expires does so at 14 days, so the only unbounded group is the forever tables, dominated by block_txs at ~92 GiB/year.

The data-model doc still said block_events_first "keeps them for 400 days", which stopped being true when retention was standardised on 14 days or forever. It now says what it is: a pre-aggregation for query speed, expiring with its source. The retention table listed 7 of the 13 tables, omitting every rollup and both analysis-job tables. All 13 now, with engines and sort keys, and both verified against a live server rather than by eye -- 13 documented, 13 present, zero mismatches on retention or sort key.

The transaction half of 82c0da8. full_tx -- a delivered transaction body -- was gated on shouldWriteTransactionEvents, the flag that controls the full per-peer announcement stream. Production runs write_tx_events=false with write_first_tx_event=true, so the source was silently dropped and tx_events held only hash_announce, leaving no record of which peer actually delivered a body. write_first_tx_event itself was never broken: WriteTransactionEvents has no internal guard and is gated at the call site by eventHashes, the same asymmetry that let hash_announce survive on the block side. As with the block sources, the flag is a volume control and full_tx is not volume. Measured on a two-sensor stack over ~150k transactions: hash_announce 8.16 rows per transaction per sensor and climbing with --max-peers, full_tx 2.03 because the sensor's LRU filters repeats. Projected at the 14-day TTL for the five mainnet sensors, full_tx is 4.5 GiB. Adds TestClickHouseProductionFlagsRecordProvenance, which drives the terraform default flags and asserts new_block, header and full_tx all land plus total_difficulty round-trips. It fails independently when either recordsTxEvents or recordsBlockEvents is reverted -- the block fix shipped without a test, which is why this recurred. Its transaction nonce varies per run: with a fixed nonce the hash repeats, rows from an earlier run satisfy the assertion, and the first draft passed with the defect reintroduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

clique's encodeSigHeader panics, rather than errors, on any post-Merge trailing header field it does not expect: WithdrawalsHash, ExcessBlobGas, BlobGasUsed, ParentBeaconRoot, and as of go-ethereum v1.17.4 SlotNumber. All are rlp:"optional", so a peer populates them at will. The sensor calls Ecrecover on every header it persists, synchronously on the peer's Protocol.Run goroutine, and geth does not recover around proto.Run. A peer answering GetBlockHeaders with a single header carrying withdrawalsRoot therefore kills the whole sensor. Reproduced: "panic: unexpected withdrawal hash value in clique", stack running through newChBlock. Predates the ClickHouse schema work -- newChBlock on main already calls Ecrecover unconditionally from both WriteBlock and WriteBlockHeaders -- so this is a latent bug being fixed, not a regression. Converts the panic to an error instead of pre-checking the fields. A hand-kept field list would go stale at the next geth bump that adds one, silently, and that is exactly how this reaches production. Ecrecover's other caller, Conns.RecoverSigner, gets the same protection. The test covers all four fields that panic on v1.17.4's predecessors and verifies a well-formed clique header still recovers, so the recover cannot mask a real regression. Confirmed it fails (panics) with the recover removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The batcher context was context.WithCancel(ctx), which inherits the caller's cancellation -- so the comment claiming Close could stop the batchers "independently of the parent context" described a property the code did not have. The sensor shuts down by cancelling its signal context, and stops serving peers only afterwards: stopServer and conns.Close are deferred later than db.Close, so they run first. At SIGINT the batchers therefore drained and exited immediately, while peers kept delivering blocks for the entire server.Stop() window. Those rows went into the buffered channel with no reader -- never flushed, and never counted as dropped, because the channel had capacity. The loss was total and silent. context.WithoutCancel detaches the batchers so Close alone stops them, which is what the comment always claimed. Also drops the stale full_tx comment that a scripted edit failed to replace in the previous commit. The regression test cancels the parent, waits, writes a header, then closes, and asserts the row landed. Verified it fails with WithoutCancel removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

These integration tests write to a real database, which in practice is the local-stack one holding live sensor data. Using heights 42, 4242 and 424242 -- with 4242 shared by two tests -- left rows behind that were indistinguishable from chain data. That is not merely untidy. Two tests writing height 4242 with independently generated signing keys produce different block hashes for one height, and both write the same transaction into block_txs. The result reads as a transaction included in four competing blocks at one height, which is exactly the signature of a reorg. While verifying a reorg-related query fix, that artifact was initially mistaken for real data. Heights now come from named constants in a band far above any real chain head, one per test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Replaces the claim that seen_date partitioning "costs bytes rather than correctness" -- it cost correctness. It is ingest-derived and absent from the dedup keys of transactions and block_txs, and ReplacingMergeTree merges only within a partition, so duplicates were stranded permanently: 10.07% of transactions rows survived a full OPTIMIZE FINAL. Records the constraint that caused it (a transaction fact has no intrinsic timestamp, so its retention clock can never be key-derived), that seen_date is now the version column on both tables, and that dedup happens on merge -- so duplicates are transient rather than absent and a reader that must not double-count still needs FINAL or LIMIT 1 BY. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ttl_only_drop_parts suppresses row-level expiry and drops a part only once all its rows have expired, so a partition coarser than the TTL pins expired rows. Both *_first rollups paired it with monthly partitions against a 14-day TTL, which retained a row from the 1st of a month until the 31st's row expired. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The value reaches the sensor only on a NewBlock announcement and was previously readable only through block_events_first, which expires at 14 days while blocks is kept forever -- so a block's total difficulty silently became 0 once its events aged out, and 0 already meant "no peer ever announced it". WriteBlock now also writes block_total_difficulty, hash-keyed and never written as 0, so absence is what says never-announced. Not gated on shouldWriteBlocks: like the event beside it, it is a fact about the announcement just received. Deliberately not a blocks column. WriteBlockHeaders also writes blocks rows and has no total difficulty, so it would write 0 and a header could clobber a real value -- the same defect that put tx/uncle counts in block_bodies. TestClickHouseTotalDifficultySurvivesEventExpiry deletes every event for a block, standing in for the TTL, and asserts v_blocks still reports the announced value. Confirmed it fails against the old rollup-sourced view: "total difficulty was lost with the events it was announced in". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

signer, coinbase, from_address and to_address were written with common.Address.Hex(), which applies the EIP-55 checksum and returns mixed case. That is a display format -- it exists so a human can spot a mistyped address -- and these values come from ecrecover and RLP, never from typing. Stored mixed case, an address column silently fails to join. ClickHouse string comparison is case-sensitive, and every validator identity in this pipeline is lowercase: the Polygon staking API returns lowercase signers (0 of 105 mixed), data-analysis derives them with hex.EncodeToString, and both it and block-latency test membership directly -- signers[block.Signer] and `if signer in results.validators`. Measured against the live validator set, 0 of 3 real-chain signers in ClickHouse matched as stored and all 3 matched after lower(). The failure mode is the problem. A join returning no rows raises nothing, so "no blocks had a known signer" reads as an answer rather than a bug -- a question like "which validators are we missing blocks from" would name every one of them. All four columns are lowercased rather than only signer, which is the only one joined against external data today. Leaving the other three checksummed keeps the same trap armed on columns that merely have not been joined yet, and leaves one table with two conventions. Hashes need no change: common.Hash.Hex() is already lowercase, having no checksum to apply, as is input_selector. This gives up the EIP-55 checksum, so a corrupted address can no longer be detected from the value alone. That is the right trade for a storage column fed by ecrecover and RLP decoding. TestAddressesAreStoredLowercase covers all four; confirmed it fails independently when signer or to_address is reverted. TestClickHouseWrites asserted the checksummed signer and now expects what the writer stores. Two test-harness fixes came out of writing it. signedHeader returns the lowercase address, matching storage. And signedHeaderWithCoinbase exists because the clique seal covers Coinbase: assigning it to an already-sealed header invalidates the signature silently, and ecrecover then returns an unrelated address -- the first draft of the test did exactly that and asserted against the wrong value. The test transaction is also signed now, since types.Sender yields an empty sender for an unsigned one and the from_address assertion tested nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The third and last instance of the provenance-gating defect, after new_block/body and full_tx. WriteBlockHeaders returned early on !shouldWriteBlocks, before reaching recordsBlockEvents, which made header and header_backfill the only two provenance sources that also required --write-blocks. Headers are requested regardless -- getBlockData has no such gate -- so with block facts off the events were produced and thrown away, silently emptying v_block_provenance's header rows and losing the header_backfill marker that replaced Datastore's IsParent. A blocks row and a block event are separate concerns and are now gated separately, matching WriteBlockBody, which already had this shape: event on recordsBlockEvents, fact rows on shouldWriteBlocks. Audited all five write paths; this was the only remaining mismatch. The test drives write_blocks=false with write_first_block_event=true and asserts both header sources land AND that no blocks row is written -- the second half matters, since it proves the gates were separated rather than one merely widened. Confirmed it fails with the early return restored. It also caught a flaw in the test heights themselves. Deriving the parent as heightHeaderEvents-1 landed on heightLowercase, the previous test's block, so the suite failed on the blocks-count assertion while the test passed in isolation -- the same cross-test collision the reserved band was introduced to prevent. The constants are now spaced by ten so a test can derive height-1 or height+1 safely. Suite verified twice consecutively against one database to confirm it is idempotent over leftover rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Uncle purity: a failed uncle RLP decode logged and continued, writing uncle_count = 0 and uncles = [] while the NewBlock path wrote the true values for the same hash, so the row surviving a merge was arbitrary. That is the defect that removed the size_bytes column, and the transaction decode immediately above already returned for exactly this reason. Now returns rather than writing a partial row. Unreachable on Bor, which produces no uncles, but the shape is the point. Unavailable backend: a failed connection logged once and then discarded every write while the sensor peered, tracked the head and looked healthy -- what a ClickHouse auth failure actually did for an hour across two sensors. It now restates the failure every minute with a count of discarded writes, so a dead backend stays visible in logs and alerting rather than scrolling past at boot. No reconnect: a sensor whose database was down at startup needs a restart, and pretending otherwise would hide that. HasBlock still returns true on a nil connection, to suppress backfill that could never be satisfied, and now says why. Writing the test for that found a hang in the fix itself: the warning goroutine listened on the caller's context while Close cancels only its own, which is never set on the failure path, so Close waited forever on a goroutine nothing could stop -- the same defect as the batchers inheriting the caller's context, hanging shutdown instead of losing rows. TestUnavailableBackendKeepsWarning covers both the warning and the shutdown. CLI help: --write-first-block-event said it "requires --write-block-events=false" and the tx flag likewise. Nothing enforced that, and after enabling the block firehose the fleet ships in precisely the combination the help calls invalid, so an operator running --help on a fleet VM would conclude it is misconfigured. Both now say "ignored when ... is set", which is what the code does. Docs: WriteBlockBody was listed as writing no event when it writes source = 'body'; a mermaid label named a size column that no longer exists; and "Two traps" preceded three bullets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Code: - --peer-snapshot-interval is validated at startup. It fed time.NewTicker raw, which panics on a non-positive interval -- after the p2p ports were bound and the database opened, so 0 was a crash loop rather than a usage error. The sibling --validator-set-refresh already had the check. - block_total_difficulty is gated on shouldWriteBlocks like every other fact row. Ungated, it was the one write that happened with every flag off: forever-kept rows with no blocks row to join to, invisible to v_blocks yet accumulating permanently. The value is also copied at enqueue -- rows sit in the batcher up to a second and td aliased the caller's big.Int, the single exception to an otherwise uniform copy discipline. extraData (aliased peer memory) is cloned for the same reason. - The discarded counter counts what its name says. It incremented on HasBlock -- a read -- and on nothing else, so the dead-backend warning reported rows_discarded=0 forever in exactly the configurations where every write was being dropped (--write-blocks=false makes HasBlock unreachable). Every Write* nil-connection return now adds the rows it is dropping; flags-off returns do not count, being configuration rather than loss. - WriteBlockBody skips exactly what is undecodable, no wider. The uncle-decode fix returned early, discarding transaction facts that had decoded fine and leaving the two failure paths in different partial states (tx failure skipped the body event, uncle failure kept it). The body event is now recorded first -- the body arrived whether or not it decodes -- then each fact is written unless ITS OWN inputs failed: an uncle failure skips only the block_bodies row. - The Ecrecover panic table gains slot_number (EIP-7843, geth v1.17.4) -- the one field the fix's own comment named that its test did not cover. Tests: - The integration tests clean up after themselves. The reserved height band stopped tests colliding with each other but not with the database: fixed tx nonces under fresh per-run sealing keys mapped the same tx hash to a new block hash every run, manufacturing one-transaction-in-N-competing-blocks -- a real reorg signature -- and block_forks (no TTL, no height filter in v_forks) accumulated N-wide "forks" per run, unfilterable and permanent. Nonces now derive from the clock and every block-writing test registers a cleanup that deletes its heights in dependency order. Verified: two consecutive full-suite runs leave zero rows in the test band and zero fork residue. - TestUnavailableBackendKeepsWarning asserts the new counter semantics both ways: HasBlock must NOT count, a dropped block with two transactions must count 3. Docs (round-two drift): - make gen-doc run: the generated CLI doc still carried "requires --write-block-events=false" a full commit after the help text changed. - block_total_difficulty reaches the retention table, the write-path diagram and the batch-size list; the body event edge is drawn; block_txs' erDiagram annotation no longer claims a TTL clock the table does not have; the bucket keys name hash vs block_hash; the Nullable claim points at v_blocks rather than the table; the header_backfill "produced and discarded" claim is corrected (parent backfill is itself gated on --write-blocks, so only header events were produced); and the address-lowercase convention is stated in the writer's own doc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Two remote-input defects found by the third review round, both pre-existing on main and both more severe than anything the earlier rounds fixed. decodeTx logged bytes[0] after rlp.DecodeBytes succeeded. 0x80 is a valid RLP empty string, so DecodeBytes returns an empty slice with no error and the index panicked -- on the peer's own Protocol.Run goroutine, which geth does not recover around, reachable from TransactionsMsg, PooledTransactionsMsg, NewBlockMsg and BlockBodiesMsg. A three-byte message killed the process. This is the same class as the clique panic util.Ecrecover was taught to convert into an error one commit ago; the comment there names the mechanism exactly, and the hole was one call away the whole time. Both log sites now length-check, and the test covers seven hostile encodings. The second is a silent corruption, and it explains why the round-two rework of WriteBlockBody's decode guards was aimed at the wrong layer. decodeTxs is lenient: it drops transactions that will not decode and returns the rest. That is right for a Transactions or PooledTransactions packet, which is a batch of INDEPENDENT transactions. It is wrong for a block body, which the block hash commits to through txRoot: buildBlockBody and the NewBlock path both re-encoded whatever survived, so one garbage blob from a peer produced a body that was not the body for that hash -- block_bodies short by the dropped count, and every later block_txs.tx_index shifted down, because writeBlockTxs indexes by loop position. Both tables are ReplacingMergeTree, so the corrupt row could win the merge against an honest sensor's and persist. Round two guarded the writer's decode errors instead, which cannot fire: buildBlockBody hands the writer already-decoded values. decodeTxsStrict fails the whole body instead, and the NewBlock path -- which carries its own header, unlike BlockBodies -- additionally checks DeriveSha over the transactions against header.TxHash and drops the block on mismatch. Bodies arriving without their header still cannot be verified that way; that residual is stated where the code is. Also from round three: - chBlockEvent no longer aliases the caller's td. It was the last exception to the copy discipline, and the longer-lived of the two aliases -- raw.TD is held in the block cache for ~10 minutes and handed to BroadcastBlock. - The discarded counter counts all 4+2N rows a dropped block represents, not 1+N, and its comment now states which direction it is approximate in and why: the nil-connection check precedes each method's flag gates deliberately, since with no connection the flags are moot, so over-reporting a dead backend is the right way round. The test asserts 8 for a 2-transaction block. - The test cleanup covers the tx-side tables. It had been ordered so that deleting tx_events first emptied the subquery identifying the transactions, and it reached transactions only through tx_events -- but writeTxs is also called from WriteBlockBody, which writes no tx_events, so body-only tests left rows no query could find. Both paths are covered now, ordered so no statement depends on a table an earlier one already emptied. Verified zero residue across blocks, block_forks, tx_events, tx_events_first and transactions after three consecutive full-suite runs; the earlier claim had measured block-side only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

CI runs `shadow ./...` in addition to `go vet`; the inner `err :=` in the buildBlockBody assertion shadowed the outer one and failed the lint target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

92 GiB/year came from dividing block_txs by its row count before its parts had merged, so it counted duplicates awaiting collapse. Measured after OPTIMIZE FINAL: 36.9 B/row on disk, ~91 transactions per block, 47 GiB/year. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Round two added the table, the write-path edge and the retention row but not WriteBlock read-path flowchart edge, nor its Writes cell. The Engine column also showed bare ReplacingMergeTree for block_txs and transactions, contradicting the prose two sections later that says seen_date is the version column on both. Also documents the other NULL guards: only latency_ms was described, though round two added has_body/tx_count/uncle_count, block_time/signer and has_tx. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jhkimqd

+1 lgtm, some findings from Claude to consider taking a look

(Medium — confirm intent) Datastore first-seen mode drops the event for blocks seen first via NewBlock. In the old code, handleNewBlock → WriteBlockHashFirstSeen, and the old Datastore.WriteBlockHashFirstSeen wrote the first-seen block event when --write-first-block-event=true. This branch removed that event-write from Datastore.WriteBlockHashFirstSeen (datastore.go:252), and Datastore.WriteBlock only emits an event when ShouldWriteBlockEvents() (the full flag) is set — not the first-seen flag. So on the Datastore backend with --write-block-events=false --write-first-block-event=true, a block whose NewBlock arrives before any NewBlockHashes (common on Bor, where the sensor is often a direct peer of propagators) gets no first-seen event — later hash announcements continue past it because the block is already fully cached. This is correct for ClickHouse (the target): WriteBlock records a new_block event under recordsBlockEvents(), which honors either flag, and block_events_first derives first-seen. The gap is Datastore-only. If Datastore first-seen mode is deprecated in favor of ClickHouse, this is fine to accept — just worth an explicit "yes, Datastore first-seen is legacy" rather than an accidental regression. (The transaction analog: Datastore.WriteTransactions no longer emits events; tx events now originate at announcement time via WriteTransactionEvents. That's a defensible model change since txs are always announced before being fetched.)

(Nit) Misplaced doc comment in clickhouse.go. The newChBlock doc block (lines 745–753, describing mix_digest/nonce/post-Shanghai handling) is glued with no blank line to the copyBig comment and function, so godoc attributes it to copyBig, and newChBlock (line 781) ends up undocumented. Move the block down to sit above func newChBlock.

(Informational) External schema coupling. The writer targets tables/views (peers_current, block_events_first, v_blocks, block_total_difficulty) whose DDL lives in polygon-infrastructure/sensor-network-tools, not this repo. Column/type drift can't be caught at compile time — only at runtime insert. This is documented in the code, but the schema and this writer must be deployed in lockstep; worth noting in the PR description.

Datastore.WriteBlock only emitted a block event when --write-block-events was set. Under the production config (--write-block-events=false --write-first-block-event=true), a block whose NewBlock arrived before any NewBlockHashes got no event at all -- common on Bor, where the sensor is often a direct peer of a propagator. Later hash announcements don't close the gap: the block is already fully cached, so they're skipped. Gate the event on either flag, as ClickHouse's recordsBlockEvents already does. A whole-block delivery is one event per block per sensor; the flag exists to bound the ~52-per-block hash-announce firehose, which WriteBlockEvents handles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The block sat above copyBig with no blank line, so godoc read it as copyBig's doc and newChBlock had none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

minhd-vu added 3 commits July 29, 2026 14:30

docs(p2p/sensor): use event vocabulary consistently

180713b

minhd-vu force-pushed the minhd-vu/sensor-clickhouse-backend branch from 2524fbd to 180713b Compare July 29, 2026 18:32

minhd-vu changed the title ~~feat(p2p/sensor): ClickHouse backend for the sensor~~ feat(p2p/sensor): rewrite the ClickHouse writer for the redesigned schema Jul 29, 2026

minhd-vu and others added 20 commits July 29, 2026 14:40

fix(p2p/sensor): correct typos flagged by CI

4bd55c5

srcHeaderBackfil was missing its second l, and the typos dictionary rejects a bare "mis-" prefix, so reword the comment.

docs(p2p/sensor): document the timestamp naming rule

49bfc79

sensor_first_seen / sensor_last_seen are one sensor's; plain first_seen / last_seen are across every sensor; first_seen_latency is how far behind the earliest sensor a given sensor was.

docs(p2p/sensor): first_seen_latency_ms carries its unit

6c0a52c

docs(p2p/sensor): retention is 14 days or forever

707530d

Documents the rule and the resulting growth picture: everything that expires does so at 14 days, so the only unbounded group is the forever tables, dominated by block_txs at ~92 GiB/year.

minhd-vu force-pushed the minhd-vu/sensor-clickhouse-backend branch from 5bb64e7 to 1423f7d Compare July 30, 2026 14:48

minhd-vu and others added 4 commits July 30, 2026 14:09

fix(p2p): rename a shadowed err in the block-body test

0d4fdd9

CI runs `shadow ./...` in addition to `go vet`; the inner `err :=` in the buildBlockBody assertion shadowed the outer one and failed the lint target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

minhd-vu marked this pull request as ready for review July 30, 2026 21:10

jhkimqd previously approved these changes Jul 31, 2026

View reviewed changes

minhd-vu and others added 2 commits July 31, 2026 01:12

docs(p2p/sensor): attach the newChBlock comment to newChBlock

0465bf0

The block sat above copyBig with no blank line, so godoc read it as copyBig's doc and newChBlock had none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

minhd-vu dismissed jhkimqd’s stale review via 0465bf0 July 31, 2026 05:13

jhkimqd approved these changes Jul 31, 2026

View reviewed changes

minhd-vu merged commit 26b511b into main Jul 31, 2026
15 checks passed

minhd-vu deleted the minhd-vu/sensor-clickhouse-backend branch July 31, 2026 06:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(p2p/sensor): rewrite the ClickHouse writer for the redesigned schema - #975

feat(p2p/sensor): rewrite the ClickHouse writer for the redesigned schema#975
minhd-vu merged 30 commits into
mainfrom
minhd-vu/sensor-clickhouse-backend

minhd-vu commented Jul 29, 2026 •

edited

Loading

Uh oh!

jhkimqd left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

minhd-vu commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

The design

The defect that motivated it

Other changes

Docs

Jira / Linear Tickets

Testing

Notes

Uh oh!

jhkimqd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

minhd-vu commented Jul 29, 2026 •

edited

Loading