Skip to content

Draft-05 and sunlight mirror support - #10

Merged
mcpherrinm merged 22 commits into
mainfrom
review-fixes
Jul 21, 2026
Merged

Draft-05 and sunlight mirror support#10
mcpherrinm merged 22 commits into
mainfrom
review-fixes

Conversation

@mcpherrinm

Copy link
Copy Markdown
Owner

This updates to draft-05, and switches to using external mirrors.

Add integration testing with sunlight.

Various bugfixes from review.

this is all vibecoded.

Draft-05 changes no Merkle-tree, signature, certificate, or checkpoint
bytes: TBSCertificateLogEntry, MTCProof, CosignedMessage, the checkpoint
and log-origin encoding, the OIDs, and the ID derivation rules are all
unchanged. Three things do change.

maxSerial (§5.5) is the one breaking wire change. It is a required field
appended to the MTCCertificationAuthority SEQUENCE, so -04 and -05
encodings are mutually unparseable and any issued CA certificate has to
be reissued. §7.1 correspondingly revokes [0, minSerial) *and*
[maxSerial+1, 2^64).

That upper range is not representable as a half-open uint64 range, so
RevokedRange is now closed rather than half-open. Clamping the exclusive
end to 2^64-1 would have silently left the largest serial unrevoked,
which no existing test would have caught; TestInitialRevokedRangesUpperBound
pins it. Note a zero-valued MTCCertificationAuthority now revokes almost
everything, which is the fail-closed direction.

The ACME link relation "enhancement" becomes "acme-optional-alternate",
registered in the new §9.1. That section standardises exactly what
314dc76 arrived at independently — an optional alternate that returns
202 + Retry-After and must not block issuance — so only the relation
string changes; the status code, headers, and media type stay put.
(draft-04 had specified 503 here.)

§7.2 step 2 now requires verification to fail if signatureValue is not a
whole number of octets. encoding/asn1 accepts a non-zero unused-bits
count and reports it via BitLength, so SplitCertificate has to reject it
explicitly rather than parse a proof out of a partially-used final byte.

Also adds the four Appendix C "accumulated" test vectors, which are new
in -05. Each is one rolling SHA-256 over the formatted output of every
valid input for trees up to size 130, covering subtree hashes, subtree
inclusion proofs, subtree consistency proofs, and covering-subtree
selection. They pass unmodified, which independently confirms the §4
algorithms in tlogx — including that FindSubtrees followed the reference
Python rather than -04's buggy prose, one of the two things -05's change
log calls out as fixed.

Section-number citations throughout the tree still refer to -04 and are
renumbered separately; -05 renumbers §6.x and inserts a new Appendix B.2.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
cactus no longer acts as a mirror or witness. Mirroring becomes
push-based against an external c2sp.org/tlog-mirror (Sunlight), which
inverts the relationship: a tlog-mirror never fetches from the log, so
the log has to push to it. The pull follower and the /sign-subtree
server we exposed are the wrong half of that protocol.

The two halves went together. mirror/server.go signed subtrees of the
log copy that mirror/follower.go had fetched and verified, so keeping
the server without the follower would leave it attesting to a log it no
longer independently checks — worse than not having it. Both go, along
with the mirror.* config section, the sign-subtree listener, the four
mirror-mode metrics, and config-witness-example.json.

What stays is the CA *client* side: ca_cosigner_quorum.mirrors[] and
cert.RequestCosignatures still request cosignatures from external
mirrors. That is the direction cactus keeps.

The quorum tests previously stood up real followers and servers to
answer sign-subtree. They now use an in-package stub witness that parses
just the subtree range and hash and returns one ML-DSA-44 cosignature
line. Both tests came out stronger: the three-cosigner test had a no-op
verification loop that only counted recognised cosigner IDs and now runs
a real cert.VerifyMTCSignature on each, and the quorum test now also
asserts that the slow witness is absent from the collected signatures
while still having been contacted, which is what makes "fan out to all,
wait only for the minimum" observable rather than merely plausible.

Coverage genuinely lost with the feature: independent tree rebuild from
tiles, follower restart resume, halt-on-bad-key, and the
require_ca_signature_on_subtree DoS gate. Those tested the deleted code.

docs/test-instance.md was premised on one process being both CA and
witness and pointed at the deleted example config, so it is trimmed to a
CA-only guide rather than left dangling.

Note for anyone with a deployed config: because config.Load uses
DisallowUnknownFields, a leftover "mirror" block is now a load error
rather than an ignored key.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
A tlog-mirror never fetches from the log it mirrors, so replication is
the log's job: drive add-checkpoint to advance the mirror's pending
checkpoint, upload entries with add-entries, and collect the mirror's
cosignature on the resulting checkpoint. The new mirrorpush package
implements that client and cmd/cactus drives it from OnFlush. With no
mirror_push targets configured, behaviour is unchanged.

Three parts of the wire format are easy to get subtly wrong, so each has
a test that fails if the detail is reversed:

  - An add-entries package proves the subtree [rounded_start + i*256,
    end), not [start, end). When upload_start is unaligned the client
    transmits only its own entries but must prove the whole 256-aligned
    bundle; the mirror reconstructs the leading entries from its own
    storage. Proving [start, end) instead yields either an invalid
    subtree or a valid-but-wrong one that 422s.
  - The recovery ticket is raw bytes in the request and base64 in the
    202/409 response. It is decoded on receipt and stays raw thereafter.
  - add-entries and sign-subtree use the MTC §4.4 *subtree* consistency
    proof, while add-checkpoint uses an RFC 6962 *tree* consistency
    proof. These are different algorithms over the same tree, so the two
    are now separate named methods rather than one ambiguous one.

Also fixes two ways the existing sign-subtree client violated the
current c2sp.org/tlog-witness spec, both of which a strict mirror
rejects outright:

  - It emitted a CA subtree cosignature line. C2SP deleted subtree
    cosignature lines from the grammar; the line is not base64 and fails
    to parse as a consistency proof hash, giving 400. Removed, along
    with the now-dead CASignature/CACosignerID/CACosignerKey fields.
  - It presented a CA-only-signed checkpoint. The spec now requires the
    reference checkpoint to carry the *responding mirror's own*
    cosignature or the request is refused with 403. That cosignature
    only ever comes from an add-entries 200 response, which is why the
    push client has to exist before sign-subtree can work at all: the
    pool now feeds its retained cosigned checkpoint back in.

Since a signed note carries many signature lines and each cosigner
ignores lines that are not its own, every mirror's cosignature is
appended to a single note, so the CA still fans one body out to all
mirrors rather than building one per mirror.

State advancement is deliberately conservative: the next entry index
moves only on a 200 or a value the mirror explicitly advertised, a 202
that fails to advance is an error rather than a spin, and the ticket is
persisted with the pending size it belongs to. 422 is treated as an
integrity signal — a proof failed to verify — so it is fatal rather than
retried, as is 400.

Supporting additions: cert.MarshalSignatureInputAt takes a timestamp
(checkpoint cosignatures require a non-zero one, subtree cosignatures
require zero); TileWriter.ReadEntries and Log.Entries read entries back
out for upload; Log.TreeConsistencyProof supplies the RFC 6962 proof.

FuzzAddEntriesFraming found a builder/parser asymmetry (the parser
accepted a zero-length origin the builder refuses) and an unbounded
canonical sequence: header fields are uint64, so a corrupt header can
describe 2^55 packages. Parsing is now lazy and bounded by bytes
received. Regression corpus committed.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
The two sides of a mirroring relationship want the same key in
different encodings. A tlog-mirror's log list identifies our log by a
c2sp.org/signed-note vkey; our own mirror config wants a PEM public key.
Previously only the PEM direction existed, so wiring the two together
meant hand-assembling a vkey.

-vkey emits one, -from-vkey converts one back.

The vkey name is the COSIGNER's OID name, not the log's, and -from-vkey
recomputes the key ID from the name and key rather than trusting the
one in the string. Both details are load-bearing. A cactus checkpoint's
origin line is the log ID (<CA-ID>.0.<log number>) but its signature
line is labelled with the cosigner name, which is the CA ID (§5.4); a
verifier matches on that name, so a vkey generated under the log ID is
silently unmatchable and the mirror answers add-checkpoint with a bare
403. Recomputing the key ID likewise turns a truncated or mistyped vkey
into a startup error instead of a cosigner whose signatures never match
anything. The flag is named -cosigner-id to make the distinction hard to
get wrong, and consumers are expected to carry the log origin separately
(sunlight's log list has an `origin` line for exactly this).

Note the vkey's key material is standard base64, whose alphabet includes
'+', so parsing splits on only the first two separators.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
`make docker-up` brings up cactus, Sunlight as a c2sp.org/tlog-mirror
and ML-DSA-44 cosigner, and skylight serving the mirror's read side.
Verified end to end: cactus pushes its log, Sunlight mirrors the entries
and cosigns the checkpoint, and an issued certificate carrying that
cosignature passes full §7.2 verification.

The cactus image does not build from source. cactus needs Go 1.27 for
crypto/mldsa and no golang:1.27 image exists yet — a released Go cannot
even download the toolchain — so the binaries are cross-built on the
host with gotip and copied in. Sunlight has no such constraint (go 1.25,
filippo.io/mldsa rather than the standard library) and builds in its own
image, but does need cgo: its checkpoint lock backend is
crawshaw.io/sqlite, and CGO_ENABLED=0 builds only sunlight-keygen.

Four things about running Sunlight as a mirror cost real debugging time
and are documented where they bite:

  - submissionprefix MUST NOT carry a port. Sunlight registers its
    handler on net/http.ServeMux as "<host>/", and ServeMux strips the
    port from the request Host before matching, so a pattern containing
    one matches nothing and every endpoint 404s with nothing logged. A
    portless pattern matches any port. The https scheme is still
    required — Sunlight rejects anything else — but is never used for
    transport, since without an acme section it serves plain HTTP/h2c.
    skylight has the same trap.
  - Sunlight cannot run as a mirror alone. main() ends on
    sequencerGroup.Wait(), and sequencer goroutines are started only per
    entry in `logs:`; with none, Wait returns immediately and the process
    shuts down and exits 1 without logging anything, despite the
    witness/mirror machinery being fully initialised. A throwaway
    keepalive CT log we never submit to keeps it alive. That log needs
    notafterstart/notafterlimit, and an explicit empty roots file —
    otherwise CCADBRoots defaults to "trusted" and it fetches the CCADB
    root list over the network at startup. Its inception date must be
    today when first created, so the config is rendered at init time.
  - A log listed in both loglists and mirrorloglists logs a single
    "adding new log" line with mirror=false; the mirror upgrade path
    sets the flag and continues silently. mirror=false in the log is not
    evidence the mirror is unregistered — probe add-entries instead,
    where 422 means known and 404 means unknown.
  - Sunlight refuses to create its own checkpoint lock table, as a guard
    against pointing a log at the wrong backend, so init creates it.

Key exchange is three init containers because neither side can start
without the other's public key: cactus exports its log vkey, Sunlight
derives its keys and builds the log list, then the vkey is converted
back to the PEM form cactus consumes. All three are idempotent, so
bringing the stack up twice does not rotate keys out from under a log
that has already been mirrored; docker-down deletes the volumes and
therefore the keys.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Follows the code changes across the last four commits.

The draft-05 rename of the ACME link relation had left stale references
behind: README, MTC.md, and test-instance.md all still described
`rel="enhancement"`, which no longer exists. They now say
`acme-optional-alternate` and cite §9.1, which is also a better
explanation than the old text — the non-blocking behaviour is now
specified rather than something cactus decided on its own.

test-instance.md was trimmed to a CA-only guide when the mirror mode was
deleted, because its premise (one process being both CA and witness) had
stopped being true. Now that there is a working mirror again it points
at the compose stack for that case, rather than describing a
by-hand setup that would mean reimplementing log replication.

Also corrects two things that were quietly wrong. CLAUDE.md said
integration tests live behind the `integration` build tag; there are no
build tags in that package, so plain `gotip test ./...` has always run
them and `make integration`'s `-tags=integration` selects nothing — it
is really just the `-race -count=1` runner. And landmark sequences are
§6.4 in draft-05, not §6.3.

The remaining ~330 in-code §x.y citations still refer to draft-04
numbering and are a separate mechanical pass. Two hazards for whoever
does it: draft-05 inserts a new Appendix B.2, so stale appendix
references point at a real but wrong section rather than dangling; and
`acme/` mixes MTC and RFC 8555 citations whose numbers collide (§6.3 is
Landmark-Relative Certificates in one and POST-as-GET in the other), so
it cannot be swept mechanically.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
TestBulkIssuanceStress issues 800 certificates concurrently through the
full ACME flow, verifies each against the log, and then checks the log
is internally consistent. Behind the `stress` build tag so it stays out
of the default suite; size and concurrency are tunable via
CACTUS_STRESS_CERTS / CACTUS_STRESS_CONCURRENCY, so the same test serves
as a quick local run and a heavier soak.

800 is past both the 256-entry tile boundary and the existing
100-certificate TestParallelIssuance, so it exercises multi-tile growth
and partial tiles under concurrent load.

The load is not really the point — the post-conditions are. The log is a
single writer with no cross-process coordination, so a sequencing bug
surfaces as a duplicated or skipped index rather than as an error any
individual request would observe. Asserting only that every request
returned 200, or only that the tree ends at the right size, would miss
two entries swapping or an index being handed out twice while the count
stays correct. So the test requires the assigned indices to be exactly a
permutation of [0, n).

It also reloads the published entry tiles, replays them through
tlog.StoredHashes, and compares the recomputed root against the signed
checkpoint. The per-certificate inclusion proofs cannot catch a tile
written inconsistently under concurrent flushes, because they are served
from the same in-memory hashes the checkpoint was built from.

Measured: 800 certs in ~0.6s (~1300/sec) plain, ~1.5s under -race, and
5000 certs in ~3.1s. Clean under the race detector.

I checked the permutation assertion is not vacuous by mutating
certLogIndex to collide two indices; it fails with "log index 0 was
assigned to 2 certificates" and "log index 99 was never assigned".

Also fixes two stale claims in the docs found while writing this: the
integration package has no build tags (so `gotip test ./...` has always
run it, and `make integration`'s -tags=integration selects nothing), and
README cited integration.TestRelyingPartyFastPath, which does not exist
— the landmark fast-path case is TestLandmarkRelativeCertConstruction.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Runs integration.TestBulkIssuanceStress under -race on a schedule rather
than on every push. The test takes seconds, so cost is not the reason —
its value is in repetition. A sequencing race that survives one run may
not survive fifty, and the assertions it makes about log index
assignment are exactly the kind that fail intermittently or not at all.
Running it nightly samples that; running it once per push would not, and
would slow CI for little added signal.

It also runs on pull requests that touch the issuance path (log, ca,
acme, cert, tlogx), since that is where a regression it could catch
would originate, and on workflow_dispatch with overridable size and
concurrency for a deliberate soak. Scheduled and pull_request runs pass
empty inputs, which the test reads as "use the defaults" (800 / 64).

CI proper gains a vet step for the stress-tagged file. It is behind a
build tag, so nothing in the normal suite compiles it, and without this
a break would go unnoticed until the next nightly run — landing on
whoever reads that failure rather than on the pull request that caused
it.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Comment-only, plus one user-facing validation string.

Far less moves than the citation count suggests. Sections 4, 5, 7 and 8
are identical between the drafts, and 9, 12 and 13 only gain new
subsections without renumbering existing ones. Two families shift:

  §6.x   draft-05 inserts a new §6.1 "Certificate Inputs", pushing
         Certificate Format to §6.2, Standalone Certificates to §6.3,
         Landmark-Relative Certificates to §6.4 (and its subsections to
         §6.4.x), and Size Estimates to §6.5.
  App B  draft-05 inserts a new B.2 "Subtrees", pushing Inclusion Proof
         Evaluation to B.3, Consistency Proof Structure to B.4, and
         Consistency Proof Verification to B.5.

Done in a single pass per line, so a citation cannot shift twice — a
naive sequence of replacements turns §6.1 into §6.2 and then §6.2 into
§6.3.

Three things made this not purely mechanical.

Appendix B fails silently rather than loudly. Because -05 *inserts* B.2
rather than appending, a stale "Appendix B.2" on ProofLen still names a
real appendix — just "Subtrees" instead of "Inclusion Proof Evaluation".
Nothing would have flagged it. Same for the two §B.3 consistency-proof
references, which are Consistency Proof Structure and are now §B.4.
(An earlier reading of these had them down as RFC 9162 citations to
leave alone; the cited content matches the MTC appendices exactly, and
RFC 9162 has no subtree proofs in this form.)

§6.x collides with two other specs cited in the same files, sometimes
within a few lines. RFC 8555 numbers its own §6.2 (POST content type),
§6.3 (POST-as-GET), §6.4 (url header) and §6.5 (nonces) — acme/handler.go
cites MTC §6.1/§6.3/§6.3.1/§6.3.2 alongside all four. trust-anchor-ids
has a §6/§6.1 for the property-list PEM layout. Both are excluded by
explicit file:line rather than by pattern, since no pattern distinguishes
them.

Five sites were already mis-cited and a mechanical shift would have
preserved the error under a fresh-looking number. cert/landmark.go and
cmd/cactus-cli/cert.go cite §6.3.3 while describing how a
landmark-relative certificate is *constructed*; §6.3.3 is "Publishing
Landmarks" and construction is §6.3.4. They are corrected to §6.4.4, not
the §6.4.3 a shift would have produced.

Also drops a phantom citation: integration/multi_mirror_test.go referred
to "§6.2/9.4", and §9 has no subsections in draft-04 and only 9.1/9.2 in
draft-05.

The `draft-04` labels become `draft-05` except in cert/cacert_test.go,
where they deliberately name the old encoding the maxSerial test rejects.

Not touched, and worth knowing about: several §6.4.1 citations (formerly
§6.3.1, "Landmark Tree Sizes") sit on code handling the published
/landmarks list, whose format is really §6.4.3 "Publishing Landmarks".
The shift is correct; whether §6.3.1 was the right citation to begin with
is a pre-existing question across ~30 sites, and sorting it out means
judging intent case by case rather than renumbering.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Two issuance-authorization gaps in the CSR validator:

1. buildExtensionsFromCSR copied the subjectAltName extension value
   verbatim while Validate only cross-checked dNSName/iPAddress entries
   against the order. A CSR whose SAN carried an rfc822Name, URI, or
   otherName alongside an authorized dNSName smuggled an unvalidated
   identity onto the leaf certificate. Reject any GeneralName type other
   than dNSName/iPAddress (sanContainsOnlyDNSAndIP).

2. Validate enforced only CSR ⊆ order (every CSR SAN authorized). RFC
   8555 §7.4 requires the CSR to indicate the exact same set of
   identifiers, so also enforce order ⊆ CSR.

Adds regression tests for both.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Several RFC 8555 §6.3 access-control gaps in the ACME handlers:

- handleOrder / handleAuthz / handleChallenge fetched objects by ID and
  returned them without checking the authenticated account. Any account
  holding an ID could read another account's order, authz, or challenge
  (and in http-01 mode drive validation of a foreign challenge with the
  caller's key authorization). Trace ownership order/authz/challenge →
  account and report a foreign resource as 404.

- handleFinalize discarded the account entirely, so an account that knew
  a victim's ready order ID could finalize it with its own CSR (§6.3,
  §7.5). Bind the order to the caller before the ready→processing claim.

- §11.1: the CSR public key was never compared to the account key.
  Reject a CSR whose key equals the account key.

Also: emit Content-Type: application/json on new-account/new-order
responses, add the §7.1 rel="index" Link relation on resource responses,
and exclude invalid orders from the account orders list (§7.1.2.1).

Adds cross-account and account-key-CSR regression tests.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
The §7.2 verifier accepted the optional TBSCertificate tail fields
(issuerUniqueID [1], subjectUniqueID [2], extensions [3]) in any order
and canonicalized them into a fixed-order log entry. Two byte-distinct
certificates — one valid DER, one with the tail tags swapped (invalid
DER per X.690 SEQUENCE ordering) — therefore rebuilt to identical entry
contents and both satisfied the same inclusion proof and cosignatures, a
draft-05 §12.6 certificate-malleability violation.

Enforce strictly ascending tail tags. Adds a regression test.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
VerifyCertificate checked only the outer Certificate.signatureAlgorithm.
draft-05 §7.2 step 1 refers to the TBSCertificate's signature field
(the inner AlgorithmIdentifier, which RFC 5280 §4.1.1.2 requires to equal
the outer one). RebuildLogEntryFromTBS parsed that field only to discard
it. Validate it as id-alg-mtcProof with absent parameters so both the
inner and outer algorithm IDs are checked.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
discover() seeded next-entry from the mirror's advertised checkpoint size
via an unauthenticated GET (no signature verified). A forged or buggy
oversized value made every pushEntries take the "mirror is ahead"
bail-out, which returned nil with no log line — so add-entries was never
sent, the mirror-info correction loop never ran, and the mirror silently
dropped out of cosignature collection until process restart.

Clamp the discovered seed to our own checkpoint size (a genuinely-ahead
mirror is corrected by the authenticated add-entries loop) and log a
warning both when clamping and in the ahead-of-us bail-out. Adds a
regression test.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Three smaller correctness fixes in the push client:

- The add-checkpoint 200 path advanced pendingSize without clearing the
  ticket, so the next add-entries sent a ticket issued for the previous
  size paired with the new upload_end. Clear the ticket on advance.

- do() read exactly maxResponseBytes via LimitReader, so a truncated
  oversized body was indistinguishable from a complete one and could be
  escalated as a fatal mirror fault. Read one byte past the cap and
  return a distinct "response too large" error.

- VerifyCosignatures accepted any timestamp; reject values above 2^63-1
  (tlog-cosignature MUST) rather than retaining and republishing them.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
- log.pool_size was parsed, defaulted, and validated but never used.
  Wire it as log.Config.MaxPoolSize: Append signals the sequencer for an
  early flush once the pool reaches it, bounding pool memory and issuance
  latency. main passes cfg.Log.PoolSize.

- log.shortname was required but unused; make it optional and attach it
  to the structured logger context.

- A listener that failed to bind was only logged while the process kept
  running with a dead endpoint. startServer now reports on an error
  channel and run() shuts down and returns the error.

- acme.tls_cert / tls_key: require both or neither (a lone one silently
  served plaintext), and resolve their paths relative to data_dir like
  every other configured path.

- ca_cosigner_quorum.mirrors[].algorithm was only checked non-empty;
  require mldsa-44 to match the profile (and mirrorpush.New).

- cactus-keygen -vkey hardcodes the ML-DSA-44 signature-type byte;
  reject -alg mldsa-65/87 instead of emitting a mislabeled vkey.

Adds config and log regression tests.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Documentation only, no behavior change:

- README/CLAUDE.md: the ca_cosigner.id example used the absolute OID
  form; cactus stores trust anchor IDs relative to the 1.3.6.1.4.1 base,
  so use 44363.47.1.99. Document shortname, pool_size, tls_cert/tls_key,
  gate the pprof note on a loopback metrics listener, and fix the prove
  example's missing log-number prefix.

- docs/disk-layout.md: drop the nonexistent log/subtrees/ directory and
  the false O_EXCL tile-write claim; add state/landmarks/sequence.jsonl
  and mirrorpush/. docs/threat-model.md: the temp file is not fsynced
  either. storage.Put doc: the exclusive path is not reader-atomic.

- MTC.md: checkpoint_period_ms (not flush_period_ms); the mirror role
  lives in mirrorpush/, not a mirror/ package; per-subtree signatures are
  in-memory only; "seven" indices, not "three".

- docker/README.md: cactus-keygen -cosigner-id (not -log-id); init step
  counts are 1/2/3-of-3. landmark handler: the list format is §6.4.3.
  Makefile .PHONY: vet exists, lint does not. Bump remaining draft-04
  doc comments to draft-05.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Non-functional tidy-ups:

- cactus-cli: remove the hand-rolled min() (Go builtin) and the hexs()
  reimplementation of hex.EncodeToString.
- cmd/cactus: drop the pointless pemDecode wrapper; call pem.Decode.
- mirrorpush: pushEntries never used its note parameter; remove it.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
flush() derived the covering-subtree range from the tile writer's
pre-append size. When a flush advanced the tile writer (persisting the
treeSize file) but failed before committing the checkpoint — or a crash
landed in that window, which loadCheckpoint tolerates (tw.Size() may
exceed the checkpoint size) — the next flush saw newSize == tree size and
minted no covering subtree for the gap entries. Those entries became
committed with no signed subtree, so buildIssued fell through to the
signature-less whole-tree path and issuance for them failed (the issuer
rejects a zero-signature Issued; quorum mode hung until the deadline).

Cover [committedSize, newSize) using the last committed checkpoint size,
which re-covers the gap while remaining identical in the steady state,
and gate the no-op early return on the actual tree size. Also drop the
now-unused collectMirrorSigs parameters.

Adds a regression test that injects a checkpoint-write failure and
confirms the recovery flush produces a signed covering subtree.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
The §7.1.2.1 invalid-order filter only skipped when the order still
existed; also skip an ID whose order is gone. Currently unreachable
(OrderIDsForAccount only returns existing orders) but the guard now
reads as intended.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
Go 1.27 release candidates are out, so CI no longer needs to bootstrap
and cache a gotip devel toolchain. Both workflows now install
go1.27rc2 with a single actions/setup-go, which also handles the module
and build caches -- dropping the separate gotip SDK and Go cache steps.

The version is pinned in a GO_VERSION env var in each workflow so
moving to a later RC, or to '1.27' once 1.27.0 ships, is a one-line
edit. GOTOOLCHAIN=local stays: the RC satisfies the `go 1.27` go.mod
requirement on its own.

The Makefile still defaults to GO ?= gotip for local development, so
the CI steps pass GO=go explicitly.

Verified against go1.27rc2 locally: gofmt, vet, build, `go test -race
./...`, the integration suite, the stress-tagged vet, and `make stress`
all pass.

Claude-Session: https://claude.ai/code/session_01LgyCGiwxZCkSXxgy3PqfPC
@mcpherrinm
mcpherrinm merged commit 4e88979 into main Jul 21, 2026
2 checks passed
@mcpherrinm
mcpherrinm deleted the review-fixes branch July 21, 2026 14:05
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