[DO-NOT-MERGE] Leios prototype#1793
Draft
bladyjoker wants to merge 1539 commits into
Draft
Conversation
ch1bo
force-pushed
the
leios-prototype
branch
from
ed63794 to
9ea774a
Compare
ch1bo
force-pushed
the
leios-prototype
branch
2 times, most recently
from
a123a84 to
a571776
Compare
20 tasks
20 tasks
BEGIN IMMEDIATE results in directly locking the transaction for write access, while BEGIN does defer it to the first write statement. Thus, if we never do a write, the connection does not get blocked by other connections and reduces lock contention.
The sql query was very inefficient and also the completion tracking can't afford to check hashes. Keeping a counter in the database, written to within the same transaction seemed the best way forward.
BEGIN IMMEDIATE results in directly locking the transaction for write access, while BEGIN does defer it to the first write statement. Thus, if we never do a write, the connection does not get blocked by other connections and reduces lock contention. The sql query was very inefficient and also the completion tracking can't afford to check hashes. Keeping a counter in the database, written to within the same transaction seemed the best way forward.
Resolves input-output-hk/ouroboros-leios#837 DONE - [x] Move the EB announcement from the Ledger Block to the Praos Header - [x] Add `EbAnnouncement` type that holds an `EbHash` and its size! - [x] Remove the `certifies` field - [x] Move the LeiosState from the Ledger to the ChainDepState - [x] Adds 'cummulative' EB size tracking in the state (TODO: Grafana this) - [ ] ~~Move the Certificate from the Ledger into Consensus~~ - Ugh for now I'm keeping it in the Ledger because it's easier, albeit not ideal - [x] Update to Ledger with backward compatible Block codec - [x] Introduces `ForgeBlockArgs` for my own sanity - [x] FIX: [EB only diffused if announced in forge](85aafd4) NOTES - `ResolveLeiosBlock` machinery is a hack that works! We're blindly guessing that `blk` might contain something that can be resolved like a LeiosCertificate and it returns a `blk` that hopefully has a fully resolved `blk` that can be applied. Imo a morally correct approach would to have `data RankingBlock blk = LedgerRb blk | CertRb LeiosCertificate` which we can use to make such distinction and manage the resolution process here and elsewhere in the abstract code base where that distinction matters (like the ForgeLoop) - `applyBlock` currently does `resolveLeiosBlock` and then `tickAndReapply` which is not incorrect as in the case of there being a Leios Cert, we need to base changes from the associated EB on top of the `prev` Ledger State (I think that means we shouldn't tick?) - see input-output-hk/ouroboros-leios#857 - Apropos `tx-centrifuge` it doesn't seem like there's any way around breaking the API `BlockFetch`. For the current pragmatic solutions we hide the `Certificate` behind the `blk` and therefore `BlockFetch` will happily return you a Certificate which will break `tx-centrifuge`. That being said, `ChainSync` is also necessarily having a breaking change since we added new fields to the PraosHeader.
This should avoid most of the unnecessary re-writing from the forge loop.
The synchronous wait in the vote loop was resulting in more votes to fail than necessary. Also, the forge loop was taking several seconds when writing EB closures to disk. This is now mostly avoided by not re-writes txs we already have. A nice summary table by Claude on the storage improvements: ``` ┌─────────────────────────────────────┬─────────────────────────────────────────────────────────┬──────────────────────────────┐ │ Metric │ Previous run (same tx counts) │ After fix │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ Forge span (F→A) for ~4400-tx EB │ 1.82 s │ 0.17 s │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ Forge span for ~4000-tx EBs (range) │ 1.4–1.8 s │ 0.16–0.34 s │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ Forge span for ~1500-tx EBs │ ~1.4 s │ 0.07 s │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ LeiosNotVoted (node1, 28 min) │ 6–10 (typical, incl. tooLate & chainTipDoesNotAnnounce) │ 0 │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ LeiosCertified (unique) │ ~15–20 per 28 min │ 23 │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ LeiosBlockCertified (node1) │ ~4 per 28 min │ 5 │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ Cert inclusion delays (slots) │ 15–34 │ 17–69 (median 25) │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ Mempool ingest (tx/s) │ ~55 │ ~46 (matches 20 TxkB/s load) │ ├─────────────────────────────────────┼─────────────────────────────────────────────────────────┼──────────────────────────────┤ │ mempoolRestSize at each forge │ 0 │ 0 │ └─────────────────────────────────────┴─────────────────────────────────────────────────────────┴──────────────────────────────┘ ```
Should follow the TODOs right away next time! There are still a few places in tests and tools where connections are not correctly cleaned up, but good enough for the cardano-node now.
Should follow the TODOs right away next time!
…ension - SQLite.hs: thread-check assertion catches cross-thread connection use - LedgerDB V1/V2: store LeiosDbHandle instead of a live LeiosDbConnection; every user brackets its own connection via 'withLeiosDb' on its own thread - LeiosDemoLogic + NodeToNode: TQueue plumbing for pipelined-peer collector responses (msgLeiosBlock work now defers to the main peer thread rather than executing on the collector) - leios-db-bench: add RMW client, chain-sel reader and GC ticker roles to mirror the full production access shape Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop dead reads: leiosDbLookupEbPoint (never used outside stale tests), leiosDbQueryFetchWork + LeiosFetchWork (obsoleted by the incremental filter- api via leiosDbFilterMissingEbBodies/leiosDbFilterMissingTxs). - Rename leiosDbQueryCompletedEbByHash → leiosDbLookupEbClosure to line up with the leiosDbLookupEbBody / leiosDbInsertEbBody naming and to clarify body ≠ closure (body = txhash + size, closure = body + tx bytes). - Simplify msgLeiosBlock: no more read-then-conditional-insert on the EB point. The insert becomes unconditional (idempotent) with a FIXME to turn it into an assertion once announcement handling guarantees the point is already present. The point-insert-on-body-receive is a stop-gap, not the target design. - Drop the RMW client role from leios-db-bench (redundant after the msgLeiosBlock simplification) and rewire the chain-sel reader to leiosDbLookupEbClosure. Bench delta (post-DRY vs pre-DRY extended, +RTS -N4, 5 runs): avg 430 ms → 308 ms (-28%) The gain is not from the query surface itself but from removing the two RMW writer threads that were contending on the writer serialiser. Test suite touched: drop the LookupEbPoint/QueryFetchWork property groups; rename the queryCompletedEbByPoint group to lookupEbClosure. All 48 remaining LeiosDemoDb properties pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrote sqlBatchRetrieveTxs, sqlFilterMissingEbBodies and
sqlFilterMissingTxs to pass their batch inputs via json_each(?) instead
of INSERTing them into per-connection mem.* temp tables. Dropped the
ATTACH DATABASE ':memory:' from the open path and all
sql_{insert,filter,flush}_mem* SQL strings.
Payload encoding:
- BLOB inputs (ebHash, txHash) travel as a JSON array of hex strings;
SQL decodes with unhex(je.value) so index lookups on the target
BLOB columns still fire.
- Int inputs (tx offsets) travel as a JSON int array, consumed by
json_each directly.
The JSON is bound as TEXT via a new dbBindUtf8 helper — SQLite ≥ 3.45
interprets BLOB-bound arguments to json_each() as JSONB and rejects the
plain-JSON payload.
Bench delta on the 5-role concurrent workload (+RTS -N4, 5 runs each):
Size Baseline mem.* JSON1
LIGHT 302 ms avg 200 ms avg -34%
MEDIUM 599 ms avg 366 ms avg -39%
HEAVY 1270 ms avg 772 ms avg -39%
Run-to-run variance also collapsed (LIGHT was 273→350 across runs 1→5,
now 193→210): the removed temp-table INSERT/DELETE dance was thrashing
the WAL every batch operation, which no longer happens.
All 48 LeiosDemoDb properties still pass on both InMemory and SQLite.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sqlInsertTxs was preparing @stmtInsert@ and @stmtDecr@ with bare 'dbPrepare' inside a 'dbWithBEGIN', then finalising them after the 'forM_' loop. If any call in the loop threw (e.g. 'dbStepInsert' exhausting its busy-retry budget, or 'dbStep1'/'dbReset' erroring on the decrement), the outer generalBracket would run ROLLBACK but the two prepared statements would leak — the connection would be closed later with dangling VDBEs, whose auto-finalisation runs 'sqlite3VdbeHalt' on the shutdown path. Wrap both statements in nested 'dbWithPrepare' brackets so the finalise runs before the BEGIN's cleanup on every exit path. Not proven to be the cause of the observed 'sqlite3VdbeHalt' → SIGSEGV after ~3-4h in the proto-devnet, but it's the one place the statement lifetime was not exception-safe; closing that hole is cheap and correct on its own. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The proto-devnet keeps segfaulting inside 'sqlite3_finalize' → 'pthread_mutex_lock'. Coredump analysis showed the fault pointer is 'db->mutex = 0x4b' on the sqlite3 connection struct — the struct's memory has been freed and reused for something else while a 'Vdbe' statement still references it. Classic use-after-free. Our existing thread-check safety net only guards facade entry against cross-thread misuse; it can't see when the underlying C struct is corrupted. Add a temporary peek right after the thread check: read the 'sqlite3' struct's '.mutex' field (offset +24 for SQLite 3.45.0 with our compile flags) and refuse to proceed if it doesn't look like a pointer. Under 'SQLITE_THREADSAFE=1' (SERIALIZED), this field is always a non-NULL pointer while the connection is alive, so any small integer here means we're about to segfault. The peek costs a single 8-byte load per facade call; negligible. Marked TEMP: to be removed once we've observed either a clean crash trace (Haskell exception with call stack) or a stretch of uptime with no assertion firings. Layout offsets pinned from the specific proto-devnet coredump; will not be portable to other SQLite versions or compile-time configurations without rechecking. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rework 'openSQLiteConnection' to prepare every SQL statement the
connection needs at open time and finalise them all deterministically
at close time (immediately before 'sqlite3_close_v2'). Store the
statement handles in a per-connection 'Stmts' record; each 'sql*'
function looks up its statement, uses it via a small 'useStmt'
helper (bind → step → reset), and never prepares or finalises.
Rationale: the observed proto-devnet segfault (see previous commit's
canary rationale) is a use-after-free of the 'sqlite3' connection
struct while a 'Vdbe' still references it. Under the previous
per-op 'dbWithPrepare' pattern, every SQL call created and finalised
its own statement — so 'sqlite3_finalize' was reachable from many
paths, and any code that could observe a stale 'db' pointer had a
path to segfault via mutex-lock.
With this change:
* All statements are created inside 'openSQLiteConnection' before
the connection is exposed to callers.
* All statements are finalised inside 'close' immediately before
the connection is closed, on the same thread that owns the
connection.
* No code path outside 'close' ever calls 'sqlite3_finalize'.
That narrows the finalize surface to a single call site, removing
the specific reproduction path we saw. It doesn't rule out a
different corrupter (which is why the previous commit adds a canary
peek), but it eliminates the most common shape of "stmt outlived
its conn".
Performance side effect: preparing 13 statements once per connection
saves the recurring per-op prepare cost. Bench measurements in
'analysis-runs/bench-baseline.txt' confirm no regression.
All 48 LeiosDemoDb properties still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the two TEMP asserts that guarded the pre-prepared-statements
refactor while it was under load in the proto-devnet:
* The per-facade-entry thread-check safety net (fail loudly if a
'LeiosDbConnection' is used from a thread other than the one that
opened it). We've had enough uptime on the pre-prepared build
(node2 running > 8h at commit time, past the previous typical
crash window) with no observed cross-thread misuse to justify
removing the runtime check.
* The 'checkSqliteIntegrity' canary that peeked into the sqlite3
conn struct's '.mutex' field on every facade entry. It never
fired on any node (including the canary-only build that still
crashed), so it wasn't observing what we hoped anyway.
The pre-prepared-statements design is what actually stopped the
segfault: every SQL statement is prepared at 'openSQLiteConnection'
and finalised deterministically inside 'close', on the owner
thread. No 'sqlite3_finalize' call reaches user code from anywhere
else, so the use-after-free class of crash we saw is structurally
prevented.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
'resolveLeiosClosure' previously returned an empty tx list when 'leiosDbLookupEbClosure' didn't find the EB in the local LeiosDb. That silently let a cert-RB apply as if its EB carried no transactions, which diverges the UTxO from the honest chain: any downstream RB consuming an output produced by the missing EB then fails validation with 'ValueNotConservedUTxO' or 'BadInputsUTxO' — and the fork stays stuck without an obvious cause. Under the intended parking design, chain-sel would not have selected this block yet — a cert-RB should be parked pending closure acquisition. This is a diagnostic aid to surface the exact block/EB pair whenever the parking invariant is violated, so we can chase the actual bug instead of the downstream ledger error. Marked TEMP/FIXME in the code because it should be removed once the parking path is verified end-to-end — at that point the invariant will be maintained upstream and reaching this branch would be a consensus bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The DRY refactor (81fcad9) collapsed `resolveLeiosBlockHdr` — which gated on the block's own certificate (`leiosCertBlockBodyL`) — into `resolveLeiosClosure`, which ignores the block. The NTC serve path then inlined the previously-announced EB into every block regardless of whether it carried a certificate, so it resolved announced-but-uncertified EBs. When such an EB's closure is absent from the LeiosDb, `resolveLeiosClosure` throws and terminates the NTC block-serving mini-protocol; when present, it inlines non-chain-content txs that clients (e.g. db-sync) record as real, producing duplicate transactions. Restore the gate in the server using the existing `headerContainsLeiosCert` method: only a CertRB splices in its (predecessor-announced) EB closure; any block without a certificate is served unchanged. Change is confined to the ChainSync Server module.
It's ultimate Set EbHash, except it can be GC'd according to the SlotNo's that caused the EbHash to be in the set.
But the LeiosFetch logic didn't know that, so it eventually thought too many bytes were inflight even though they weren't, so it stopped sending any new requests.
The DRY refactor (81fcad9) collapsed `resolveLeiosBlockHdr` — which gated on the block's own certificate (`leiosCertBlockBodyL`) — into `resolveLeiosClosure`, which ignores the block. The NTC serve path then inlined the previously-announced EB into every block regardless of whether it carried a certificate, so it resolved announced-but-uncertified EBs. When such an EB's closure is absent from the LeiosDb, `resolveLeiosClosure` throws and terminates the NTC block-serving mini-protocol; when present, it inlines non-chain-content txs that clients (e.g. db-sync) record as real, producing duplicate transactions. Restore the gate in the server using the existing `headerContainsLeiosCert` method: only a CertRB splices in its (predecessor-announced) EB closure; any block without a certificate is served unchanged. Change is confined to the ChainSync Server module.
…playing This was causing the node to replay its immutable chain incorrectly on restart. It would either crash because an _uncertified_ EB was absent, OR WORSE, it'd happen to find the EB and apply it _instead of the announcing RB_ even though it wasn't certified! The fix is to not call resolveLeiosBlock in a couple places. I didn't already fix it everywhere (eg LMDB, eg some test code) for now, but I added a TODO on those calls.
Even 100 headers would be small, so this is still safe. My local node is receiving votes in very tight bursts, less than one RTT. Having the pipelining depth so shallow risks some votes (or announcements, or offers, etc!) not being relayed. This wasn't (as much of) a risk before since we only have a few committee members. But now we have many.
…ection Fixes Issue input-output-hk/ouroboros-leios#983, a race condition on startup of a completely fresh node that can crash (subsequent restart is fine).
… of Leios node (NB targets PR 2114) (#2115) Fixes Issue input-output-hk/ouroboros-leios#983. Uses the DYNAMIC option explained on that Issue's comment.
… LeiosNotify pipelining depth) (#2114) Another bug I discovered during my time with the Leios testnet in the past couple days.
- [cardano-node](IntersectMBO/cardano-node#6614) containing these fixes
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
6 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.
This PR hosts changes to ouroboros-consensus related to Leios. I'm opening a PR for easier management and transparency to make changes visible at a glance.
More importantly, this branch will be tagged and sourced for Leios demos.
IT IS NOT INTENDED TO BE REVIEWED OR MERGED!