Skip to content

Leios prototype: EB validation in vote logic - #2235

Merged
ch1bo merged 16 commits into
nfrisby/leios-first-LeiosFetch-incrementfrom
ch1bo/leios-eb-validation
Aug 29, 2026
Merged

Leios prototype: EB validation in vote logic#2235
ch1bo merged 16 commits into
nfrisby/leios-first-LeiosFetch-incrementfrom
ch1bo/leios-eb-validation

Conversation

@ch1bo

@ch1bo ch1bo commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Resolves and validates EB closures in runLeiosVoting.

This is using the TxCache from #2188 to determine whether we need to applyTx or reapplyTx via the existing LedgerSupportsMempool API.

As this change is heavily restructuring the Leios voting loop, I realized that we incorrectly used the currentSlot to determine whether voting timed out -> this is wrong! The current slot (derived via ChainDB.getCurrentLedger) is only ever advancing when the tip advances. Thus, the "time" was lagging quite a lot. Changed the semantics determine onset and deadline of voting period using the wall clock (via slotToWallclock).

@ch1bo ch1bo linked an issue Aug 25, 2026 that may be closed by this pull request
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch 5 times, most recently from f62c00e to 66045e7 Compare August 25, 2026 19:46
@ch1bo
ch1bo changed the base branch from nfrisby/leios-first-txcache-increment to nfrisby/leios-first-LeiosFetch-increment August 25, 2026 19:55
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch 2 times, most recently from b9c5e42 to 221836f Compare August 25, 2026 20:50

@nfrisby nfrisby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

First pass, which excluded two modules: ThreadNet tests and LeiosVoting itself. (That's as far as I got before my dev code finished compiling :D)

--
-- NOTE: the cache's tag does not record which protocol version the tx was
-- validated under, so an EB straddling a protocol-version change could skip
-- a static check whose rules changed. The mempool has the same exposure, and

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The mempool has the same exposure, and the cache only retains a short window of EBs.

Neither of those justifies tolerating this. In particular, a bad tx in the Mempool doesn't lead to a bad tx on chain (RB validation will reject the block). But voting for a bad tx in an EB does.

My initial instinct is that we should change the CertRB validity rules to be invalid if they're attempting to certify an EB from the previous era. (In other words, the first block of an era must not be a CertRB.)

@bladyjoker bladyjoker Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did we finally arrive to the dreaded topic of cross-era concerns x) Let's use this to dig a bit through this...

What does it mean to certify an EB that from the previous era? Which rules should be applied to that EB? Original era, or the new one?

My take is; We're never actually in the new era when certifying!

To cross an era we must tick right? But we verifyLeiosCert and applyLeiosClosure before ticking!

So in effect, we're always applying the rules to EBs and Certs of a single era (that they originated in). In other words, no such thing as cross-era EB/Cert.

Am I missing something?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After discussing in Slack, we do tickThenApply that invokes BBODY Ledger rule AFTER TICK which means in the new ERA. I find that to be an implementation problem, and we should instead:

  1. verifyLeiosCert and invoke a special Ledger BCertBody
  2. applyLeiosClosure
  3. tick

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I, of course, have only worked and reviewed the code .. not the comments that the 🧞 produced while we paired. I do agree with that we don't want to allow certification across era boundaries. I would even say we must not certify across epoch boundaries -> so protocol parameters can be assumed static too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bladyjoker not taking the bait because I truly believe it's not worth it to worry about cross era / epoch certification at this point.

Comment thread ouroboros-consensus/test/consensus-test/Test/LeiosVoting.hs Outdated
Comment thread ouroboros-consensus/test/consensus-test/Test/LeiosVoting.hs

@nfrisby nfrisby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm Requesting Changes primarily for the commit message and comments that libel against BlockchainTime :P

I've also made some suggestions that seem quite preferable (eg only acquiring the LeiosTxCache log twice per EB instead of thousands of times, a few helpful explanatory code comments, etc)

Comment thread ouroboros-consensus-cardano/test/cardano-test/Test/ThreadNet/Leios.hs Outdated
Comment thread ouroboros-consensus-cardano/test/cardano-test/Test/ThreadNet/Leios.hs Outdated
Comment thread ouroboros-consensus-cardano/test/cardano-test/Test/ThreadNet/Leios.hs Outdated
Comment thread ouroboros-consensus-cardano/test/cardano-test/Test/ThreadNet/Leios.hs Outdated
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
Added weight mCert -> do
traceWith tracer TraceLeiosVoted{vote, weight}
traceWith tracer TraceLeiosVoteAcquired{vote}
-- Trace certification whenever the tally crosses

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • I'd be surprised if the log-writer deduped. So that seems like we'd rather have addVote track exclude the cert/set a flag/etc when first emitting.
  • This comment says it fires "whenever the tally crosses ". Which can only happen once. But then it says it may fire again. So either it is already deduped or the "when it crosses" is the wrong wording (crossings only happen once, right? Unless concurrency is the only cause of dupes?).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The mCert is only set when we first certify. I don't understand your comment / what are you wanting me to change?

Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch from 221836f to c0d0e62 Compare August 26, 2026 11:05

@bladyjoker bladyjoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Heyo,

I'd say the most salient comment is about the the model of Voting thread.

One point I think holds pretty well is that it should be driven by "selection change" events, just like the Mempool (not on AcquiredEbTxs).

The thread then waits for the status of the associated EB announcement to be "closure acquired" and proceeds to vote. This part should be cancellable due to either deadline or selection change.

I think this even removes the need for timers maybe? Could be simplified?

Another missing part is that we probably don't have the ebAnnouncementStatusVar :: TVar (Map LeiosPoint AnnouncementStatus) just laying around. However, we do have all the events we need to make that happen in subscribeEbNotifications, a thread that monitors these notifications and maintains the ebAnnouncementStatusVar var sounds doable.

Wdyt?

mconcat
[ "kind" .= Aeson.String "LeiosVoteScheduled"
, "ebHash" .= prettyEbHash ebHash
, "ebSlot" .= ebSlot

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we still carrying the slot for? This should be RbHash.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to drop the LeiosPoint / eb tracing or add the RbHash or do both?

mconcat
[ "kind" .= Aeson.String "LeiosEbValidated"
, "ebHash" .= prettyEbHash ebHash
, "ebSlot" .= ebSlot

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as before, this should be RbHash

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remember in case of forks that have the same EB announced at the same slot this becomes ambiguous.

Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosDemoTypes.hs
Comment on lines +173 to +175
{ scheduleVoteTime = \point -> do
extLedger <- atomically $ ChainDB.getCurrentLedger chainDB
case slotOnset lcfg (ledgerState extLedger) (pointSlotNo point) of

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is bothering me, what's the relation between extLedger and point?

If votes are cast per announcement on our currently selected ledger state, then these two can differ here in a way that's suspicious.

For the sake of argument,

  • extLedger contains the PraosState that has an associated RbHash, Slot and EbAnnouncement.
  • point has a Slot and EbHash

This method doesn't assure that current extLedger is carrying the EbAnnouncement for which we're voting.

In fact, if we keep it this way, if extLedger changed because our selection changed, we shouldn't proceed with voting right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is not a LeiosPoint, but a Point. We do check the announcement when it's time to vote - selection could change in between! See the ChainTipDoesNotAnnounce case

Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs
let keys = foldMap (getTransactionKeySets . snd) closure
values <- resolveValues keys
-- Determine which txs we can just reapply (the cache hits)
decided <- withLookupTx txCache $ \look -> mapM (decide look) closure

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't look how this works, but I'd be surprised if this does batched lookups? This is ideally a Set.intersection txCache closureTxHashes.

Currently, we're O(length closure * O(look)) which I assumes is at best O(length closure * O(log (length txCache))).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm using what @nfrisby provides :)

RelativeTime ->
m (Either LeiosNotVotedReason ())
goVote leiosConn sk point deadline = do
let vk = deriveVerKeyDSIGN sk

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bind vk once outside and pass it as argument

@ch1bo ch1bo Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No. That would then be a "bigger than necessary signature" and the two values could drift -> I would need to check whether the vk matches the sk

forever $
atomically ((Left <$> waitAcquiredEbTxs) <|> (Right <$> waitNextVoteTime))
>>= \case
Left point -> scheduleVoteTime point

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this, why are we scheduling a vote for every acquired EB?

Let me take a stab at the logic of this Voting thread...

In my view, Voting thread should subscribe on new selections (just like the Mempool) and that should be the main driver.

It then proceeds to relate EB announcements and their status to the current selection

  1. Check if there's an EB announcement for the current praos/ledger state
    2. If we continue using Praos headers for announcement then check the PraosState praosStateLeiosAnnouncement
    3. If we use the LeiosNotify for EB announcements, then look it up there
  2. If we found an EB announcement, check it's status
    3. "waiting on EB body" -> " EB body acquired" -> "EB closure acquired"
  3. Wait on the EB announcement status to reach "EB closure acquired"
  4. (go)Vote if we're not passed the deadline

This entire procedure must be interruptible to avoid wasting resources:

  1. Deadline is exceeded
  2. New selection comes about

@ch1bo ch1bo Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is multiple options in how to structure the voting loop. I originally picked the form of being driven by EB announcements / their acquisition and only check whether it matches our selection at the time of voting (this is important as it can change while we wait for the voting period). I didn't change it in this PR (only refactored the semantics). Feel free to rewrite to the semantics you suggest and see if it is more clear. I certainly won't do it in this PR.

This entire procedure must be interruptible to avoid wasting resources:

I agree that this is the challenge it's not done well right now

Comment on lines +323 to +325
now <- lift $ systemTimeCurrent systemTime
when (now > deadline) $
throwE TooLate

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd rather see a thread cancelled due to a timeout then this check.

Why? Because this check could be added at any point in this function, actually, it makes sense to add it everywhere to stop ASAP. That just tells me that we should use timeout and cancel thread execution when the deadline is hit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah the current logic is more synchronous than it needs to be. The same is true for the forge loop and other aspects of the system. Synchronous is often easier to write and read :)

Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch from c0d0e62 to 7b34aa8 Compare August 28, 2026 08:59
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch 2 times, most recently from 79ebf9d to fcfd9f0 Compare August 28, 2026 12:31
@ch1bo
ch1bo requested review from bladyjoker and nfrisby August 28, 2026 12:32
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch 2 times, most recently from e84548d to a51c7b6 Compare August 28, 2026 13:55
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch from a51c7b6 to 9d45eff Compare August 28, 2026 19:06

@dnadales dnadales left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Full disclosure, these concerns were raised by Claude. I filtered them, and I'm posting here the ones that seemed to make sense.

Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosVoting.hs Outdated
Comment thread ouroboros-consensus/test/consensus-test/Test/LeiosVoting.hs Outdated
Comment thread ouroboros-consensus/test/consensus-test/Test/LeiosVoting.hs
Comment thread ouroboros-consensus/src/ouroboros-consensus/LeiosDemoTypes.hs Outdated
Comment thread ouroboros-consensus-cardano/test/cardano-test/Test/ThreadNet/Leios.hs Outdated
@@ -14,60 +15,99 @@ module LeiosVoting

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(This is really about the export list just above, LeiosVoting.hs:11-14, which isn't in the diff so I can't anchor there.)

Should we name what we want to export?

module LeiosVoting publishes everything, and the module is in exposed-modules (ouroboros-consensus.cabal:123), so this PR newly makes VoteTimers(..), newVoteTimers, slotOnset, and the (?>=) helper at line 489 part of the library's API. validateEbClosure and EbClosureVerdict(..) we do want, since Test.LeiosVoting imports them.

The wildcard predates this PR, so this isn't really something the PR introduced. But (?>=) is a very general name to be handing out from a module about voting, and it's new here. Would an explicit list be worth doing while we're in the file? Something like:

module LeiosVoting
  ( runLeiosVoting
  , validateEbClosure
  , EbClosureVerdict (..)
  , HasLeiosVoting (..)
  ) where

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't really like explicit export lists for applications (We are implementing the voting logic for a node here, not a generally reusable library)

Is this a Must, Should or Could comment?

ch1bo added a commit that referenced this pull request Aug 29, 2026
Voting applied each tx with 'DoNotIntervene', which is the mempool's
be-generous-to-remote-peers setting. For Alonzo and later that is not a
courtesy, it rewrites the transaction: applyAlonzoBasedTx forces isValid
true, and on a ValidationTagMismatch retries with it false. So a Plutus tx
declaring isValid true whose scripts fail validated here as a
collateral-only tx, and we voted for the EB carrying it.

The apply path does none of that. applyLeiosClosure runs the LEDGER rule
with ValidateNone, script evaluation sits inside when2Phase and is skipped,
and the UTxO effect branches on the declared flag -- so the chain consumes
the tx's whole input set. The transaction we validated and the transaction
the chain applies were not the same one, which is precisely the trust
applyLeiosClosure cites the certificate for.

'Intervene' leaves the flag alone and lets the mismatch escape as an error,
so we now fail closed on exactly the txs the apply path would mishandle.

Reported by @dnadales in review of #2235.
ch1bo added a commit that referenced this pull request Aug 29, 2026
All from @dnadales's review of #2235.

Fail a vote rather than the node. addVote returning anything but Added was
an error call, which under forkLinkedThread takes the node down. This PR
made it reachable: seatId is resolved from the forker, addVote re-reads the
committee from the current selection, and closure validation now sits
between the two, so an epoch boundary crossed in that gap silently makes the
positional LeiosSeatId someone else's seat. It is now a VoteRejected reason,
traced like the others.

That is containment, not a fix, and the TODO beside it says so: signing
first and finding out afterwards is the wrong shape. We should establish
that we are still on the same committee, and still hold that seat, before
casting the vote.

Check the deadline before validating, not only after. scheduleVoteTime arms
on acquisition and AcquiredEbTxs only fires when the last tx of a closure
lands, so a closure completing late gives a negative delay and the timer
fires already expired. We were spending ~1.5s applying a closure for a vote
that was dead before the timer existed. The check after validation stays:
it is the one that judges the window by the clock at signing time.

Commit every notification read. Looping past AcquiredEb inside the STM
transaction meant a run ending in retry discarded the read-end write, so
skipped messages came back to be skipped again on the next wake. It now
returns Maybe and the caller ignores Nothing.

Also: say what scheduleVoteTime actually does with an already-open window
now that the max 1 clamp is gone, and unescape a haddock pipe.
ch1bo added a commit that referenced this pull request Aug 29, 2026
Both from @dnadales's review of #2235.

The harness answered every table read with the whole initial UTxO, ignoring
the keys it was asked for. Since applyMempoolDiffs restricts the diffs
rather than the values, the key sets could have been empty and all four
properties would still have passed -- confirmed by blinding them, which now
fails three. Restricting the reply also gives the transitive-closure TODO in
validateEbClosure something to fail against once we get to it.

And prop_stopsAtFirstFailure claimed to cover a tx after the failure while
the generator put the failing tx last, so abandon skipping the rest of the
closure was never exercised. The generator now emits a tail that would have
applied if reached, and the property asserts it stays untagged.
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch from 9d45eff to 63891a3 Compare August 29, 2026 13:30
ch1bo and others added 16 commits August 29, 2026 15:31
'resolveLeiosClosure' returns [(TxHash, GenTx blk)] instead of [GenTx blk].
'leiosDbLookupEbClosure' already reads the hash out of the LeiosDb and the
Dijkstra instance was throwing it away; the voting thread is about to need it
to key the LeiosTxCache.

Callers that only want the transactions drop it with 'map snd'. No behaviour
change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebuilds the 'Validated' token for a closure tx, so a caller holding evidence
that the tx was already validated can pick 'reapplyTx' (which skips only the
static checks) over a full 'applyTx'.

The production LeiosTxCache cannot hand back such a token itself: its table
packs a refcount and a three-valued tag into a Word64, so 'lookupTx' yields
'Maybe (Either () ())' and nothing more. The tag is the evidence; the tx we
already resolved is the payload. For Dijkstra that makes rebuilding it a
tx-id hash, and 'SL.reapplyTx' derives the state-dependent annotation from
the state at hand, so nothing stale rides along.

No caller yet -- the voting thread picks it up two commits from here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sed tx

The voting thread signs a vote for every EB whose closure it acquires without
ever looking at the transactions (ouroboros-leios#1016), so a committee member
will happily certify an EB full of invalid txs. This is the test that says so.

'prop_leios_invalid_eb' runs 4 nodes with node 1 adversarial: it appends a tx
spending a phantom TxIn to 'fbEbTxs'. Only the EBs it announces are poisoned --
'fbRbTxs' is untouched, so its RBs stay valid, the chain keeps growing, and
honest nodes' own EBs still certify.

To let a test install a misbehaving node, 'runThreadNet'' takes a per-node tweak
of 'TestNodeInitialization' and 'runThreadNet' passes the identity. The tweak
ranges over the whole 'TestNodeInitialization' rather than just its
'BlockForging', so the next test that needs a misbehaving node can reach the
protocol info or the crucial txs without changing this plumbing again.

The bogus tx fails on a state-dependent check (BadInputsUTxO), not a missing
witness: a witness is a static check, which is exactly what the reapply path is
allowed to skip, so a witness-only failure would not pin down the behaviour we
care about.

'EbTxsInvalid' and 'TraceLeiosEbValidated' are the observable contract the test
asserts on. Nothing emits either yet, hence:

    invalid endorsed tx is not certified: FAIL
      [failed] poisonedNeverCertified
      a poisoned EB was certified

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes ouroboros-leios#1016. 'validateEbClosure' folds an EB's closure onto the
announcing RB's unticked ledger state -- ticked to the EB's slot, which mirrors
what chain-sel does when a later RB certifies it -- through the ordinary Mempool
API. A tx the LeiosTxCache has tagged validated goes through 'reapplyTx', which
re-runs every state-dependent check but skips the static ones; anything else
goes through a full 'applyTx'. On the first failure the EB gets no vote.

Two things the shape of this is trying to protect:

  * It runs strictly after the three cheap gates (TooLate,
    ChainTipDoesNotAnnounce, NotOnCommittee), so an EB we would not vote for is
    never validated.

  * The cache lookups happen in one short pass under the cache's lock and the
    ledger work runs outside it, so validating a 15k-tx closure cannot stall
    LeiosFetch's inserts behind us.

Txs validated here are tagged applied afterwards, so a later EB sharing them
reapplies instead of validating from scratch. In the honest ThreadNet run 18%
of closures are fully reapplied (a forger's own EB) and 14% fully applied, the
rest partial; the remaining full validation is what a mempool-to-cache sync
(the other half of ouroboros-leios#1021) would take away.

'TraceLeiosEbValidated' reports the split, because the issue asks us to watch
what this costs certification. Worth watching on the devnet: validation is
inline in the voting loop, so it serialises EB voting and eats into L_vote.

    invalid endorsed tx is not certified: OK
      poisoned EB turned down on: 100% EbTxsInvalid

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
In an honest net every acquired closure must apply against the announcing RB's
ledger state, so 'propClosuresValidate' fails if any voter reports
'EbTxsInvalid'. A rejection there does not mean a tx was bad -- it means the
voting thread validates against the wrong state or the wrong slot.

Also tabulates the reapply-vs-apply split, so the tx-cache's hit rate shows up
in the run output rather than having to be dug out of the traces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ThreadNet property cannot see any of this: it observes whether an EB gets
certified, not what the tx-cache ends up holding, so it passes whether or not
we record what we validated. These tests look at the cache directly.

To get the function under test, the forker plumbing moves out to the caller:
'validateEbClosure' now takes the announcing RB's ledger state and a
'resolveValues' callback -- the same shape 'resolveAndApplyLeiosClosure' already
uses on the apply path -- instead of a ChainDB. 'EbClosureNoLedger' goes with
it, since the caller now handles a missing forker directly.

Mock blocks are the vehicle: what matters here is the orchestration -- which
ledger rule each tx goes through, and what is recorded -- not the ledger rules,
and the mock UTxO ledger already has generators for valid and invalid txs. That
needs two real 'ResolveLeiosBlock' methods on 'SimpleBlock', narrowed to
@ext' ~ ext@ because that is where 'GenTx' is defined; the certificate-shaped
methods keep their defaults.

Four properties:

  * a fresh closure is validated in full, and every tx ends up tagged
  * a second pass over the same closure reapplies instead of applying, which is
    the cache actually being consulted
  * a closure that fails part-way still records everything validated before the
    failure
  * the failing tx itself is not recorded

Checked they bite, by mutation. Dropping the write-back entirely fails three of
the four (the last passes vacuously -- it is only meaningful next to the
others). Restoring the specific bug this fixed -- collect, and record only if
the whole closure applies -- fails exactly the third.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two problems with the old gates, both flagged by the FIXME this removes.

They counted slots, but the protocol parameters they stub for are diff times,
not slot counts -- there is no whole-slot value to count against. Both gates are
now wall-clock offsets from the announcing slot's onset, which 'slotOnset' reads
from the ledger's hard fork summary: 'lHdrWait' before a vote may be cast,
'lVoteWindow' after that before it is too late. Same 3 and 4, now seconds rather
than slots.

And the deadline was checked before the closure was validated. Validating a
15k-tx closure takes real time, so that reading was stale by the time we were
ready to sign. 'decideVote' now reads the clock after validation. The cheap
checks still come first, so an EB we would not vote for is still never
validated.

Consequences for the loop. The L_hdr gate can no longer be an STM retry on
'getCurrentSlot', so it waits on a 'registerDelay' timeout raced against
'takeAcquisition' instead. That keeps the loop reactive: an EB turning up late
with an older slot does not have to sit out the wait of the one we happened to
be watching, which a plain 'threadDelay' would have cost us. 'ingest' drains
arrivals without blocking when there is already something pending, and every
pass re-reads the earliest, so the decision is always taken against the current
head of the queue.

'slotOnset' returns 'Either PastHorizonException', not a 'runQueryPure' that
throws. The loop is the wrong place to discover that a slot is past the horizon
-- it cannot happen for an EB announced on our own chain, whose slot is at or
behind the tip we just read the summary from, but "cannot happen" inside a
'forever' is exactly what should not be an exception. The point is dropped and
traced rather than retried, so a wedged conversion cannot spin the thread.

'runLeiosVoting' takes the 'SystemTime' in place of the 'BlockchainTime' it no
longer needs, and gains the 'MonadTimer' constraint 'initNodeKernel' already
carried.

Six unit tests for 'decideVote', which is now separable from the loop's state
queries. Mutation-checked: moving the deadline check back before validation
fails exactly "reads the clock after validating".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
'runLeiosVoting' had grown three layers doing one job. 'validateClosure' was
plumbing around 'validateEbClosure', 'decideVote' decided, and 'voteOn' called
the two and cast the vote, threading the validation counts through 'decideVote'
purely so 'voteOn' could trace them. That is one function's worth of work, so it
is one function: 'goVote' reads the committee and ledger state, validates, checks
the deadline and casts, with every way of not voting leaving via 'throwE' and the
reason traced once at the call site.

'goVote' lives in the 'where' clause, taking the three things it cannot reach
from there -- the LeiosDb connection and the derived voting key and signer. It
touches neither the pending set nor the notification channel, so the branch-local
'let' is now only what genuinely closes over those: 'startVoting' with the queue
plumbing and the scheduling loop.

One forker read where there were two reads and a race. The header state that has
to announce the EB and the ledger state its closure has to apply to now both come
from a single forker opened at 'VolatileTip', rather than reading the ChainDB's
ledger and then opening a forker at its tip -- which could move in between.
'withForkerAt' keeps that in plain 'm'; the early-exit flavoured
'withReadOnlyForkerAtPoint' would drag 'WithEarlyExit' through the loop, and
being 'MaybeT' underneath it cannot carry an abstention reason back out anyway,
which forced the reason to be traced twice.

'runLeiosVoting' takes the 'LedgerConfig' rather than the whole
'TopLevelConfig' -- both uses were 'configLedger', and the signing key already
arrived separately.

Removes the six 'decideVote' unit tests: the function no longer exists as a seam,
and 'goVote' closes over too much to reach from a test. What is no longer pinned
is the ordering -- that validation runs before the clock is read, and that a
failed cheap check never validates. The four 'validateEbClosure' tests are
untouched, and the ThreadNet adversary property still covers rejection
end to end.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This is only relative time to avoid a dependency on SystemStart in the
vote loop.
The vote gates are durations: the protocol parameters they stub for are diff
times, not slot counts, so they are measured from the announcing slot's onset
rather than counted against the current slot. The haddock on 'slotOnset' now says
that and nothing more -- it previously offered a rationale about the current slot
that did not hold.

Also from review: 'committing' reworded to 'issuing our vote'; TODOs on both
constants about coming from the ledger state rather than staying constants; and a
note on why the timer delay is clamped to 1us, which is what an already-open
window produces.

In the tests, the claim that an entry can only be upgraded after being marked
acquired is false -- a tx taken straight from the mempool reaches Applied without
that step -- so the comment now says what the harness does rather than asserting
an invariant. The 'NoThunks' remark is moved onto the 'force' it explains, where
it is no longer a non-sequitur between two sentences about the adversary. The
tabulated reapply/apply split no longer claims a forger's own EB should be
entirely reapplied: that is not guaranteed by the implementation and nothing
depends on it.

Finally, the closure-validation property now says why a rejection implicates the
voter rather than the transactions: 'partitionMempool' cuts both the RB's and the
EB's transactions from a single mempool snapshot, which is internally consistent,
and the announcing RB is that snapshot's state plus its own share of the split --
so the EB's share applies against the RB's post-state by construction.
… use site

Review points, in three parts.

The harness marked every closure tx acquired before validating, so the voting
logic was never asked what to do when the cache holds nothing for a tx. That is
reachable -- a tx can arrive straight from the mempool and go to Applied without
passing through the acquired step, and a just-restarted node may hold neither --
and a miss must behave exactly like an unapplied entry. Each property now draws a
per-tx mask deciding what the cache has seen, and the first one tabulates it: at
400 tests the split runs from 0/4 to 4/4, with all four passing throughout.

'invalidEbTx' is now an ordinary invalid transaction, with
'assumeValidatedClosureTx' applied where the adversary uses it. The lie -- that
the tx is mempool-validated, so the forge will endorse it -- is told to the forge
logic and nowhere else, which is the point of the fixture.

The TODO about starting validation earlier conflated two changes. Split: validate
once the whole closure has arrived rather than when the vote window opens, and
validate as the closure streams in. The first is much the cheaper and the second
subsumes it; neither is attempted here.

Also unbreaks consensus-test, which had not compiled since 'recordAnnouncedEb'
gained its onset argument. The fetch invariants do not read the onset, so they
pass SNothing.
…r tx

'recordValidated' took the LeiosTxCache lock for every transaction it validated,
so a full closure acquired it ~13.5k times. The hashes are now accumulated as the
fold runs and written in a single 'withLockedInsertAppliedTx'.

The reason it was per-tx still holds and is preserved: a closure that fails
part-way must keep the tags for everything validated before the failure, since
that work was done against a real ledger state and holds regardless of the
transaction that sank the EB. So the flush happens on both exits -- the valid one
and 'abandon' -- rather than only at the end. Removing it from 'abandon' fails
"records the txs validated before a failure" on the first test case, which is the
guard for exactly this.

Order within the batch is irrelevant, each entry being an independent tag, so the
accumulator is not reversed.
Voting applied each tx with 'DoNotIntervene', which is the mempool's
be-generous-to-remote-peers setting. For Alonzo and later that is not a
courtesy, it rewrites the transaction: applyAlonzoBasedTx forces isValid
true, and on a ValidationTagMismatch retries with it false. So a Plutus tx
declaring isValid true whose scripts fail validated here as a
collateral-only tx, and we voted for the EB carrying it.

The apply path does none of that. applyLeiosClosure runs the LEDGER rule
with ValidateNone, script evaluation sits inside when2Phase and is skipped,
and the UTxO effect branches on the declared flag -- so the chain consumes
the tx's whole input set. The transaction we validated and the transaction
the chain applies were not the same one, which is precisely the trust
applyLeiosClosure cites the certificate for.

'Intervene' leaves the flag alone and lets the mismatch escape as an error,
so we now fail closed on exactly the txs the apply path would mishandle.

Reported by @dnadales in review of #2235.
All from @dnadales's review of #2235.

Fail a vote rather than the node. addVote returning anything but Added was
an error call, which under forkLinkedThread takes the node down. This PR
made it reachable: seatId is resolved from the forker, addVote re-reads the
committee from the current selection, and closure validation now sits
between the two, so an epoch boundary crossed in that gap silently makes the
positional LeiosSeatId someone else's seat. It is now a VoteRejected reason,
traced like the others.

That is containment, not a fix, and the TODO beside it says so: signing
first and finding out afterwards is the wrong shape. We should establish
that we are still on the same committee, and still hold that seat, before
casting the vote.

Check the deadline before validating, not only after. scheduleVoteTime arms
on acquisition and AcquiredEbTxs only fires when the last tx of a closure
lands, so a closure completing late gives a negative delay and the timer
fires already expired. We were spending ~1.5s applying a closure for a vote
that was dead before the timer existed. The check after validation stays:
it is the one that judges the window by the clock at signing time.

Commit every notification read. Looping past AcquiredEb inside the STM
transaction meant a run ending in retry discarded the read-end write, so
skipped messages came back to be skipped again on the next wake. It now
returns Maybe and the caller ignores Nothing.

Also: say what scheduleVoteTime actually does with an already-open window
now that the max 1 clamp is gone, and unescape a haddock pipe.
Both from @dnadales's review of #2235.

The harness answered every table read with the whole initial UTxO, ignoring
the keys it was asked for. Since applyMempoolDiffs restricts the diffs
rather than the values, the key sets could have been empty and all four
properties would still have passed -- confirmed by blinding them, which now
fails three. Restricting the reply also gives the transitive-closure TODO in
validateEbClosure something to fail against once we get to it.

And prop_stopsAtFirstFailure claimed to cover a tx after the failure while
the generator put the failing tx last, so abandon skipping the rest of the
closure was never exercised. The generator now emits a tail that would have
applied if reached, and the property asserts it stays untagged.
@ch1bo
ch1bo force-pushed the ch1bo/leios-eb-validation branch from 63891a3 to 9d81c7f Compare August 29, 2026 13:32
@ch1bo
ch1bo merged commit f6f4650 into leios-prototype Aug 29, 2026
7 of 20 checks passed
@ch1bo
ch1bo deleted the ch1bo/leios-eb-validation branch August 29, 2026 14:47
ch1bo added a commit that referenced this pull request Aug 29, 2026
…#2235, #2245)

Merges the stacked series, bottom to top:

* #2188 Leios: add LeiosTxCache, but don't rely on it at all yet
* #2237 Rewrite the LeiosFetch decision logic
* #2235 Leios prototype: EB validation in vote logic
* #2245 LeiosDb: bound the WAL, remove the write hotspot, stop dying on
  lock contention

IMPORTANT: the schema change in #2245 has no migration. An existing
database is not readable by this version and requires a wipe & resync.
@ch1bo ch1bo added the Leios label Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prototype: EB validation

4 participants