Skip to content

New pigeonhole thinclient API - #985

Merged
david415 merged 443 commits into
mainfrom
new_pigeonhole_thinclient2
May 4, 2026
Merged

New pigeonhole thinclient API#985
david415 merged 443 commits into
mainfrom
new_pigeonhole_thinclient2

Conversation

@david415

@david415 david415 commented Feb 2, 2026

Copy link
Copy Markdown
Member

NOTE: that this branch enables the replica static uint8 identity patchset which changes the replica and dirauth config formats to include a replica ID field which is a unique uint8 integer. this helps keep our hash based sharded storage scheme consistent.

Here we modify the client, courier and replica to get the protocol working end to end.

Initially, this set of commits is mainly concerned with updating the client2 thinclient API to contain the latest Pigeonhole thinclient API. HOWEVER, we also update the github CI workflow to remove the old pigeonhole tests and run the new tests. Also we make sure to separate out the Sphinx benchmarks and run them only once per month whilest running all the other benchmarks once per day.

We fix some bugs in the courier and replica and client.... so that finally we make the messages flow end to end and the docker mixnet pigeonhole integration test passes.

@david415 david415 changed the title New pigeonhole thinclient2 New pigeonhole thinclient API Feb 2, 2026
@david415
david415 force-pushed the new_pigeonhole_thinclient2 branch from 6d509ab to 03d7b03 Compare February 20, 2026 14:23
@david415
david415 force-pushed the new_pigeonhole_thinclient2 branch 4 times, most recently from 892594d to 0d80f22 Compare March 7, 2026 16:59
david415 added 24 commits March 15, 2026 11:56
…ately

When the dedup cache contains only read errors (e.g. BoxIDNotFound),
trigger a background re-dispatch to the replicas so the cache refreshes
for the next client retry. Return the cached error immediately via
handleOldMessage rather than calling handleNewMessage, so NoRetry
clients receive the error and stop instead of looping on ACKs.
Adds a dedicated LambdaR parameter to the PKI document to govern the
send rate of decoy traffic between couriers and storage replicas,
replacing the temporary LambdaP placeholder. Wired through the voting
authority config, state machine, genconfig, and docker Makefile.
note that this feature is essentially disabled by default unless
the config option MetricsAddress is set
here we also fix the courier such that the replica integration tests pass
Generate Grafana provisioning configs and a pre-built dashboard
for courier/replica decoy traffic metrics. Add prometheus scrape
jobs with per-instance labels (replica1, courier1, etc.).

Add --noMetrics flag to skip prometheus/grafana containers in
docker-compose, used in CI to avoid pulling external images.
The queue length gauge was only updated on enqueue, causing it to
flatline at 1. Now also updates when the sender drains a message,
so the gauge reflects actual queue depth.
The 2-minute warped epoch is too short for local testing with decoy
traffic enabled. The KATZENPOST_EPOCH_DURATION env var can override
this at runtime for contexts that need the shorter period.
sendLoopDecoy and sendDropDecoy panicked when no PKI document or
echo services were available. Log a warning and return instead.
david415 added 29 commits April 23, 2026 10:55
Heretofore dispatchCopyEnvelope would, upon receiving but a solitary
reply of BoxAlreadyExists before the deadline, most rashly pronounce
"both intermediates report BoxAlreadyExists" and abort the Copy
entire — a conclusion drawn, one regrets to say, from evidence
patently insufficient. Under the ordinary K=2 arrangement, one
intermediate's write arrives first at the shard (whereupon Success is
returned) whilst its peer sees the prior write and replies
BoxAlreadyExists; should the Success reply chance to tarry past the
thirty-second deadline, the old logic would terminate the Copy with
CopyStatusFailed in error.

Henceforth any terminal verdict demands the testimony of both
intermediates. A Success reply continues to short-circuit the attempt
forthwith. A shortfall of replies is treated as a transient
embarrassment and retried. Only with two non-Success replies does one
consult bestErr and, upon BoxAlreadyExists at attempt zero, abort —
preserving the invariant upheld by TestCopyOntoAlreadyExistingBoxError.

Pinned by the new TestDispatchCopyEnvelopeSingleBoxAlreadyExistsReply
IsTransient, which feeds a single BoxAlreadyExists reply on attempt
zero, lets the deadline elapse, and requires the courier to re-dispatch
rather than abort.
Legacy-format Consensus replies and malformed Consensus2 chunks
off the gateway wire previously paniced the whole daemon, taking
every connected thin client with it. Follow the pattern already
used for unknown command types: log, set wireErr, return — the
reconnect loop then retries cleanly.
Three retry sites (arqDoResend, handlePigeonholeARQReply SendNewSURB,
handlePayloadReply BoxIDNotFound retry) previously returned without
rearming the ARQ timer when ComposeSphinxPacketForQuery tripped —
leaving the ARQMessage orphaned and the thin client's request wedged
until disconnect. A fresh helper now keys the message under a
placeholder SURBID and pushes a one-second retry, so a transient
PKI stumble no longer strands a request in silence.
The pigeonhole API event loops wrote isConnected bare upon receiving
a ConnectionStatusEvent, whilst dispatchMessage did so under connMu;
three public send methods likewise read the field bare. -race caught
the collision at thin.go:799 against thin.go:1506. Introduce a
setConnected helper and route every write through it, every read
through IsConnected.

Pinned by TestIsConnectedRaceFree.
A ConnectionStatusEvent with IsConnected=false previously panicked
the calling goroutine via panic(errConnectionLost); an event type the
switch did not recognise panicked with "impossible event type". A
mid-flight disconnection is an ordinary recoverable outcome and an
unknown event type warrants a debug line and continuation, not the
loss of the process.

Pinned by TestBlockingSendMessageDisconnectReturnsError and
TestBlockingSendMessageUnknownEventDoesNotPanic.
Every outbound packet traversed two RWMutex cycles to read
isConnected and isShutdown. The two booleans never need multi-field
consistency, so sync/atomic.Bool serves the same purpose with a
single memory-order load per read. Test sites updated to match.
writeResponse previously allocated a fresh slice to concatenate the
4-byte length prefix with the CBOR-encoded response before a single
Write. net.Buffers{prefix, blob}.WriteTo coalesces into one writev
syscall on tcp and unix transports, and emits two Writes on those
that lack it — either way, no intermediate allocation.
ComposeSphinxPacket and ComposeSphinxPacketForQuery each retried
path selection indefinitely when it straddled an epoch or produced
excessive delays, leaving only the halt channel as an escape. Under
a pathological PKI document the loops spin at full CPU per outbound
packet. A cap of 32 attempts returns a clear error well above the
handful a healthy epoch rollover requires.
…ariants

The three public methods differed only in two boolean flags on the
outgoing request and in their own name appearing in the log prefix.
Collapse the ~100-line body into startResendingEncryptedMessageImpl
parameterised by the two booleans; the three public methods become
one-line shells preserving their doc comments. Log lines now include
(noRetry=%v, returnBoxExists=%v) so the variant remains visible.

Net −160 lines. Behaviour unchanged; existing tests
(pigeonhole_test.go:519,554) still pin flag propagation.
The fan-out worker formerly evicted any drain that did not accept
within 100ms, with nothing but a Warning log to the evicted
subscriber. Neither the Rust nor the Python thin client drops
events: Rust's drains are tokio UnboundedSenders, Python calls
handlers directly. The Go divergence was silent and surprising to
integrators porting across.

Remove the timeout. The fan-out now blocks until the subscriber
accepts; HaltCh remains the escape hatch for Close(). A stuck
subscriber will stall the fan-out for every subscriber — the same
hazard Python already exposes with a stuck callback — so the
EventSink doc is updated to say so plainly.

Pinned by TestEventSinkNoEviction, which sleeps past the old 100ms
threshold with an idle subscriber and asserts all events arrive.
A CancelResending* landing between handleReply's delete and the
handler's re-register silently no-opped whilst the daemon carried on
polling. handleReply no longer deletes; rotation paths guard with a
still-present check under replyLock. Pinned by three tests in
pigeonhole_cancel_race_test.go.
injectCourierEnvelope returns on the courier's immediate write-ACK,
well before the shards have durably stored. The 5s post-write sleep
was racing replication under CI load and the subsequent proxy read
saw BoxIDNotFound. Bump to 15s.
sendAndWait subscribed a drain via EventSink() but never released
it. The old 100ms eviction masked the leak; after the switch to the
no-loss fan-out contract (commit 4d92da3) each leaked capacity-1
drain stalled the fan-out for every other subscriber on the same
thin client. repeatSendAndWait (5 iterations on one client) accrued
enough leaks to wedge TestLegacyTests/TestDockerClientSendReceive
for the 1h suite timeout. Add the missing defer StopEventSink.
When a BoxID is occupied, the stored Box is now compared against the
incoming write. Matching Payload and Signature returns success; only
differing data still returns ErrBoxAlreadyExists. This dissolves the
ambiguity the courier's K=2 copy path faced when a reply was lost on
retry, which TestCopyOntoAlreadyExistingBoxError exposed. The
replication layer already assumed this idempotence; the state layer
now agrees.
With the replica's matching-data writes now idempotent, a non-Success
reply from any intermediate reflects a genuine conflict — retrying
will not change the verdict. dispatchCopyEnvelope no longer waits
for both replies before declaring failure, and no longer treats a
retry's BoxAlreadyExists as our own prior write landing. Retries
are reserved for transport failures (SendMessage error, no replies
before the deadline). Two tests whose premises the replica fix
dissolved are removed; one new test pins the single-reply abort.
The pruneDocuments path removed expired entries from the in-memory
maps but never deleted the corresponding sub-buckets and document
keys from bolt. The descriptors, replica_descriptors and documents
buckets therefore accumulated state for every epoch the dirauth had
ever served, so persistence.db grew without bound and would in time
fill the disk. Restart only made matters worse, since restorePersistence
loads at most three epochs into memory and leaves the rest as dead
weight on disk.

This adds a small bolt transaction at the end of pruneDocuments which,
using the same preservation window as the in-memory pass, deletes any
sub-bucket or key whose 8-byte big-endian epoch is strictly older than
now minus three epochs. A unit test exercises the pruner directly
against a temporary bolt database and confirms that fresh epochs and
non-epoch keys are left untouched and that a second pass is a no-op.

Note: bolt does not return freed pages to the filesystem in place, so
operators with an already-bloated persistence.db will need to compact
or simply replace the file once this fix has stopped further growth.
…etch

GetPKIDocumentForEpoch previously walked the configured authorities in
sequence, calling fetchConsensus on each in turn. As fetchConsensus
itself retries with exponential backoff (up to RetryMaxAttempts) before
giving up on a single peer, a lone unresponsive dirauth at the head of
the random permutation could consume the caller's entire context budget
before any other peer was tried. This blocked the mix-server PKI worker
on every epoch and, more grievously, prevented dirauth bootstrap from
populating prior-epoch documents after a consensus failure, contributing
to the cascade observed in production.

Each configured authority is now contacted in its own goroutine. The
first peer to return a verified, threshold-signed, well-formed document
for the requested epoch wins; the deferred cancel terminates the
remaining in-flight fetches, which write to a buffered channel and
exit. A single down or slow dirauth no longer holds up its peers.

A regression test, TestGetPKIDocumentForEpochParallelRace, pins the
new behaviour: with four of five peers stalled at dial, the fetch
completes in well under a second using the responsive peer.
TestGetPKIDocumentForEpochParallelRace was flaking on darwin CI: the
post-quantum (XWING) handshake alone took over five seconds on the
shared macOS runner, exceeding the original five-second elapsed-time
assertion. The wall-clock budget was tightened to twenty seconds, with
a comment explaining the choice. Sequential behaviour, were it ever to
return, would still block for the full thirty-second context, so the
threshold remains amply discriminating. No behavioural change to the
production code.
…s test

TestParallelFailingAuthorities asserts that allPeersRoundTrip returns
within three seconds when several peers refuse the dial outright. The
post-quantum handshake performed for each successful peer makes that
budget too tight on a shared macOS CI runner whose host has been busy
working through the rest of authority/.... Bumping the bound to twenty
seconds, in line with the earlier accommodation made for the parallel
race test. The threshold remains amply discriminating: a sequential
implementation, were it to return, would block on the full thirty
second context. No production-code change.
Daemon.encryptRead and similar callers reach Client.CurrentDocument,
which calls WaitForCurrentDocument. When the cached document for the
current epoch is absent (whether at startup, across an epoch boundary,
or under any future bug), WaitForCurrentDocument falls through to a
synchronous updateDocument and ultimately to pki.getDocument, which
calls p.c.PKIClient.Deserialize(...). If PKIClient is nil, the call
panics with a nil-interface method dispatch fault. Although Client.Start
sets PKIClient before any caller can normally observe it, there is no
guarantee against a future regression or an exotic shutdown sequence.

A defensive nil-check is added at the call site so the synchronous
fetch returns a clean, logged error rather than crashing the process.

The unit test TestAliceSendsBobMessage now wires up a mockPKIClient on
its hand-built Client. The previous setup populated only the document
cache and silently relied on the cache hit; on a slow darwin runner the
cache miss path was reached and the panic surfaced. With a mock present
the test produces a deterministic outcome whichever path runs.
@david415
david415 merged commit bd071a8 into main May 4, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant