From 5ae17e9cb0fdacffd37f6b1c9399bb5458dfa9ef Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:52:03 +0900 Subject: [PATCH 01/12] chore: add strict golangci-lint bootstrap --- .agent-tasks/epaxos-conflict-gc/GOALS.md | 25 +++++++++++++++++++++++ .github/workflows/ci.yml | 5 +++++ .golangci.yml | 26 ++++++++++++++++++++++++ AGENTS.md | 13 ++++++++++++ 4 files changed, 69 insertions(+) create mode 100644 .agent-tasks/epaxos-conflict-gc/GOALS.md create mode 100644 .golangci.yml create mode 100644 AGENTS.md diff --git a/.agent-tasks/epaxos-conflict-gc/GOALS.md b/.agent-tasks/epaxos-conflict-gc/GOALS.md new file mode 100644 index 0000000..bc95fa2 --- /dev/null +++ b/.agent-tasks/epaxos-conflict-gc/GOALS.md @@ -0,0 +1,25 @@ +# EPaxos conflict engine and executed-instance GC + +## Goal + +Replace the $O(\text{all instances})$ conflict and attrs machinery with a per-lane conflict engine, add two-tier executed-instance retirement with an asynchronous record-load handshake, expose `VisitConflicts`, and enforce strict linting. + +## Success criteria + +- R1: Attribute computation and TryPreAccept conflict checks avoid resident-instance scans. +- R2: The conflict index remains coherent for every record mutation and removal. +- R3: Embeddings receive a minimal, zero-allocation public conflict query API. +- R4: Executed instances retire automatically with configurable per-lane retention. +- R5: Payload-drop and fold reclamation bound resident memory, with proposal backpressure. +- R6: Wire format and durable storage remain unchanged; durable compaction remains separate. +- R7: Folded-record recovery uses a deterministic asynchronous Ready handshake. +- R8: Property tests, invariants, TLA evidence, and resident-state benchmarks prove behavior. +- R9: Strict golangci-lint, CI enforcement, and task-goal scaffolding are in place. +- R10: The work is delivered as atomic GitHub issues and PRs targeting `main`. + +## Verifiable-goals scaffolding + +- [Conflict engine tests](../../epaxos/conflict_engine_test.go) +- [Retirement tests](../../epaxos/retire_test.go) + +Remove this task-goal file after the final implementation unit merges. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d7099dc..9ab46cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,5 +52,10 @@ jobs: chmod +x /tmp/lein echo /tmp >> "$GITHUB_PATH" + - name: Run golangci-lint + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 + with: + version: v2.12.2 + - name: Run repository verification gates run: tests/ci.sh diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..cecfa3a --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,26 @@ +version: "2" + +run: + go: "1.26" + +linters: + enable: + - errcheck + - errorlint + - exhaustive + - gocritic + - gosec + - govet + - ineffassign + - revive + - staticcheck + - unused + - wastedassign + settings: + exhaustive: + default-signifies-exhaustive: false + govet: + enable-all: true + staticcheck: + checks: + - all diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..4690c49 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,13 @@ +# Agent Instructions + +Run these commands before completing relevant changes: + +```sh +go test -race -count=1 ./... +go vet ./... +golangci-lint run ./... +tests/ci.sh +``` + +Per-task goals live in `.agent-tasks//GOALS.md`. +Richer project conventions are init's job. From 77ddaae1a15c599b95397210ba084a3901016ec2 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:01:06 +0900 Subject: [PATCH 02/12] chore: use default govet and keep correctness linters Drop govet enable-all; keep revive/gosec/exhaustive enabled for R9. --- .golangci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index cecfa3a..f91fbbf 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,6 +4,9 @@ run: go: "1.26" linters: + # govet: DEFAULT analyzer set only. Do NOT set enable-all (fieldalignment/shadow noise). + # All of errcheck, errorlint, exhaustive, gocritic, gosec, govet, ineffassign, revive, + # staticcheck, unused, wastedassign stay enabled — R9 correctness/style gate. enable: - errcheck - errorlint @@ -19,8 +22,6 @@ linters: settings: exhaustive: default-signifies-exhaustive: false - govet: - enable-all: true staticcheck: checks: - all From 8bbbeab58061e83f1a3b7587bb95f9452e6331ca Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:53:01 +0900 Subject: [PATCH 03/12] style: remediate epaxos golangci findings Make epaxos package lint-clean under the U1 strict config: errcheck on Tick paths, revive renames, intentional test nolints for G115/G304/exhaustive subsets, and leave production wire G115 checks intact. --- epaxos/bootstrap.go | 19 +++- epaxos/bootstrap_core_test.go | 4 +- epaxos/bootstrap_recovery_test.go | 2 +- epaxos/branch_test.go | 36 +++++-- epaxos/codec.go | 30 ++++-- epaxos/codec_evidence_scratch_test.go | 14 +++ epaxos/dst_test.go | 18 ++-- epaxos/exhaustion_test.go | 14 +-- epaxos/fault_campaign_test.go | 34 +++---- epaxos/faultsim_harness_test.go | 104 +++++++++---------- epaxos/faultsim_oracle_test.go | 28 +++--- epaxos/faultsim_trace_test.go | 34 +++---- epaxos/hard_state_ready_test.go | 24 +++-- epaxos/internal_test.go | 24 +++-- epaxos/malformed_fuzz_test.go | 4 +- epaxos/message.go | 19 +++- epaxos/node.go | 42 ++++++-- epaxos/optimized_test.go | 4 +- epaxos/performance_benchmark_test.go | 12 ++- epaxos/pool_ownership_test.go | 22 ++-- epaxos/protocol_coverage_test.go | 58 ++++++++--- epaxos/recovery_boundary_test.go | 20 ++-- epaxos/recovery_test.go | 139 +++++++++++++++++--------- epaxos/remaining_test.go | 112 +++++++++++++++------ epaxos/revisited_test.go | 6 +- epaxos/sim_test.go | 34 ++++--- epaxos/sparse_progress.go | 6 +- epaxos/storage.go | 1 + epaxos/stress_test.go | 20 ++-- epaxos/toq_test.go | 18 +++- epaxos/types.go | 6 +- epaxos/vote_set.go | 2 +- epaxos/vote_set_test.go | 4 +- 33 files changed, 591 insertions(+), 323 deletions(-) diff --git a/epaxos/bootstrap.go b/epaxos/bootstrap.go index e0e0775..19cc30c 100644 --- a/epaxos/bootstrap.go +++ b/epaxos/bootstrap.go @@ -130,7 +130,7 @@ func validateBootstrapFrontier(f BootstrapFrontier, base ConfState) error { if coverage > InstanceNum(maxBootstrapSparseRefs-totalCoverage) { return ErrBootstrapBounds } - totalCoverage += int(coverage) + totalCoverage += int(coverage) //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. for j, instance := range lane.Sparse { if instance == 0 || instance <= lane.CompactedExecutedThrough || instance > lane.ObservedThrough || (j > 0 && lane.Sparse[j-1] >= instance) { @@ -340,6 +340,7 @@ func (c ReadyCertificate) Clone() ReadyCertificate { // BootstrapPhase is the durable phase of a voter plan. type BootstrapPhase uint8 +// BootstrapPhase values describe voter-plan progress. const ( BootstrapPhaseUnspecified BootstrapPhase = iota BootstrapPhasePreparing @@ -356,6 +357,7 @@ const ( // BootstrapOutcome is a terminal replicated membership-control outcome. type BootstrapOutcome uint8 +// BootstrapOutcome values describe terminal bootstrap results. const ( BootstrapOutcomeUnspecified BootstrapOutcome = iota BootstrapOutcomeActivated @@ -424,6 +426,7 @@ func (r BootstrapRecord) Clone() BootstrapRecord { // LocalVoterStatus is the durable local admission state. type LocalVoterStatus uint8 +// LocalVoterStatus values describe local admission eligibility. const ( LocalVoterStatusUnspecified LocalVoterStatus = iota LocalVoterStatusStaged @@ -523,6 +526,7 @@ type BootstrapStatusSnapshot struct { // BootstrapExit selects one reserved terminal control ref for recovery. type BootstrapExit uint8 +// BootstrapExit values select reserved terminal controls. const ( BootstrapExitActivate BootstrapExit = iota + 1 BootstrapExitAbort @@ -531,6 +535,7 @@ const ( // BootstrapMessageType identifies an authenticated out-of-band bootstrap message. type BootstrapMessageType uint8 +// BootstrapMessageType values identify bootstrap control messages. const ( BootstrapMsgFenceRequest BootstrapMessageType = iota + 1 BootstrapMsgFenceAck @@ -1405,6 +1410,8 @@ func validateBootstrapRecord(record BootstrapRecord) error { if record.Outcome != BootstrapOutcomeAborted || record.TerminalRef != record.Plan.Reservations.Abort { return ErrBootstrapControl } + case BootstrapPhaseUnspecified, BootstrapPhasePreparing, BootstrapPhasePrepared, BootstrapPhaseLocalFenced, BootstrapPhaseFenced, BootstrapPhaseCertified, BootstrapPhaseTargetReady, BootstrapPhaseFinalizing: + fallthrough default: if record.Outcome != BootstrapOutcomeUnspecified || !record.TerminalRef.IsZero() { return ErrBootstrapControl @@ -1455,6 +1462,8 @@ func validateMembershipResult(record InstanceRecord) error { if !confStateIsZero(result.Successor) || record.ConfChangeResult.Outcome == ConfChangeApplied { return fmt.Errorf("%w: rejected membership result has successor", ErrInvalidConfig) } + case BootstrapOutcomeUnspecified: + fallthrough default: return fmt.Errorf("%w: unknown membership outcome", ErrInvalidConfig) } @@ -1523,7 +1532,7 @@ func DecodeBootstrapMessage(src []byte, message *BootstrapMessage) error { if p.uvarint() != bootstrapWireVersion { return ErrInvalidBootstrapMessage } - message.Type = BootstrapMessageType(p.uvarint()) + message.Type = BootstrapMessageType(p.uvarint()) //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. p.fixed((*[32]byte)(&message.Cluster)) p.fixed((*[32]byte)(&message.Plan)) message.From = ReplicaID(p.uvarint()) @@ -1666,9 +1675,9 @@ func (p *bootstrapParser) fixed(dst *[32]byte) { p.b = p.b[len(dst):] } -func (p *bootstrapParser) bytes(max int) []byte { +func (p *bootstrapParser) bytes(maxLen int) []byte { length := p.uvarint() - if p.err || length > uint64(max) || length > uint64(len(p.b)) { + if p.err || length > uint64(maxLen) || length > uint64(len(p.b)) { //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. p.err = true return nil } @@ -2823,6 +2832,7 @@ func (n *RawNode) admitWhileFenced(message Message) error { if message.Command.Kind == CommandUser && len(message.Command.Payload) == 0 { return nil } + case MsgPreAccept, MsgAccept, MsgCommit, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidence, MsgEvidenceResp: } wire, err := decodeMembershipCommand(message.Command) if err != nil || wire.Operation != operation || wire.Plan.Request.Plan != state.record.Plan.Request.Plan { @@ -2858,6 +2868,7 @@ func (n *RawNode) admitWhileFenced(message Message) error { if inst == nil { return ErrBootstrapContradiction } + case MsgPreAcceptResp, MsgAcceptResp, MsgPrepareResp, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidence, MsgEvidenceResp: } return nil } diff --git a/epaxos/bootstrap_core_test.go b/epaxos/bootstrap_core_test.go index 739c642..d36442a 100644 --- a/epaxos/bootstrap_core_test.go +++ b/epaxos/bootstrap_core_test.go @@ -21,9 +21,9 @@ func newBootstrapTestFixture(t *testing.T, voters int, local ReplicaID) bootstra for i := range identities { identities[i] = VoterIdentity{Replica: ReplicaID(i + 1), Incarnation: 1} } - target := VoterIdentity{Replica: ReplicaID(voters + 1), Incarnation: 1} + target := VoterIdentity{Replica: ReplicaID(voters + 1), Incarnation: 1} //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. cluster := ClusterID{1, 2, 3} - planID := BootstrapID{9, byte(voters), byte(local)} + planID := BootstrapID{9, byte(voters), byte(local)} //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. store := NewMemoryStorage() store.Hard = HardState{Conf: conf.Clone()} store.ConfigHistory = []ConfigHistoryEntry{{Conf: conf.Clone()}} diff --git a/epaxos/bootstrap_recovery_test.go b/epaxos/bootstrap_recovery_test.go index 5ebd18a..b3e73cd 100644 --- a/epaxos/bootstrap_recovery_test.go +++ b/epaxos/bootstrap_recovery_test.go @@ -772,7 +772,7 @@ func TestDueRecoveryTimersShareBoundedFairDriveBudget(t *testing.T) { func TestLiveOldQuorumAfterFencerCrashesCanFinalizeButCannotChooseOrdinaryWork(t *testing.T) { for voters := 3; voters <= 6; voters++ { - t.Run(string(rune('0'+voters)), func(t *testing.T) { + t.Run(string(rune('0'+voters)), func(t *testing.T) { //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. fixture, plan := standaloneBootstrapPlan(t, voters) frontier := standaloneFrontier(plan, 0) quorum := slowQuorumSize(voters) diff --git a/epaxos/branch_test.go b/epaxos/branch_test.go index 1f068e1..518e813 100644 --- a/epaxos/branch_test.go +++ b/epaxos/branch_test.go @@ -110,13 +110,21 @@ func TestDirectProtocolBranches(t *testing.T) { t.Fatal(err) } inst := rn.instances[ref2] - rn.handlePreAcceptResp(Message{Type: MsgPreAcceptResp, From: 2, To: 1, Ref: ref2, Ballot: Ballot{Number: 5, Replica: 2}, Reject: true, RejectHint: Ballot{Number: 5, Replica: 2}, Deps: rn.q.deps()}) + if err := rn.handlePreAcceptResp(Message{Type: MsgPreAcceptResp, From: 2, To: 1, Ref: ref2, Ballot: Ballot{Number: 5, Replica: 2}, Reject: true, RejectHint: Ballot{Number: 5, Replica: 2}, Deps: rn.q.deps()}); err != nil { + panic(err) + } if inst.phase != phasePrepare { t.Fatal("reject did not start prepare") } - rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: ref2, RecordStatus: StatusAccepted, Ballot: Ballot{Number: 6, Replica: 2}, RecordBallot: Ballot{Number: 6, Replica: 2}, Seq: 3, Deps: rn.q.deps(), Command: inst.rec.Command}) - rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: ref2, RecordStatus: StatusAccepted, Ballot: Ballot{Number: 6, Replica: 2}, RecordBallot: Ballot{Number: 6, Replica: 2}, Seq: 3, Deps: rn.q.deps(), Command: inst.rec.Command}) - rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 3, To: 1, Ref: ref2, RecordStatus: StatusCommitted, Ballot: Ballot{Number: 7, Replica: 3}, RecordBallot: Ballot{Number: 7, Replica: 3}, Seq: 4, Deps: rn.q.deps(), Command: inst.rec.Command}) + if err := rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: ref2, RecordStatus: StatusAccepted, Ballot: Ballot{Number: 6, Replica: 2}, RecordBallot: Ballot{Number: 6, Replica: 2}, Seq: 3, Deps: rn.q.deps(), Command: inst.rec.Command}); err != nil { + panic(err) + } + if err := rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: ref2, RecordStatus: StatusAccepted, Ballot: Ballot{Number: 6, Replica: 2}, RecordBallot: Ballot{Number: 6, Replica: 2}, Seq: 3, Deps: rn.q.deps(), Command: inst.rec.Command}); err != nil { + panic(err) + } + if err := rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 3, To: 1, Ref: ref2, RecordStatus: StatusCommitted, Ballot: Ballot{Number: 7, Replica: 3}, RecordBallot: Ballot{Number: 7, Replica: 3}, Seq: 4, Deps: rn.q.deps(), Command: inst.rec.Command}); err != nil { + panic(err) + } rn.startAccept(inst, inst.rec.Attributes()) rn.commit(inst, inst.rec.Attributes()) } @@ -132,13 +140,19 @@ func TestAcceptResponseAndConfigBranches(t *testing.T) { } inst := rn.instances[ref] rn.startAccept(inst, inst.rec.Attributes()) - rn.handleAcceptResp(Message{Type: MsgAcceptResp, From: 2, To: 1, Ref: ref, Ballot: Ballot{Number: 9, Replica: 2}, Reject: true, RejectHint: Ballot{Number: 9, Replica: 2}, Deps: rn.q.deps()}) + if err := rn.handleAcceptResp(Message{Type: MsgAcceptResp, From: 2, To: 1, Ref: ref, Ballot: Ballot{Number: 9, Replica: 2}, Reject: true, RejectHint: Ballot{Number: 9, Replica: 2}, Deps: rn.q.deps()}); err != nil { + panic(err) + } inst.phase = phaseAccept inst.rec.Status = StatusAccepted inst.rec.RecordBallot = inst.rec.Ballot inst.accOK = 0 - rn.handleAcceptResp(Message{Type: MsgAcceptResp, From: 2, To: 1, Ref: ref, Ballot: inst.rec.Ballot, RecordBallot: inst.rec.Ballot, RecordStatus: StatusAccepted, Seq: inst.rec.Seq, Deps: rn.q.deps()}) - rn.handleAcceptResp(Message{Type: MsgAcceptResp, From: 2, To: 1, Ref: ref, Ballot: inst.rec.Ballot, RecordBallot: inst.rec.Ballot, RecordStatus: StatusAccepted, Seq: inst.rec.Seq, Deps: rn.q.deps()}) + if err := rn.handleAcceptResp(Message{Type: MsgAcceptResp, From: 2, To: 1, Ref: ref, Ballot: inst.rec.Ballot, RecordBallot: inst.rec.Ballot, RecordStatus: StatusAccepted, Seq: inst.rec.Seq, Deps: rn.q.deps()}); err != nil { + panic(err) + } + if err := rn.handleAcceptResp(Message{Type: MsgAcceptResp, From: 2, To: 1, Ref: ref, Ballot: inst.rec.Ballot, RecordBallot: inst.rec.Ballot, RecordStatus: StatusAccepted, Seq: inst.rec.Seq, Deps: rn.q.deps()}); err != nil { + panic(err) + } rn.enqueueRecord(InstanceRecord{Ref: InstanceRef{Replica: 1, Instance: 99, Conf: 1}, Deps: rn.q.deps(), Command: Command{Kind: CommandNoop}}) rn.enqueueMessage(Message{Type: MsgPrepareResp, From: 1, To: 2, Ref: ref, Ballot: Ballot{Number: 1, Replica: 2}, Deps: rn.q.deps()}) if _, err := FastQuorum(0); err == nil { @@ -183,7 +197,9 @@ func TestRestartAcceptedForeignInstanceSchedulesPrepareRecovery(t *testing.T) { } advanceOK(t, rn, initial) for tick := uint64(1); tick < 4; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + panic(err) + } tickReady := rn.Ready() wantTick := HardState{Conf: wantInitial.Conf, Tick: tick} if !tickReady.HardState.Equal(wantTick) || !tickReady.MustSync || @@ -196,7 +212,9 @@ func TestRestartAcceptedForeignInstanceSchedulesPrepareRecovery(t *testing.T) { advanceOK(t, rn, tickReady) } - rn.Tick() + if err := rn.Tick(); err != nil { + panic(err) + } if inst.phase != phasePrepare { t.Fatalf("foreign accepted recovery phase = %d, want prepare after recovery deadline", inst.phase) } diff --git a/epaxos/codec.go b/epaxos/codec.go index 5468241..b55bd57 100644 --- a/epaxos/codec.go +++ b/epaxos/codec.go @@ -11,8 +11,8 @@ const ( maxWireDeps = 128 maxWireConflictKeys = 128 maxWireAcceptEvidence = 128 - maxWireCommandPayload = 1 << 20 - maxWireConflictKey = 1 << 16 + maxWireCommandPayload = 1 << 20 + maxWireConflictKey = 1 << 16 ) // DecodeScratch owns reusable metadata buffers for DecodeMessageWithScratch. @@ -200,7 +200,7 @@ func decodeMessage(src []byte, m *Message, scratch *DecodeScratch) error { return decodeMessageError(m, scratch, ErrInvalidMessage) } p := parser{b: src[len(wireMagic) : len(src)-32], scratch: scratch} - m.Type = MessageType(p.uvarint()) + m.Type = MessageType(p.uvarint8()) m.From = ReplicaID(p.uvarint()) m.To = ReplicaID(p.uvarint()) m.Ref = InstanceRef{Replica: ReplicaID(p.uvarint()), Instance: InstanceNum(p.uvarint()), Conf: ConfID(p.uvarint())} @@ -254,7 +254,7 @@ func decodeMessage(src []byte, m *Message, scratch *DecodeScratch) error { for i := range m.AcceptEvidence { m.AcceptEvidence[i].Sender = ReplicaID(p.uvarint()) m.AcceptEvidence[i].Seq = p.uvarint() - deps := int(p.uvarint()) + deps := int(p.uvarint()) //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. if scratch != nil { end := evidenceOffset + deps m.AcceptEvidence[i].Deps = evidenceArena[evidenceOffset:end:end] @@ -270,12 +270,12 @@ func decodeMessage(src []byte, m *Message, scratch *DecodeScratch) error { m.Command = p.command() m.Reject = p.byte() == 1 m.RejectHint = Ballot{Epoch: p.uvarint(), Number: p.uvarint(), Replica: ReplicaID(p.uvarint())} - m.RejectReason = RejectReason(p.uvarint()) + m.RejectReason = RejectReason(p.uvarint8()) m.ConflictRef = InstanceRef{Replica: ReplicaID(p.uvarint()), Instance: InstanceNum(p.uvarint()), Conf: ConfID(p.uvarint())} - m.ConflictStatus = Status(p.uvarint()) + m.ConflictStatus = Status(p.uvarint8()) m.FastPathEligible = p.byte() == 1 m.DepsCommitted = p.uvarint() - m.RecordStatus = Status(p.uvarint()) + m.RecordStatus = Status(p.uvarint8()) if p.err || len(p.b) != 0 { return decodeMessageError(m, scratch, ErrInvalidMessage) } @@ -347,6 +347,15 @@ func (p *parser) uvarint() uint64 { return v } +func (p *parser) uvarint8() uint8 { + v := p.uvarint() + if v > uint64(^uint8(0)) { + p.err = true + return 0 + } + return uint8(v) +} + func (p *parser) byte() byte { if len(p.b) == 0 { p.err = true @@ -357,9 +366,9 @@ func (p *parser) byte() byte { return v } -func (p *parser) bytesBound(max uint64) []byte { +func (p *parser) bytesBound(maxLen uint64) []byte { n := p.uvarint() - if n > max || n > uint64(len(p.b)) { + if n > maxLen || n > uint64(len(p.b)) { p.err = true return nil } @@ -371,9 +380,8 @@ func (p *parser) bytes() []byte { return p.bytesBound(^uint64(0)) } - func (p *parser) command() Command { - c := Command{ID: CommandID{Client: p.uvarint(), Sequence: p.uvarint()}, Kind: CommandKind(p.uvarint())} + c := Command{ID: CommandID{Client: p.uvarint(), Sequence: p.uvarint()}, Kind: CommandKind(p.uvarint8())} c.Payload = p.bytesBound(maxWireCommandPayload) keys := p.uvarint() if keys > maxWireConflictKeys { diff --git a/epaxos/codec_evidence_scratch_test.go b/epaxos/codec_evidence_scratch_test.go index 73ef58c..ce9a26e 100644 --- a/epaxos/codec_evidence_scratch_test.go +++ b/epaxos/codec_evidence_scratch_test.go @@ -7,6 +7,20 @@ import ( "testing" ) +func TestDecodeMessageRejectsOversizedMessageType(t *testing.T) { + valid, _ := evidenceScratchTestMessages() + valid.Type = MessageType(1) + encoded := mustEncodeMessageSeed(valid) + frame := append([]byte(nil), encoded[:len(wireMagic)]...) + frame = binary.AppendUvarint(frame, 257) + frame = append(frame, encoded[len(wireMagic)+1:]...) + + var got Message + if err := DecodeMessage(frame, &got); !errors.Is(err, ErrInvalidMessage) { + t.Fatalf("oversized message type error = %v, want ErrInvalidMessage", err) + } +} + func TestDecodeMessageWithScratchReusesAcceptEvidenceWithoutAllocation(t *testing.T) { large, small := evidenceScratchTestMessages() largeEncoded := mustEncodeMessageSeed(large) diff --git a/epaxos/dst_test.go b/epaxos/dst_test.go index c50c29c..0cd2374 100644 --- a/epaxos/dst_test.go +++ b/epaxos/dst_test.go @@ -230,7 +230,7 @@ func TestDSTFailureBoundarySlowQuorumSizesOneThroughSeven(t *testing.T) { s.omit(id) } - cmd := dstPut(1000+uint64(tc.n), 1, "slow-quorum-"+strconv.Itoa(tc.n), "progress") + cmd := dstPut(1000+uint64(tc.n), 1, "slow-quorum-"+strconv.Itoa(tc.n), "progress") //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. ref, err := s.nodes[1].Propose(cmd) if err != nil { t.Fatal(err) @@ -266,7 +266,7 @@ func TestDSTFailureBoundarySlowQuorumSizesOneThroughSeven(t *testing.T) { s.omit(id) } - cmd := dstPut(2000+uint64(tc.n), 1, "slow-quorum-"+strconv.Itoa(tc.n), "after-heal") + cmd := dstPut(2000+uint64(tc.n), 1, "slow-quorum-"+strconv.Itoa(tc.n), "after-heal") //nolint:gosec // G115: test harness converts bounded int index/count ref, err := s.nodes[1].Propose(cmd) if err != nil { t.Fatal(err) @@ -301,7 +301,7 @@ func TestDSTStorageFailureBoundarySlowQuorumSizesOneThroughSeven(t *testing.T) { s.stores[id].FailWrites = true } - cmd := dstPut(3000+uint64(n*10+failedCount), 1, "storage-boundary-"+strconv.Itoa(n), "progress-"+strconv.Itoa(failedCount)) + cmd := dstPut(3000+uint64(n*10+failedCount), 1, "storage-boundary-"+strconv.Itoa(n), "progress-"+strconv.Itoa(failedCount)) //nolint:gosec // G115: test harness converts bounded int index/count ref, err := s.nodes[1].Propose(cmd) if err != nil { t.Fatal(err) @@ -334,7 +334,7 @@ func TestDSTStorageFailureBoundarySlowQuorumSizesOneThroughSeven(t *testing.T) { s.stores[id].FailWrites = true } - cmd := dstPut(4000+uint64(n), 1, "storage-boundary-"+strconv.Itoa(n), "after-restore") + cmd := dstPut(4000+uint64(n), 1, "storage-boundary-"+strconv.Itoa(n), "after-restore") //nolint:gosec // G115: test harness converts bounded int index/count ref, err := s.nodes[1].Propose(cmd) if err != nil { t.Fatal(err) @@ -701,7 +701,7 @@ func dstHighestReplicaIDs(n, count int) []ReplicaID { } ids := make([]ReplicaID, 0, count) for id := n - count + 1; id <= n; id++ { - ids = append(ids, ReplicaID(id)) + ids = append(ids, ReplicaID(id)) //nolint:gosec // G115: test harness converts bounded int index/count } return ids } @@ -880,9 +880,7 @@ func dstDriveAllowingStorageFailures(t *testing.T, s *simCluster, rounds int) in blockedWrites++ continue } - for _, c := range rd.Committed { - s.apps[id] = append(s.apps[id], c) - } + s.apps[id] = append(s.apps[id], rd.Committed...) for _, m := range rd.Messages { if !s.deliver(m) { s.delayed = append(s.delayed, m) @@ -902,7 +900,9 @@ func dstTickAllAllowingStorageFailures(t *testing.T, s *simCluster, ticks int) i for range ticks { for _, id := range s.ids() { if !s.paused[id] { - s.nodes[id].Tick() + if err := s.nodes[id].Tick(); err != nil { + panic(err) + } } } blockedWrites += dstDriveAllowingStorageFailures(t, s, 100) diff --git a/epaxos/exhaustion_test.go b/epaxos/exhaustion_test.go index c4a9836..454fffe 100644 --- a/epaxos/exhaustion_test.go +++ b/epaxos/exhaustion_test.go @@ -90,8 +90,8 @@ func TestRestartAtMaximumLocalInstanceRemainsExhausted(t *testing.T) { } func TestRecoveryTicksDefaultMultiplicationOverflow(t *testing.T) { - max := ^uint64(0) - exact := max / 3 + maxTick := ^uint64(0) + exact := maxTick / 3 rn, err := NewRawNode(Config{ID: 1, Voters: makeIDs(3), RetryTicks: exact}) if err != nil { t.Fatalf("exact representable default RecoveryTicks: %v", err) @@ -106,7 +106,7 @@ func TestRecoveryTicksDefaultMultiplicationOverflow(t *testing.T) { } func TestTimeOptimizationProcessAtCheckedBeforeMutation(t *testing.T) { - max := ^uint64(0) + maxTick := ^uint64(0) exact, err := NewRawNode(Config{ ID: 1, Voters: makeIDs(3), @@ -119,15 +119,15 @@ func TestTimeOptimizationProcessAtCheckedBeforeMutation(t *testing.T) { t.Fatal(err) } remainingApplyHardStateOnly(t, exact, 0) - exact.tick = max - 2 + exact.tick = maxTick - 2 exact.currentHardState.Tick = exact.tick exact.acknowledgedHardState = exact.currentHardState.Clone() ref, err := exact.Propose(Command{ID: CommandID{Client: 303, Sequence: 1}, Payload: []byte("exact-max"), ConflictKeys: [][]byte{[]byte("exact-max-key")}}) if err != nil { t.Fatalf("exact-Max ProcessAt proposal: %v", err) } - if got := exact.instances[ref].rec.ProcessAt; got != max { - t.Fatalf("exact-Max ProcessAt=%d, want %d", got, max) + if got := exact.instances[ref].rec.ProcessAt; got != maxTick { + t.Fatalf("exact-Max ProcessAt=%d, want %d", got, maxTick) } overflow, err := NewRawNode(Config{ @@ -142,7 +142,7 @@ func TestTimeOptimizationProcessAtCheckedBeforeMutation(t *testing.T) { t.Fatal(err) } remainingApplyHardStateOnly(t, overflow, 0) - overflow.tick = max - 2 + overflow.tick = maxTick - 2 overflow.currentHardState.Tick = overflow.tick overflow.acknowledgedHardState = overflow.currentHardState.Clone() before := snapshotRemainingNodeProtocol(overflow) diff --git a/epaxos/fault_campaign_test.go b/epaxos/fault_campaign_test.go index 4da1dec..2f4ff80 100644 --- a/epaxos/fault_campaign_test.go +++ b/epaxos/fault_campaign_test.go @@ -24,26 +24,26 @@ var faultCampaignDimensions = []string{ } type faultCampaignCell struct { - Size int - Dimension string - Disposition string - Reason string - Counter string + Size int + Dimension string + Disposition string + Reason string + Counter string } func faultCampaignManifest() []faultCampaignCell { cells := make([]faultCampaignCell, 0, 7*len(faultCampaignDimensions)) counter := map[string]string{ - "loss": "drop", - "duplicate": "duplicate", - "reorder": "reorder", - "asymmetric-partition": "partition", - "crash-restart": "crash", - "storage-failure": "storage-failure", - "clock-rollback": "clock-rollback", - "malformed-input": "malformed", + "loss": "drop", + "duplicate": "duplicate", + "reorder": "reorder", + "asymmetric-partition": "partition", + "crash-restart": "crash", + "storage-failure": "storage-failure", + "clock-rollback": "clock-rollback", + "malformed-input": "malformed", "membership-transition": "membership", - "bounded-overload": "overload", + "bounded-overload": "overload", } for size := 1; size <= 7; size++ { for _, dimension := range faultCampaignDimensions { @@ -98,12 +98,12 @@ func faultCampaignSeed(base uint64, cell faultCampaignCell) uint64 { hash ^= uint64(cell.Dimension[index]) hash *= 1099511628211 } - return base ^ uint64(cell.Size)*0x9e3779b97f4a7c15 ^ hash + return base ^ uint64(cell.Size)*0x9e3779b97f4a7c15 ^ hash //nolint:gosec // G115: test harness converts bounded int index/count } func faultCampaignOperation(size int, sequence uint64, suffix string) faultClientOperation { return faultClientOperation{ - Client: uint64(size*1000) + sequence, + Client: uint64(size*1000) + sequence, //nolint:gosec // G115: test harness converts bounded int index/count Sequence: sequence, Kind: "put", Writes: []faultKV{{Key: "campaign-key", Value: fmt.Sprintf("N%d-%s-%d", size, suffix, sequence)}}, @@ -333,7 +333,7 @@ func faultRunCampaignCell(t *testing.T, cell faultCampaignCell, seed uint64) (*f Reason: "safe add-voter bootstrap/catch-up is not exposed by the public API; this cell exercises only the supported remove transition", }) } - removed := ReplicaID(cell.Size) + removed := ReplicaID(cell.Size) //nolint:gosec // G115: test harness converts bounded int index/count faultMustAction(t, h, faultSimAction{Kind: faultActionPartition, From: 1, To: removed}) change := ConfChange{Type: ConfChangeRemoveVoter, Replica: removed} receipt := faultMustAction(t, h, faultSimAction{Kind: faultActionConfChange, Node: 1, ConfChange: &change}) diff --git a/epaxos/faultsim_harness_test.go b/epaxos/faultsim_harness_test.go index 9fcd909..3db03d9 100644 --- a/epaxos/faultsim_harness_test.go +++ b/epaxos/faultsim_harness_test.go @@ -43,12 +43,12 @@ const ( type faultCrashCut string const ( - faultCrashBeforeReady faultCrashCut = "before-ready" - faultCrashAfterFrozenReady faultCrashCut = "after-frozen-ready-before-persistence" - faultCrashAfterPersistence faultCrashCut = "after-records-hard-state-before-application" - faultCrashAfterApplication faultCrashCut = "after-application-before-advance" - faultCrashAfterAdvance faultCrashCut = "after-advance-before-executed-persistence" - faultCrashAfterExecutedPersistence faultCrashCut = "after-executed-persistence" + faultCrashBeforeReady faultCrashCut = "before-ready" + faultCrashAfterFrozenReady faultCrashCut = "after-frozen-ready-before-persistence" + faultCrashAfterPersistence faultCrashCut = "after-records-hard-state-before-application" + faultCrashAfterApplication faultCrashCut = "after-application-before-advance" + faultCrashAfterAdvance faultCrashCut = "after-advance-before-executed-persistence" + faultCrashAfterExecutedPersistence faultCrashCut = "after-executed-persistence" ) type faultKV struct { @@ -160,8 +160,8 @@ func (r faultOperationResponse) equal(other faultOperationResponse) bool { } type faultApplicationEvent struct { - Ref InstanceRef `json:"ref"` - Command Command `json:"command"` + Ref InstanceRef `json:"ref"` + Command Command `json:"command"` Response faultOperationResponse `json:"response,omitempty"` } @@ -232,12 +232,12 @@ func (a *faultApplicationImage) apply(committed []CommittedCommand) ([]faultAppl } type faultHistoryOperation struct { - Operation faultClientOperation `json:"operation"` - Proposer ReplicaID `json:"proposer"` - Ref InstanceRef `json:"ref"` - Invoke uint64 `json:"invoke"` - Complete uint64 `json:"complete,omitempty"` - Result string `json:"result"` + Operation faultClientOperation `json:"operation"` + Proposer ReplicaID `json:"proposer"` + Ref InstanceRef `json:"ref"` + Invoke uint64 `json:"invoke"` + Complete uint64 `json:"complete,omitempty"` + Result string `json:"result"` Response faultOperationResponse `json:"response,omitempty"` } @@ -294,23 +294,23 @@ func (e faultEnvelope) clone() faultEnvelope { func (e faultEnvelope) bytes() []byte { return append([]byte(nil), e.wire...) } type faultSimAction struct { - Kind string `json:"kind"` - Node ReplicaID `json:"node,omitempty"` - Envelope uint64 `json:"envelope,omitempty"` - From ReplicaID `json:"from,omitempty"` - To ReplicaID `json:"to,omitempty"` - Count int `json:"count,omitempty"` - Clock uint64 `json:"clock,omitempty"` - Cut faultCrashCut `json:"cut,omitempty"` - Image string `json:"image,omitempty"` - Name string `json:"name,omitempty"` - Enabled bool `json:"enabled,omitempty"` - Mutation string `json:"mutation,omitempty"` - Reason string `json:"reason,omitempty"` - Data []byte `json:"data,omitempty"` - Operation *faultClientOperation `json:"operation,omitempty"` - ConfChange *ConfChange `json:"conf_change,omitempty"` - Required bool `json:"required,omitempty"` + Kind string `json:"kind"` + Node ReplicaID `json:"node,omitempty"` + Envelope uint64 `json:"envelope,omitempty"` + From ReplicaID `json:"from,omitempty"` + To ReplicaID `json:"to,omitempty"` + Count int `json:"count,omitempty"` + Clock uint64 `json:"clock,omitempty"` + Cut faultCrashCut `json:"cut,omitempty"` + Image string `json:"image,omitempty"` + Name string `json:"name,omitempty"` + Enabled bool `json:"enabled,omitempty"` + Mutation string `json:"mutation,omitempty"` + Reason string `json:"reason,omitempty"` + Data []byte `json:"data,omitempty"` + Operation *faultClientOperation `json:"operation,omitempty"` + ConfChange *ConfChange `json:"conf_change,omitempty"` + Required bool `json:"required,omitempty"` } func (a faultSimAction) clone() faultSimAction { @@ -356,22 +356,22 @@ type faultReplica struct { } type faultSimHarness struct { - cfg faultSimConfig - ids []ReplicaID - replicas map[ReplicaID]*faultReplica - envelopes map[uint64]faultEnvelope - envelopeState map[uint64]faultEnvelopeState - partitions map[[2]ReplicaID]bool - clocks map[ReplicaID]uint64 - nextEnvelope uint64 - step uint64 - actions []faultSimAction - receipts []faultActionReceipt - counters map[string]uint64 - history []faultHistoryOperation - historyByID map[CommandID]int - outstanding int - failClosed []faultFailClosedCheck + cfg faultSimConfig + ids []ReplicaID + replicas map[ReplicaID]*faultReplica + envelopes map[uint64]faultEnvelope + envelopeState map[uint64]faultEnvelopeState + partitions map[[2]ReplicaID]bool + clocks map[ReplicaID]uint64 + nextEnvelope uint64 + step uint64 + actions []faultSimAction + receipts []faultActionReceipt + counters map[string]uint64 + history []faultHistoryOperation + historyByID map[CommandID]int + outstanding int + failClosed []faultFailClosedCheck lastProgressBound int } @@ -1184,12 +1184,12 @@ func faultSameCommand(a, b Command) bool { } type faultTerminalRecord struct { - Node ReplicaID - Hard HardState - Config []ConfState + Node ReplicaID + Hard HardState + Config []ConfState Records []InstanceRecord - State []faultKV - Log []faultApplicationEvent + State []faultKV + Log []faultApplicationEvent } type faultTerminalEnvelope struct { diff --git a/epaxos/faultsim_oracle_test.go b/epaxos/faultsim_oracle_test.go index cc59fce..0337d1c 100644 --- a/epaxos/faultsim_oracle_test.go +++ b/epaxos/faultsim_oracle_test.go @@ -7,17 +7,17 @@ import ( ) type faultOracleResult struct { - OK bool `json:"ok"` - Linearizable bool `json:"linearizable"` - ChosenAgreement bool `json:"chosen_agreement"` - ExactlyOnce bool `json:"exactly_once_application"` - ConflictOrder bool `json:"converged_conflict_order"` - ConvergedState bool `json:"converged_state"` - MinorityFailClosed bool `json:"minority_fail_closed"` - DurableSlowQuorum bool `json:"durable_slow_quorum"` - HealedLiveness bool `json:"healed_liveness"` - LogicalStepBound int `json:"logical_step_bound,omitempty"` - Error string `json:"error,omitempty"` + OK bool `json:"ok"` + Linearizable bool `json:"linearizable"` + ChosenAgreement bool `json:"chosen_agreement"` + ExactlyOnce bool `json:"exactly_once_application"` + ConflictOrder bool `json:"converged_conflict_order"` + ConvergedState bool `json:"converged_state"` + MinorityFailClosed bool `json:"minority_fail_closed"` + DurableSlowQuorum bool `json:"durable_slow_quorum"` + HealedLiveness bool `json:"healed_liveness"` + LogicalStepBound int `json:"logical_step_bound,omitempty"` + Error string `json:"error,omitempty"` } func faultOracleExpectedNodes(h *faultSimHarness) []ReplicaID { @@ -534,12 +534,12 @@ func TestFaultCrashCutsReplayExactly(t *testing.T) { } for cutIndex, cut := range cuts { t.Run(string(cut), func(t *testing.T) { - h, err := newFaultSimHarness(faultSimConfig{Size: 1, Seed: uint64(100 + cutIndex)}) + h, err := newFaultSimHarness(faultSimConfig{Size: 1, Seed: uint64(100 + cutIndex)}) //nolint:gosec // G115: test harness converts bounded int index/count if err != nil { t.Fatal(err) } faultMustAction(t, h, faultSimAction{Kind: faultActionPump, Required: true}) - op := faultClientOperation{Client: 500 + uint64(cutIndex), Sequence: 1, Kind: "put", Writes: []faultKV{{Key: "cut", Value: string(cut)}}} + op := faultClientOperation{Client: 500 + uint64(cutIndex), Sequence: 1, Kind: "put", Writes: []faultKV{{Key: "cut", Value: string(cut)}}} //nolint:gosec // G115: test harness converts bounded int index/count faultMustAction(t, h, faultSimAction{Kind: faultActionPropose, Node: 1, Operation: &op}) faultMustAction(t, h, faultSimAction{Kind: faultActionCrash, Node: 1, Cut: cut}) refs := []InstanceRef{} @@ -547,7 +547,7 @@ func TestFaultCrashCutsReplayExactly(t *testing.T) { ref, _ := h.refFor(op.commandID()) refs = append(refs, ref) } - canary := faultClientOperation{Client: 900 + uint64(cutIndex), Sequence: 1, Kind: "put", Writes: []faultKV{{Key: "canary", Value: string(cut)}}} + canary := faultClientOperation{Client: 900 + uint64(cutIndex), Sequence: 1, Kind: "put", Writes: []faultKV{{Key: "canary", Value: string(cut)}}} //nolint:gosec // G115: test harness converts bounded int index/count faultMustAction(t, h, faultSimAction{Kind: faultActionPropose, Node: 1, Operation: &canary}) canaryRef, _ := h.refFor(canary.commandID()) refs = append(refs, canaryRef) diff --git a/epaxos/faultsim_trace_test.go b/epaxos/faultsim_trace_test.go index e2948cb..3df1b0b 100644 --- a/epaxos/faultsim_trace_test.go +++ b/epaxos/faultsim_trace_test.go @@ -17,16 +17,16 @@ import ( const faultTraceVersion = 1 type faultTrace struct { - Version int `json:"version"` - SourceRevision string `json:"source_revision"` - Config faultSimConfig `json:"config"` - Seed uint64 `json:"seed"` - Actions []faultSimAction `json:"actions"` + Version int `json:"version"` + SourceRevision string `json:"source_revision"` + Config faultSimConfig `json:"config"` + Seed uint64 `json:"seed"` + Actions []faultSimAction `json:"actions"` Operations []faultHistoryOperation `json:"operations"` - Receipts []faultActionReceipt `json:"receipts"` - Counters map[string]uint64 `json:"counters"` - TerminalHash string `json:"terminal_hash"` - Oracle faultOracleResult `json:"oracle"` + Receipts []faultActionReceipt `json:"receipts"` + Counters map[string]uint64 `json:"counters"` + TerminalHash string `json:"terminal_hash"` + Oracle faultOracleResult `json:"oracle"` } func faultSourceRevision() string { @@ -67,9 +67,9 @@ func faultRepositoryRevision() string { } for { gitDirectory := filepath.Join(directory, ".git") - head, headErr := os.ReadFile(filepath.Join(gitDirectory, "HEAD")) + head, headErr := os.ReadFile(filepath.Join(gitDirectory, "HEAD")) //nolint:gosec // G304: test fixture path is under repository control. if headErr != nil { - pointer, pointerErr := os.ReadFile(gitDirectory) + pointer, pointerErr := os.ReadFile(gitDirectory) //nolint:gosec // G304: test fixture path is under repository control. if pointerErr == nil { value := strings.TrimSpace(string(pointer)) if strings.HasPrefix(value, "gitdir: ") { @@ -77,7 +77,7 @@ func faultRepositoryRevision() string { if !filepath.IsAbs(gitDirectory) { gitDirectory = filepath.Join(directory, gitDirectory) } - head, headErr = os.ReadFile(filepath.Join(gitDirectory, "HEAD")) + head, headErr = os.ReadFile(filepath.Join(gitDirectory, "HEAD")) //nolint:gosec // G304: test fixture path is under repository control. } } } @@ -87,10 +87,10 @@ func faultRepositoryRevision() string { return value } reference := strings.TrimSpace(strings.TrimPrefix(value, "ref: ")) - if revision, err := os.ReadFile(filepath.Join(gitDirectory, filepath.FromSlash(reference))); err == nil { + if revision, err := os.ReadFile(filepath.Join(gitDirectory, filepath.FromSlash(reference))); err == nil { //nolint:gosec // G304: test opens controlled fixture path under temp dir return strings.TrimSpace(string(revision)) } - if packed, err := os.ReadFile(filepath.Join(gitDirectory, "packed-refs")); err == nil { + if packed, err := os.ReadFile(filepath.Join(gitDirectory, "packed-refs")); err == nil { //nolint:gosec // G304: test opens controlled fixture path under temp dir for _, line := range strings.Split(string(packed), "\n") { fields := strings.Fields(line) if len(fields) == 2 && fields[1] == reference { @@ -217,14 +217,14 @@ func faultWriteTrace(path string, trace faultTrace) error { if err != nil { return err } - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. return err } - return os.WriteFile(path, data, 0o644) + return os.WriteFile(path, data, 0o600) //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. } func faultReadTrace(path string) (faultTrace, error) { - data, err := os.ReadFile(path) + data, err := os.ReadFile(path) //nolint:gosec // G304: test opens controlled fixture path under temp dir if err != nil { return faultTrace{}, err } diff --git a/epaxos/hard_state_ready_test.go b/epaxos/hard_state_ready_test.go index ed05e12..03ad24d 100644 --- a/epaxos/hard_state_ready_test.go +++ b/epaxos/hard_state_ready_test.go @@ -58,7 +58,9 @@ func TestTickReadyPersistsAndRestartsLogicalTime(t *testing.T) { t.Fatal(err) } - rn.Tick() + if err := rn.Tick(); err != nil { + panic(err) + } rd := rn.Ready() if rd.HardState.Tick != 1 || rd.HardState.Conf.ID != 1 || !rd.MustSync { t.Fatalf("tick-only Ready = %#v, want complete hard state at tick 1", rd) @@ -84,7 +86,9 @@ func TestTickReadyPersistsAndRestartsLogicalTime(t *testing.T) { if err := rn.Advance(rd); err != nil { t.Fatal(err) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } next := rn.Ready() if next.HardState.Tick != 2 { t.Fatalf("next hard-state tick = %d, want 2", next.HardState.Tick) @@ -102,7 +106,9 @@ func TestOutstandingReadyFrozenWhileTickAndStepAccumulate(t *testing.T) { t.Fatalf("initial frozen Ready = %#v", frozen) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } ref := InstanceRef{Replica: 1, Instance: 1, Conf: 1} msg := Message{ Type: MsgPreAccept, @@ -189,7 +195,9 @@ func TestInvalidHardStateAcknowledgementsStutter(t *testing.T) { if err != nil { t.Fatal(err) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } canonical := rn.Ready() if canonical.HardState.Tick != 1 { t.Fatalf("canonical hard-state tick = %d, want 1", canonical.HardState.Tick) @@ -316,7 +324,9 @@ func TestReadyHardStateCloneReleaseAndAckValidationAllocations(t *testing.T) { if err != nil { t.Fatal(err) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } rd := rn.Ready() clone := rd.Clone() clone.HardState.Conf.Voters[0] = 9 @@ -408,7 +418,9 @@ func TestTickGeneratedRetryIsFencedByHardState(t *testing.T) { t.Fatal(err) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } retry := rn.Ready() if retry.HardState.Tick != 1 || retry.HardState.Conf.ID != 1 || !retry.MustSync { t.Fatalf("retry Ready is not fenced by tick 1 hard state: %#v", retry) diff --git a/epaxos/internal_test.go b/epaxos/internal_test.go index b7b92bb..70a8591 100644 --- a/epaxos/internal_test.go +++ b/epaxos/internal_test.go @@ -298,7 +298,9 @@ func TestPrepareRecoveryPath(t *testing.T) { t.Fatal(err) } inst := s.nodes[1].instances[ref] - s.nodes[1].startPrepare(inst) + if err := s.nodes[1].startPrepare(inst); err != nil { + panic(err) + } s.drain() if len(s.apps[1]) != 1 { t.Fatalf("prepare path did not commit locally: %d", len(s.apps[1])) @@ -331,12 +333,22 @@ func TestRejectPathsAndTimers(t *testing.T) { t.Fatal(err) } inst := s.nodes[1].instances[ref] - s.nodes[1].onTimer(inst, timerPreAccept) + if err := s.nodes[1].onTimer(inst, timerPreAccept); err != nil { + panic(err) + } s.nodes[1].startAccept(inst, inst.rec.Attributes()) - s.nodes[1].onTimer(inst, timerAccept) - s.nodes[1].startPrepare(inst) - s.nodes[1].onTimer(inst, timerPrepare) - s.nodes[1].onTimer(inst, timerFastWait) + if err := s.nodes[1].onTimer(inst, timerAccept); err != nil { + panic(err) + } + if err := s.nodes[1].startPrepare(inst); err != nil { + panic(err) + } + if err := s.nodes[1].onTimer(inst, timerPrepare); err != nil { + panic(err) + } + if err := s.nodes[1].onTimer(inst, timerFastWait); err != nil { + t.Fatal(err) + } } func TestEncodeMessageRejectsWireLimitOverflowWithoutAppending(t *testing.T) { diff --git a/epaxos/malformed_fuzz_test.go b/epaxos/malformed_fuzz_test.go index 4afd64a..0ec5eec 100644 --- a/epaxos/malformed_fuzz_test.go +++ b/epaxos/malformed_fuzz_test.go @@ -26,7 +26,7 @@ func FuzzDecodeMessageWithScratchNeverPanics(f *testing.F) { f.Add([]byte(nil)) f.Add([]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01}) f.Add(encoded) - f.Fuzz(func(t *testing.T, frame []byte) { + f.Fuzz(func(_ *testing.T, frame []byte) { scratch := DecodeScratch{ Deps: make([]InstanceNum, 0, 7), AcceptDeps: make([]InstanceNum, 0, 7), @@ -124,7 +124,7 @@ func FuzzRestartNeverPanicsOnMalformedChecksummedRecord(f *testing.F) { f.Add([]byte(nil)) f.Add([]byte{1, 1, 1, byte(StatusCommitted), byte(CommandNoop), 3}) f.Add([]byte{0xff, 0, 2, 0xff, 0xff, 9, 9, 9}) - f.Fuzz(func(t *testing.T, data []byte) { + f.Fuzz(func(_ *testing.T, data []byte) { at := func(i int) byte { if len(data) == 0 { return 0 diff --git a/epaxos/message.go b/epaxos/message.go index 85fd548..3ae7e27 100644 --- a/epaxos/message.go +++ b/epaxos/message.go @@ -62,6 +62,7 @@ func (t MessageType) String() string { // RejectReason classifies TryPreAccept recovery rejections. type RejectReason uint8 +// RejectReason values explain rejected protocol messages. const ( RejectNone RejectReason = iota RejectStaleBallot @@ -294,6 +295,7 @@ func (m Message) Validate(conf ConfState) error { if m.From != m.Ballot.Replica { return ErrInvalidMessage } + case MsgPreAcceptResp, MsgAcceptResp, MsgCommit, MsgPrepareResp, MsgTryPreAcceptResp, MsgEvidenceResp: } validRef := func(ref InstanceRef) bool { return ref.Replica != 0 && ref.Instance != 0 && ref.Conf == conf.ID && conf.Contains(ref.Replica) @@ -306,7 +308,7 @@ func (m Message) Validate(conf ConfState) error { if !conflictPresent { return ErrInvalidMessage } - } else if conflictPresent && !(m.Type == MsgTryPreAcceptResp && m.Reject) { + } else if conflictPresent && (m.Type != MsgTryPreAcceptResp || !m.Reject) { return ErrInvalidMessage } if m.IgnoreDependency != (TryPreAcceptIgnore{}) { @@ -369,6 +371,8 @@ func (m Message) Validate(conf ConfState) error { if !m.Reject || m.RejectReason != RejectAcceptedTarget { return ErrInvalidMessage } + case MsgPreAccept, MsgPreAcceptResp, MsgAcceptResp, MsgPrepare, MsgEvidence: + fallthrough default: return ErrInvalidMessage } @@ -405,6 +409,7 @@ func (m Message) Validate(conf ConfState) error { if m.RecordStatus >= StatusPreAccepted && m.Seq == 0 { return ErrInvalidMessage } + case MsgAcceptResp, MsgPrepare, MsgEvidence: } } if (m.AcceptSeq == 0) != (len(m.AcceptDeps) == 0) { @@ -423,6 +428,8 @@ func (m Message) Validate(conf ConfState) error { if !m.Reject || m.RejectReason != RejectAcceptedTarget { return ErrInvalidMessage } + case MsgPreAccept, MsgPreAcceptResp, MsgPrepare, MsgTryPreAccept, MsgEvidence: + fallthrough default: return ErrInvalidMessage } @@ -453,6 +460,8 @@ func (m Message) Validate(conf ConfState) error { if hasAcceptMetadata && (!m.Reject || m.RejectReason != RejectAcceptedTarget) { return ErrInvalidMessage } + case MsgPreAccept, MsgPreAcceptResp, MsgPrepare, MsgTryPreAccept, MsgEvidence: + fallthrough default: if hasAcceptMetadata { return ErrInvalidMessage @@ -466,6 +475,8 @@ func (m Message) Validate(conf ConfState) error { if m.ProcessAt != 0 { switch m.Type { case MsgPreAccept, MsgAccept, MsgCommit, MsgPrepareResp, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidenceResp: + case MsgPreAcceptResp, MsgAcceptResp, MsgPrepare, MsgEvidence: + fallthrough default: return ErrInvalidMessage } @@ -482,6 +493,8 @@ func (m Message) Validate(conf ConfState) error { m.RecordBallot != (Ballot{Replica: m.Ref.Replica}) { return ErrInvalidMessage } + case MsgPreAccept, MsgAccept, MsgAcceptResp, MsgCommit, MsgPrepare, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidence: + fallthrough default: return ErrInvalidMessage } @@ -547,9 +560,13 @@ func (m Message) Validate(conf ConfState) error { if m.RejectHint != (Ballot{}) { return ErrInvalidMessage } + case RejectNone, RejectAcceptedTarget: + fallthrough default: return ErrInvalidMessage } + case MsgPreAccept, MsgAccept, MsgCommit, MsgPrepare, MsgTryPreAccept, MsgEvidence, MsgEvidenceResp: + fallthrough default: return ErrInvalidMessage } diff --git a/epaxos/node.go b/epaxos/node.go index 8b3d28c..510b18b 100644 --- a/epaxos/node.go +++ b/epaxos/node.go @@ -4,6 +4,7 @@ import ( "bytes" "container/heap" "encoding/binary" + "errors" "fmt" "sort" ) @@ -269,6 +270,8 @@ func messageCarriesValue(message Message) bool { case MsgTryPreAcceptResp: return (!message.Reject && message.RecordStatus >= StatusPreAccepted) || (message.Reject && message.RejectReason == RejectAcceptedTarget) + case MsgPreAcceptResp, MsgAcceptResp, MsgPrepare, MsgEvidence: + fallthrough default: return false } @@ -284,6 +287,8 @@ func (n *RawNode) messageTimingDomainEnabled(message Message) bool { } switch message.Type { case MsgAccept, MsgCommit: + case MsgPreAccept, MsgPreAcceptResp, MsgAcceptResp, MsgPrepare, MsgPrepareResp, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidence, MsgEvidenceResp: + fallthrough default: return false } @@ -524,6 +529,8 @@ func validateConfChangeResult(rec InstanceRecord) error { if !confStateIsZero(result.Conf) { return invalid("has configuration voters on a rejected result") } + case ConfChangeOutcomeUnspecified: + fallthrough default: return invalid("has no terminal configuration result") } @@ -550,9 +557,9 @@ func validateStoredInstanceRecord(rec InstanceRecord, conf ConfState) error { return invalid("has invalid timing metadata") } commandValid := commandValidForConfiguration(rec.Command, rec.Ref, conf) - if !commandValid && !(rec.Command.Kind == CommandConfChange && - rec.Status == StatusExecuted && - rec.ConfChangeResult.Outcome == ConfChangeRejectedInvalid) { + if !commandValid && (rec.Command.Kind != CommandConfChange || + rec.Status != StatusExecuted || + rec.ConfChangeResult.Outcome != ConfChangeRejectedInvalid) { return invalid("has an invalid command") } if err := validateConfChangeResult(rec); err != nil { @@ -805,7 +812,7 @@ func NewRawNode(cfg Config) (*RawNode, error) { !VerifyRecordChecksumWithoutTimingDomain(rec) && !VerifyRecordChecksumWithoutConfChangeResult(rec) { return ErrChecksumMismatch } - if rec.Command.Kind == CommandNoop { + if rec.Command.Kind == CommandNoop { //nolint:gocritic // ifElseChain: status dispatch keeps explicit ordering if cfg.LegacyProcessAtDomain == nil { return fmt.Errorf("%w: durable no-op record %s has ambiguous legacy timing domain", ErrInvalidConfig, rec.Ref) } @@ -924,7 +931,7 @@ func NewRawNode(cfg Config) (*RawNode, error) { heap.Init(&n.logicalPreAccepts) heap.Init(&n.toqPreAccepts) restartTimerError := func(err error) error { - if err == ErrLogicalTimeExhausted && n.tick == ^uint64(0) { + if errors.Is(err, ErrLogicalTimeExhausted) && n.tick == ^uint64(0) { return nil } return err @@ -974,6 +981,7 @@ func NewRawNode(cfg Config) (*RawNode, error) { if err := restartTimerError(n.schedule(inst, timerPrepare, n.recoveryTicks)); err != nil { return nil, err } + case phaseTryPreAccept, phaseCommitted: } } n.beginDrive() @@ -1011,7 +1019,7 @@ func sameReplicaIDs(a, b []ReplicaID) bool { return false } for i := range a { - if a[i] != b[i] { + if a[i] != b[i] { //nolint:gosec // G602: index is guarded by matching slice lengths. return false } } @@ -1026,6 +1034,8 @@ func phaseFromStatus(s Status) phase { return phaseAccept case StatusCommitted, StatusExecuted: return phaseCommitted + case StatusNone: + fallthrough default: return phaseIdle } @@ -1093,6 +1103,7 @@ func (n *RawNode) seedLocalRestartVote(inst *instance) { } inst.prepareOK = getRecordVoteSet() inst.prepareOK.add(n.confFor(inst.rec.Ref.Conf), n.id, inst.rec) + case phaseIdle, phaseTryPreAccept, phaseCommitted: } } @@ -1339,6 +1350,7 @@ func (n *RawNode) replayExecutedConfigRecords(records []InstanceRecord) error { if !applied && !checkpoint { return fmt.Errorf("%w: superseded configuration command %s has no applied successor", ErrInvalidConfig, rec.Ref) } + case ConfChangeOutcomeUnspecified, ConfChangeApplied: } } } @@ -2070,6 +2082,7 @@ func (n *RawNode) Step(m Message) error { return nil } +// HasReady reports whether Ready has unacknowledged work. func (n *RawNode) HasReady() bool { return n.awaitAdvance || !n.pendingReady.Empty() || !n.currentHardState.Equal(n.acknowledgedHardState) } @@ -3194,7 +3207,9 @@ func (n *RawNode) startAccept(inst *instance, attrs Attributes) { return } n.broadcast(MsgAccept, inst.rec) - n.schedule(inst, timerAccept, n.retryTicks) + if err := n.schedule(inst, timerAccept, n.retryTicks); err != nil { + panic(err) // Tick prevents logical-clock exhaustion before scheduling. + } } func (n *RawNode) startTryPreAccept(inst *instance, attrs Attributes, witnesses voterMask, countInitialLeader bool) { @@ -3235,7 +3250,9 @@ func (n *RawNode) startTryPreAccept(inst *instance, attrs Attributes, witnesses return } n.broadcastTryPreAccept(inst) - n.schedule(inst, timerTryPreAccept, n.retryTicks) + if err := n.schedule(inst, timerTryPreAccept, n.retryTicks); err != nil { + panic(err) // Tick prevents logical-clock exhaustion before scheduling. + } } func (n *RawNode) handleTryPreAccept(m Message) error { @@ -4453,7 +4470,7 @@ func (n *RawNode) computeAttrsAt(cmd Command, exclude InstanceRef, processAt uin if cmd.Kind == CommandNoop { return Attributes{Seq: seq, Deps: deps} } - if commandHasGlobalConflictScope(cmd.Kind) { + if commandHasGlobalConflictScope(cmd.Kind) { //nolint:gocritic // ifElseChain: attrs path keeps explicit ordering addIndexedLanes(n.allConflicts) if len(n.allConflicts) == 0 { for ref, inst := range n.instances { @@ -4759,6 +4776,8 @@ func (n *RawNode) wouldReplaceInstance(m Message, current *instance, ordered boo } rec := n.acceptRecord(m, current) return current.rec.Status != StatusAccepted || !instanceRecordEqual(current.rec, rec) + case MsgPreAcceptResp, MsgAcceptResp, MsgCommit, MsgPrepare, MsgPrepareResp, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidence, MsgEvidenceResp: + fallthrough default: return false } @@ -5017,6 +5036,8 @@ func (n *RawNode) responseTimingTransition(m Message) (*instance, bool, bool, bo prospective[m.From] = InstanceRecord{Ref: m.Ref, Ballot: m.Ballot, RecordBallot: m.RecordBallot, Status: m.RecordStatus, Seq: m.Seq, Deps: append([]InstanceNum(nil), m.Deps...), AcceptSeq: m.AcceptSeq, AcceptDeps: append([]InstanceNum(nil), m.AcceptDeps...), AcceptEvidence: cloneAcceptEvidence(m.AcceptEvidence), Command: m.Command.Clone(), FastPathEligible: m.FastPathEligible, ProcessAt: m.ProcessAt, TimingDomain: timingDomainFromMessage(m)} _, failClosed := n.tryEvidenceDecision(inst, key, prospective) return inst, failClosed, failClosed, false + case MsgPreAccept, MsgAccept, MsgCommit, MsgPrepare, MsgTryPreAccept, MsgEvidence: + fallthrough default: return inst, false, false, false } @@ -5061,6 +5082,7 @@ func (n *RawNode) preflightTimingStep(m Message) error { } case MsgPreAcceptResp, MsgAcceptResp, MsgPrepareResp, MsgTryPreAcceptResp, MsgEvidenceResp: inst, advanceGeneration, retryTimer, recoveryTimer = n.responseTimingTransition(m) + case MsgCommit, MsgEvidence: } generationDelta := uint64(0) if advanceGeneration { @@ -5072,6 +5094,7 @@ func (n *RawNode) preflightTimingStep(m Message) error { if retryTimer { generationDelta = n.startAcceptGenerationDelta(inst) } + case MsgPreAccept, MsgAccept, MsgAcceptResp, MsgCommit, MsgPrepare, MsgTryPreAccept, MsgEvidence: } } if !advanceGeneration && !retryTimer && !recoveryTimer { @@ -5137,6 +5160,7 @@ func (n *RawNode) preflightRecoveryStep(m Message) error { base = inst.rec.Ballot } } + case MsgPreAccept, MsgAccept, MsgCommit, MsgPrepare, MsgTryPreAccept, MsgEvidence, MsgEvidenceResp: } if base == (Ballot{}) { return nil diff --git a/epaxos/optimized_test.go b/epaxos/optimized_test.go index af5d099..f7f719f 100644 --- a/epaxos/optimized_test.go +++ b/epaxos/optimized_test.go @@ -149,7 +149,9 @@ func optimizedStartRecovery(t *testing.T, voters int, ref InstanceRef) (*RawNode rn := optimizedNewRawNode(t, 1, voters) inst := &instance{rec: InstanceRecord{Ref: ref, Status: StatusNone, Deps: rn.q.deps()}} rn.instances[ref] = inst - rn.startPrepare(inst) + if err := rn.startPrepare(inst); err != nil { + panic(err) + } ballot := inst.rec.Ballot rd := rn.Ready() advanceOK(t, rn, rd) diff --git a/epaxos/performance_benchmark_test.go b/epaxos/performance_benchmark_test.go index d14b839..b694445 100644 --- a/epaxos/performance_benchmark_test.go +++ b/epaxos/performance_benchmark_test.go @@ -175,7 +175,9 @@ func BenchmarkOptimizedRecovery(b *testing.B) { ref := InstanceRef{Replica: 2, Instance: 1, Conf: 1} inst := &instance{rec: InstanceRecord{Ref: ref, Status: StatusNone, Deps: n.q.deps()}} n.instances[ref] = inst - n.startPrepare(inst) + if err := n.startPrepare(inst); err != nil { + panic(err) + } ballot := inst.rec.Ballot rd := n.Ready() if err = n.Advance(rd); err != nil { @@ -288,14 +290,14 @@ func BenchmarkLiveInstanceRetention(b *testing.B) { var resident uintptr for i := range corpus { rec := InstanceRecord{ - Ref: InstanceRef{Replica: ReplicaID(i%voters + 1), Instance: InstanceNum(i/voters + 1), Conf: 1}, + Ref: InstanceRef{Replica: ReplicaID(i%voters + 1), Instance: InstanceNum(i/voters + 1), Conf: 1}, //nolint:gosec // G115: benchmark converts bounded int index/count Ballot: Ballot{Replica: 1}, RecordBallot: Ballot{Replica: 1}, Status: StatusExecuted, Seq: uint64(i + 1), - Deps: make([]InstanceNum, voters, voters), + Deps: make([]InstanceNum, voters), Command: Command{ Kind: CommandUser, - Payload: make([]byte, 64, 64), - ConflictKeys: [][]byte{make([]byte, len("fixed-conflict-key"), len("fixed-conflict-key"))}, + Payload: make([]byte, 64), + ConflictKeys: [][]byte{make([]byte, len("fixed-conflict-key"))}, }, } corpus[i] = instance{rec: rec, phase: phaseCommitted} diff --git a/epaxos/pool_ownership_test.go b/epaxos/pool_ownership_test.go index 47b6806..52a7124 100644 --- a/epaxos/pool_ownership_test.go +++ b/epaxos/pool_ownership_test.go @@ -110,7 +110,7 @@ func TestReadyReleaseClearsHeadersAndBackingArraysWithoutAllocation(t *testing.T allocRecords[0] = InstanceRecord{Deps: deps, Command: Command{Payload: payload, ConflictKeys: keys}} allocMessages[0] = Message{Deps: deps, Command: Command{Payload: payload, ConflictKeys: keys}} allocCommitted[0] = CommittedCommand{Deps: deps, Command: Command{Payload: payload, ConflictKeys: keys}} - ready := Ready{Records: allocRecords[:], Messages: allocMessages[:], Committed: allocCommitted[:], MustSync: true} + ready := Ready{Records: allocRecords, Messages: allocMessages, Committed: allocCommitted, MustSync: true} //nolint:gocritic // unslice: keep explicit slice form in test fixture ready.Release() }) if allocs != 0 { @@ -938,7 +938,7 @@ func deterministicShortByteSeeds() [][]byte { seed := make([]byte, n) for i := range seed { state = state*1664525 + 1013904223 - seed[i] = byte(state >> 24) + seed[i] = byte(state >> 24) //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. } seeds = append(seeds, seed) } @@ -1048,31 +1048,31 @@ func countMaterializedDependenciesInView(rn *RawNode, view *executionView, base func TestDependencyIteratorAllocationIndependentOfEndpoint(t *testing.T) { small, smallBase := dependencyIteratorAllocationNode(t, 8) - max, maxBase := dependencyIteratorAllocationNode(t, ^InstanceNum(0)) - if got, want := countMaterializedDependencies(small, smallBase), countMaterializedDependencies(max, maxBase); got != want || got != 2 { - t.Fatalf("iterator cardinality small/max = %d/%d, want 2/2", got, want) + maxNode, maxBase := dependencyIteratorAllocationNode(t, ^InstanceNum(0)) + if got, want := countMaterializedDependencies(small, smallBase), countMaterializedDependencies(maxNode, maxBase); got != want || got != 2 { + t.Fatalf("iterator cardinality small/maxNode = %d/%d, want 2/2", got, want) } countMaterializedDependencies(small, smallBase) - countMaterializedDependencies(max, maxBase) + countMaterializedDependencies(maxNode, maxBase) smallAllocs := testing.AllocsPerRun(100, func() { dependencyIteratorCountSink = countMaterializedDependencies(small, smallBase) }) maxAllocs := testing.AllocsPerRun(100, func() { - dependencyIteratorCountSink = countMaterializedDependencies(max, maxBase) + dependencyIteratorCountSink = countMaterializedDependencies(maxNode, maxBase) }) if smallAllocs != maxAllocs { - t.Fatalf("iterator allocations depend on endpoint: small=%v max=%v", smallAllocs, maxAllocs) + t.Fatalf("iterator allocations depend on endpoint: small=%v maxNode=%v", smallAllocs, maxAllocs) } smallView := small.newExecutionView() - maxView := max.newExecutionView() + maxView := maxNode.newExecutionView() smallPrimitiveAllocs := testing.AllocsPerRun(100, func() { dependencyIteratorCountSink = countMaterializedDependenciesInView(small, &smallView, smallBase) }) maxPrimitiveAllocs := testing.AllocsPerRun(100, func() { - dependencyIteratorCountSink = countMaterializedDependenciesInView(max, &maxView, maxBase) + dependencyIteratorCountSink = countMaterializedDependenciesInView(maxNode, &maxView, maxBase) }) if smallPrimitiveAllocs != 0 || maxPrimitiveAllocs != 0 { - t.Fatalf("warmed dependency iterator allocated: small=%v max=%v", smallPrimitiveAllocs, maxPrimitiveAllocs) + t.Fatalf("warmed dependency iterator allocated: small=%v maxNode=%v", smallPrimitiveAllocs, maxPrimitiveAllocs) } } diff --git a/epaxos/protocol_coverage_test.go b/epaxos/protocol_coverage_test.go index 3535fad..9012c6c 100644 --- a/epaxos/protocol_coverage_test.go +++ b/epaxos/protocol_coverage_test.go @@ -464,7 +464,9 @@ func TestDependencyPredicatesConservativelyRejectMissingConfigKnowledge(t *testi func TestTryPreAcceptTimerRebroadcastsOnlyWhileInTryPhase(t *testing.T) { rn, inst, ref := protocolTryInstance(t, 3, 1) - rn.onTimer(inst, timerTryPreAccept) + if err := rn.onTimer(inst, timerTryPreAccept); err != nil { + panic(err) + } rd := rn.Ready() for _, to := range []ReplicaID{2, 3} { msg := optimizedRequireMessage(t, rd.Messages, MsgTryPreAccept, to) @@ -497,7 +499,9 @@ func TestTryPreAcceptTimerDropsStaleEvidenceChecksBeforeRetry(t *testing.T) { unrelatedTargetKey: {}, } - rn.onTimer(inst, timerTryPreAccept) + if err := rn.onTimer(inst, timerTryPreAccept); err != nil { + panic(err) + } if _, ok := rn.tryEvidenceChecks[staleTargetKey]; ok { t.Fatalf("TryPreAccept timer left stale evidence check for target: %#v", rn.tryEvidenceChecks) } @@ -526,11 +530,15 @@ func TestTryPreAcceptTimerDropsStaleEvidenceChecksBeforeRetry(t *testing.T) { t.Fatalf("accepted first TryPreAccept retry Ready left outstanding work: %#v", rn.Ready()) } for tick := uint64(1); tick < rn.retryTicks; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } protocolCoverageAdvanceHardStateOnly(t, rn, tick) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } retry := rn.Ready() for _, to := range []ReplicaID{2, 3} { msg := optimizedRequireMessage(t, retry.Messages, MsgTryPreAccept, to) @@ -1203,7 +1211,9 @@ func TestPendingEvidenceTimeoutFallsBackToSlowAccept(t *testing.T) { rn.instances[target] = inst rn.tryEvidenceChecks = map[tryEvidenceKey]map[ReplicaID]InstanceRecord{{target: target, conflict: conflict, ballot: inst.rec.Ballot}: {}} - rn.onTimer(inst, timerTryPreAccept) + if err := rn.onTimer(inst, timerTryPreAccept); err != nil { + panic(err) + } if inst.phase != phaseAccept || inst.rec.Status != StatusAccepted { t.Fatalf("pending evidence timeout phase/status = %d/%s, want slow accept/%s", inst.phase, inst.rec.Status, StatusAccepted) } @@ -1752,7 +1762,7 @@ func TestReorderedPreAcceptDoesNotDowngradeAcceptedRecord(t *testing.T) { func TestNormalFastPathAdoptsCoveringRemoteTupleAtPaperQuorum(t *testing.T) { for _, voters := range []int{3, 5, 7} { - t.Run(string(rune('0'+voters))+"-voters", func(t *testing.T) { + t.Run(string(rune('0'+voters))+"-voters", func(t *testing.T) { //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. rn := protocolCoverageNewRawNode(t, 1, voters) cmd := optimizedTestCommand("covering-normal-fast", "covering-normal-fast-key") ref, err := rn.Propose(cmd) @@ -1779,7 +1789,7 @@ func TestNormalFastPathAdoptsCoveringRemoteTupleAtPaperQuorum(t *testing.T) { covering.Seq++ covering.Deps[1] = dependency.Instance neededRemote := rn.fastQuorumForConf(ref.Conf) - 1 - for id := ReplicaID(2); id < ReplicaID(2+neededRemote); id++ { + for id := ReplicaID(2); id < ReplicaID(2+neededRemote); id++ { //nolint:gosec // G115: test harness converts bounded int index/count if err := rn.Step(Message{Type: MsgPreAcceptResp, From: id, To: 1, Ref: ref, @@ -1822,7 +1832,9 @@ func TestRecoveryRecomputesAttrsForUnsafePreAcceptedCandidate(t *testing.T) { rec.Checksum = ChecksumRecord(rec) inst := &instance{rec: rec, phase: phaseIdle} rn.instances[target] = inst - rn.startPrepare(inst) + if err := rn.startPrepare(inst); err != nil { + panic(err) + } if err := rn.Step(Message{ Type: MsgPrepareResp, From: 2, @@ -1869,10 +1881,14 @@ func TestPromisedFollowerTakesOverAfterCoordinatorDisappears(t *testing.T) { deadline := rn.recoveryTicks * 2 for tick := uint64(1); tick < deadline; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } protocolCoverageAdvanceHardStateOnly(t, rn, tick) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } rd := rn.Ready() if !rd.HardState.Equal(HardState{Conf: rn.Status().Conf, Tick: deadline}) || !rd.MustSync { t.Fatalf("takeover Ready hard state = %#v, want tick %d", rd.HardState, deadline) @@ -1901,10 +1917,14 @@ func TestTryPreAcceptFollowerTakesOverAfterCoordinatorDisappears(t *testing.T) { deadline := rn.recoveryTicks * 2 for tick := uint64(1); tick < deadline; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } protocolCoverageAdvanceHardStateOnly(t, rn, tick) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } rd := rn.Ready() if !rd.HardState.Equal(HardState{Conf: rn.Status().Conf, Tick: deadline}) || !rd.MustSync { t.Fatalf("TryPreAccept takeover Ready hard state = %#v, want tick %d", rd.HardState, deadline) @@ -1923,9 +1943,13 @@ func TestLateTimedPreAcceptFallsBackToConservativeConflictOrdering(t *testing.T) t.Fatal(err) } protocolCoverageAdvanceHardStateOnly(t, rn, 0) - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } protocolCoverageAdvanceHardStateOnly(t, rn, 1) - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } protocolCoverageAdvanceHardStateOnly(t, rn, 2) key := "late-timed-conflict" @@ -2065,7 +2089,9 @@ func TestUnresolvedKnownConflictStartsRecoveryOnAnyHealthyReplica(t *testing.T) deadline := rn.recoveryTicks * 2 var rd Ready for tick := uint64(1); tick <= deadline; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } inst := rn.instances[blocker] if inst != nil && inst.phase == phasePrepare { rd = rn.Ready() @@ -2450,6 +2476,7 @@ func TestValueBearingMessagesRejectIncompatibleTimingDomains(t *testing.T) { message.RecordBallot = Ballot{Replica: ref.Replica} message.RecordStatus = StatusAccepted message.ConflictRef = InstanceRef{Conf: 1, Replica: 1, Instance: 99} + case MsgPreAccept, MsgPreAcceptResp, MsgAccept, MsgAcceptResp, MsgPrepare, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidence: } return message } @@ -2654,6 +2681,7 @@ func TestTimedNoopMessagesRejectWithoutMutation(t *testing.T) { message.RecordBallot = Ballot{Replica: ref.Replica} message.RecordStatus = StatusAccepted message.ConflictRef = InstanceRef{Conf: 1, Replica: 1, Instance: 99} + case MsgPreAccept, MsgPreAcceptResp, MsgAccept, MsgAcceptResp, MsgPrepare, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidence: } return message } diff --git a/epaxos/recovery_boundary_test.go b/epaxos/recovery_boundary_test.go index 164fb83..a077110 100644 --- a/epaxos/recovery_boundary_test.go +++ b/epaxos/recovery_boundary_test.go @@ -120,8 +120,8 @@ func TestBallotNextCheckedBoundaries(t *testing.T) { }) } - max := Ballot{Epoch: ^uint64(0), Number: ^uint64(0), Replica: 2} - got, err := max.Next(1) + maxBallot := Ballot{Epoch: ^uint64(0), Number: ^uint64(0), Replica: 2} + got, err := maxBallot.Next(1) if !errors.Is(err, ErrBallotExhausted) || got != (Ballot{}) { t.Fatalf("absolute exhaustion = %#v, %v, want zero, %v", got, err, ErrBallotExhausted) } @@ -152,6 +152,8 @@ func recoveryBoundaryRejectFixture(t *testing.T, typ MessageType, hint Ballot) ( case MsgPrepareResp: inst.phase = phasePrepare inst.prepareOK = testRecordVoteSet(t, rn.q.conf, map[ReplicaID]InstanceRecord{1: inst.rec.Clone()}) + case MsgPreAccept, MsgAccept, MsgCommit, MsgPrepare, MsgTryPreAccept, MsgTryPreAcceptResp, MsgEvidence, MsgEvidenceResp: + fallthrough default: t.Fatalf("unsupported reject fixture %s", typ) } @@ -189,10 +191,10 @@ func TestRejectPathsIncrementObservedBallotExactlyOnce(t *testing.T) { } func TestStepBallotExhaustionIsTypedAndHasNoEffect(t *testing.T) { - max := Ballot{Epoch: ^uint64(0), Number: ^uint64(0), Replica: 2} + maxBallot := Ballot{Epoch: ^uint64(0), Number: ^uint64(0), Replica: 2} for _, typ := range []MessageType{MsgPreAcceptResp, MsgAcceptResp, MsgPrepareResp} { t.Run(typ.String(), func(t *testing.T) { - rn, inst, msg := recoveryBoundaryRejectFixture(t, typ, max) + rn, inst, msg := recoveryBoundaryRejectFixture(t, typ, maxBallot) before := snapshotRecoveryBoundary(rn, inst) err := rn.Step(msg) if !errors.Is(err, ErrBallotExhausted) { @@ -257,7 +259,9 @@ func TestTickBallotExhaustionStuttersWithoutEffects(t *testing.T) { Command: Command{Kind: CommandNoop}, }), phase: phaseIdle} rn.instances[ref] = inst - rn.schedule(inst, timerPrepare, 1) + if err := rn.schedule(inst, timerPrepare, 1); err != nil { + panic(err) + } before := snapshotRecoveryBoundary(rn, inst) err := rn.Tick() @@ -323,11 +327,11 @@ func TestEpochCarryBallotIsRecoveryAcrossValidationRestartAndFastGuard(t *testin func TestRestartReconstructsDurablePrepareSelfVoteAtMaximumFailures(t *testing.T) { for _, voters := range []int{3, 5, 7} { - t.Run(string(rune('0'+voters))+" voters", func(t *testing.T) { + t.Run(string(rune('0'+voters))+" voters", func(t *testing.T) { //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. store := NewMemoryStorage() rn := recoveryBoundaryNode(t, 1, voters, store) ref := InstanceRef{Replica: 2, Instance: 1, Conf: 1} - cmd := Command{ID: CommandID{Client: 600 + uint64(voters), Sequence: 1}, Payload: []byte("restart-prepare"), ConflictKeys: [][]byte{[]byte("restart-prepare-key")}} + cmd := Command{ID: CommandID{Client: 600 + uint64(voters), Sequence: 1}, Payload: []byte("restart-prepare"), ConflictKeys: [][]byte{[]byte("restart-prepare-key")}} //nolint:gosec // G115: test harness converts bounded int index/count inst := &instance{rec: checkedRecord(InstanceRecord{ Ref: ref, Ballot: Ballot{Replica: ref.Replica}, @@ -419,7 +423,7 @@ func TestTryPreAcceptAcceptedTargetRestartsPrepareAndCompletes(t *testing.T) { if same { name = "same tuple" } - t.Run(string(rune('0'+voters))+" voters/"+name, func(t *testing.T) { + t.Run(string(rune('0'+voters))+" voters/"+name, func(t *testing.T) { //nolint:gosec // G115: test harness converts bounded int to rune coordinator := recoveryBoundaryNode(t, 1, voters, nil) follower := recoveryBoundaryNode(t, 2, voters, nil) ref := InstanceRef{Replica: 2, Instance: 1, Conf: 1} diff --git a/epaxos/recovery_test.go b/epaxos/recovery_test.go index 2c56ec1..f66f3f8 100644 --- a/epaxos/recovery_test.go +++ b/epaxos/recovery_test.go @@ -16,7 +16,9 @@ func recoveryFinishPreAcceptedValidation(t *testing.T, store *MemoryStorage, rn for tick := uint64(1); tick < rn.retryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, rn, "PreAccept slow fallback before retry deadline") } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } rd = rn.Ready() } if inst.phase == phaseAccept { @@ -203,7 +205,9 @@ func TestRestartResumesForeignRecoveryBallot(t *testing.T) { for tick := 1; tick < 5; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "foreign recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() recoveryRequirePrepareRefs(t, rd.Messages, []InstanceRef{ref}) for _, m := range rd.Messages { @@ -244,7 +248,9 @@ func TestOldConfigRecoveryUsesPinnedVotersAfterRemoval(t *testing.T) { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() prepareBallot := recoveryRequireMessageTargets(t, rd.Messages, MsgPrepare, ref, []ReplicaID{1, 3, 4}) recoveryApplyReady(t, store, restarted, rd) @@ -336,7 +342,9 @@ func TestOldConfigRecoveryUsesPinnedMidChainVotersAfterAddThenRemove(t *testing. recoveryPersistTickOnlyHardState(t, store, restarted, "mid-chain old-config recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() prepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, rd.Messages, MsgPrepare, ref, []ReplicaID{2, 3, 4}, 4) recoveryApplyReady(t, store, restarted, rd) @@ -430,7 +438,9 @@ func TestOldConfigChainRecoveryRetryCompletesAfterLostPreRetryResponses(t *testi recoveryPersistTickOnlyHardState(t, store, restarted, "mid-chain old-config recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() prepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, rd.Messages, MsgPrepare, ref, []ReplicaID{2, 3, 4}, 4) recoveryApplyReady(t, store, restarted, rd) @@ -467,7 +477,9 @@ func TestOldConfigChainRecoveryRetryCompletesAfterLostPreRetryResponses(t *testi for tick := uint64(1); tick < recoveryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "mid-chain recovery prepare retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } retry := restarted.Ready() retryPrepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, retry.Messages, MsgPrepare, ref, []ReplicaID{2, 3, 4}, 4) if retryPrepareBallot != prepareBallot { @@ -503,7 +515,9 @@ func TestOldConfigChainRecoveryRetryCompletesAfterLostPreRetryResponses(t *testi for tick := uint64(1); tick < retryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "mid-chain recovery accept retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } retry = restarted.Ready() retryAcceptBallot := recoveryRequireMessageTargetsWithDepsWidth(t, retry.Messages, MsgAccept, ref, []ReplicaID{2, 3, 4}, 4) if retryAcceptBallot != acceptBallot { @@ -556,7 +570,9 @@ func TestOldConfigRecoveryRetryUsesPinnedVotersAfterRemoval(t *testing.T) { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() prepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, rd.Messages, MsgPrepare, ref, []ReplicaID{1, 3, 4}, 4) recoveryApplyReady(t, store, restarted, rd) @@ -567,7 +583,9 @@ func TestOldConfigRecoveryRetryUsesPinnedVotersAfterRemoval(t *testing.T) { for tick := uint64(1); tick < recoveryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery prepare retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } retry := restarted.Ready() retryPrepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, retry.Messages, MsgPrepare, ref, []ReplicaID{1, 3, 4}, 4) if retryPrepareBallot != prepareBallot { @@ -603,7 +621,9 @@ func TestOldConfigRecoveryRetryUsesPinnedVotersAfterRemoval(t *testing.T) { for tick := uint64(1); tick < retryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery accept retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } retry = restarted.Ready() retryAcceptBallot := recoveryRequireMessageTargetsWithDepsWidth(t, retry.Messages, MsgAccept, ref, []ReplicaID{1, 3, 4}, 4) if retryAcceptBallot != acceptBallot { @@ -642,7 +662,9 @@ func TestOldConfigRecoveryRetryCompletesAfterLostPreRetryAcceptResponse(t *testi recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() prepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, rd.Messages, MsgPrepare, ref, []ReplicaID{1, 3, 4}, 4) recoveryApplyReady(t, store, restarted, rd) @@ -681,7 +703,9 @@ func TestOldConfigRecoveryRetryCompletesAfterLostPreRetryAcceptResponse(t *testi for tick := uint64(1); tick < retryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery accept retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } retry := restarted.Ready() retryAcceptBallot := recoveryRequireMessageTargetsWithDepsWidth(t, retry.Messages, MsgAccept, ref, []ReplicaID{1, 3, 4}, 4) if retryAcceptBallot != acceptBallot { @@ -732,7 +756,9 @@ func TestOldConfigRecoveryRetryCompletesAfterLostPreRetryPrepareResponse(t *test recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() prepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, rd.Messages, MsgPrepare, ref, []ReplicaID{1, 3, 4}, 4) recoveryApplyReady(t, store, restarted, rd) @@ -752,7 +778,9 @@ func TestOldConfigRecoveryRetryCompletesAfterLostPreRetryPrepareResponse(t *test for tick := uint64(1); tick < recoveryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery prepare retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } retry := restarted.Ready() retryPrepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, retry.Messages, MsgPrepare, ref, []ReplicaID{1, 3, 4}, 4) if retryPrepareBallot != prepareBallot { @@ -820,7 +848,9 @@ func TestOldConfigRecoveryRetryUsesPinnedVotersAfterAddition(t *testing.T) { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() prepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, rd.Messages, MsgPrepare, ref, []ReplicaID{1, 3}, 3) recoveryApplyReady(t, store, restarted, rd) @@ -831,7 +861,9 @@ func TestOldConfigRecoveryRetryUsesPinnedVotersAfterAddition(t *testing.T) { for tick := uint64(1); tick < recoveryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery prepare retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } retry := restarted.Ready() retryPrepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, retry.Messages, MsgPrepare, ref, []ReplicaID{1, 3}, 3) if retryPrepareBallot != prepareBallot { @@ -862,7 +894,9 @@ func TestOldConfigRecoveryRetryUsesPinnedVotersAfterAddition(t *testing.T) { for tick := uint64(1); tick < retryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery accept retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } retry = restarted.Ready() retryAcceptBallot := recoveryRequireMessageTargetsWithDepsWidth(t, retry.Messages, MsgAccept, ref, []ReplicaID{1, 3}, 3) if retryAcceptBallot != acceptBallot { @@ -901,7 +935,9 @@ func TestOldConfigRecoveryUsesPinnedVotersAfterAddition(t *testing.T) { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config recovery before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() prepareBallot := recoveryRequireMessageTargetsWithDepsWidth(t, rd.Messages, MsgPrepare, ref, []ReplicaID{1, 3}, 3) recoveryApplyReady(t, store, restarted, rd) @@ -1010,7 +1046,9 @@ func TestOldConfigTransitionRetryUsesPinnedVotersAfterRemoval(t *testing.T) { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config transition retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() retry := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, tt.msgType, ref, []ReplicaID{2, 3, 4}, cmd, tt.deps) if retry.Ballot != tt.ballot { @@ -1095,7 +1133,9 @@ func TestOldConfigTransitionRetryUsesPinnedVotersAfterAddition(t *testing.T) { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config transition retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() retry := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, tt.msgType, ref, []ReplicaID{2, 3}, cmd, tt.deps) if retry.Ballot != tt.ballot { @@ -1207,7 +1247,9 @@ func TestOldConfigChainTransitionRetryUsesPinnedVotersAfterAddThenRemove(t *test recoveryPersistTickOnlyHardState(t, store, restarted, "old-config chain transition retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() retry := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, tt.msgType, tt.ref, tt.targets, cmd, tt.deps) if retry.Ballot != ballot { @@ -1332,7 +1374,7 @@ func TestOldConfigTransitionRetryCompletesAfterLostPreRetryResponses(t *testing. }) response := func(from ReplicaID, ballot Ballot) Message { - switch tt.status { + switch tt.status { //nolint:exhaustive // test table only constructs subset of Status values case StatusPreAccepted: return Message{Type: tt.responseType, From: from, To: 1, Ref: tt.ref, Ballot: ballot, Seq: tt.seq, Deps: append([]InstanceNum(nil), tt.deps...), FastPathEligible: true} case StatusAccepted: @@ -1364,7 +1406,9 @@ func TestOldConfigTransitionRetryCompletesAfterLostPreRetryResponses(t *testing. for tick := uint64(1); tick < retryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config transition retry before deadline after lost response") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() retry := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, tt.retryType, tt.ref, tt.retryTargets, cmd, tt.deps) if retry.Ballot != ballot { @@ -1386,7 +1430,7 @@ func TestOldConfigTransitionRetryCompletesAfterLostPreRetryResponses(t *testing. t.Fatal(err) } rd = restarted.Ready() - switch tt.status { + switch tt.status { //nolint:exhaustive // test table only constructs subset of Status values case StatusPreAccepted: rd = recoveryFinishPreAcceptedValidation(t, store, restarted, rd, tt.ref, tt.retryTargets, cmd, tt.seq, tt.deps) accept := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, MsgAccept, tt.ref, tt.retryTargets, cmd, tt.deps) @@ -1505,7 +1549,7 @@ func TestOldConfigChainTransitionRetryCompletesAfterLostPreRetryResponses(t *tes }) response := func(from ReplicaID, ballot Ballot) Message { - switch tt.status { + switch tt.status { //nolint:exhaustive // test table only constructs subset of Status values case StatusPreAccepted: return Message{Type: tt.responseType, From: from, To: 1, Ref: tt.ref, Ballot: ballot, Seq: tt.seq, Deps: append([]InstanceNum(nil), tt.deps...), FastPathEligible: true} case StatusAccepted: @@ -1548,7 +1592,9 @@ func TestOldConfigChainTransitionRetryCompletesAfterLostPreRetryResponses(t *tes for tick := uint64(1); tick < retryTicks; tick++ { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config chain transition retry before deadline after lost response") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() retry := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, tt.retryType, tt.ref, tt.retryTargets, cmd, tt.deps) if retry.Ballot != ballot { @@ -1571,7 +1617,7 @@ func TestOldConfigChainTransitionRetryCompletesAfterLostPreRetryResponses(t *tes t.Fatal(err) } rd = restarted.Ready() - switch tt.status { + switch tt.status { //nolint:exhaustive // test table only constructs subset of Status values case StatusPreAccepted: rd = recoveryFinishPreAcceptedValidation(t, store, restarted, rd, tt.ref, tt.retryTargets, cmd, tt.seq, tt.deps) accept := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, MsgAccept, tt.ref, tt.retryTargets, cmd, tt.deps) @@ -1654,7 +1700,9 @@ func TestOldConfigTransitionDedupUsesPinnedVotersAfterRemoval(t *testing.T) { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config transition removal retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() retry := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, tt.retryType, ref, []ReplicaID{2, 3, 4}, cmd, tt.deps) if retry.Seq != tt.seq { @@ -1666,7 +1714,7 @@ func TestOldConfigTransitionDedupUsesPinnedVotersAfterRemoval(t *testing.T) { recoveryRequireNoRecordOrApplicationEffects(t, restarted, rd, "old-config transition removal "+tt.name+" retry") recoveryApplyReady(t, store, restarted, rd) - switch tt.status { + switch tt.status { //nolint:exhaustive // test table only constructs subset of Status values case StatusPreAccepted: preAcceptResp := func(from ReplicaID) Message { return Message{Type: MsgPreAcceptResp, From: from, To: 1, Ref: ref, Ballot: retry.Ballot, Seq: retry.Seq, Deps: append([]InstanceNum(nil), retry.Deps...), FastPathEligible: true} @@ -1778,7 +1826,9 @@ func TestOldConfigTransitionDedupUsesPinnedVotersAfterAddition(t *testing.T) { recoveryPersistTickOnlyHardState(t, store, restarted, "old-config transition addition retry before deadline") } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() retry := recoveryRequireMessageTargetsWithCommandAndDeps(t, rd.Messages, tt.retryType, ref, []ReplicaID{2, 3}, cmd, tt.deps) if retry.Seq != tt.seq { @@ -1790,7 +1840,7 @@ func TestOldConfigTransitionDedupUsesPinnedVotersAfterAddition(t *testing.T) { recoveryRequireNoRecordOrApplicationEffects(t, restarted, rd, "old-config transition addition "+tt.name+" retry") recoveryApplyReady(t, store, restarted, rd) - switch tt.status { + switch tt.status { //nolint:exhaustive // test table only constructs subset of Status values case StatusPreAccepted: preAcceptResp := func(from ReplicaID) Message { return Message{Type: MsgPreAcceptResp, From: from, To: 1, Ref: ref, Ballot: retry.Ballot, Seq: retry.Seq, Deps: append([]InstanceNum(nil), retry.Deps...), FastPathEligible: true} @@ -2145,7 +2195,9 @@ func recoveryPersistInitialHardState(t *testing.T, store *MemoryStorage, rn *Raw func recoveryPersistTickOnlyHardState(t *testing.T, store *MemoryStorage, rn *RawNode, context string) { t.Helper() - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } rd := rn.Ready() recoveryRequireCurrentTickHardState(t, rn, rd, context) if len(rd.Records) != 0 || len(rd.Messages) != 0 || len(rd.Committed) != 0 { @@ -2209,7 +2261,7 @@ func recoveryRequireMessageTargetsWithDepsWidth(t *testing.T, messages []Message t.Fatalf("%s for %s sent to self: %#v", typ, ref, m) } seen[m.To] = m - switch typ { + switch typ { //nolint:exhaustive // test exercises subset of MessageType case MsgPrepare: if !commandEqual(m.Command, Command{}) || len(m.Deps) != 0 || @@ -2282,7 +2334,7 @@ func recoveryRequireMessageTargetsWithCommandAndDeps(t *testing.T, messages []Me if len(m.Deps) != len(wantDeps) || !instanceNumsEqual(m.Deps, wantDeps) { t.Fatalf("%s retry for %s deps = %v, want old config deps %v: %#v", typ, ref, m.Deps, wantDeps, m) } - switch typ { + switch typ { //nolint:exhaustive // test exercises subset of MessageType case MsgPreAccept, MsgAccept, MsgTryPreAccept: if m.RecordBallot != (Ballot{}) || m.RecordStatus != StatusNone { t.Fatalf("%s retry for %s exposed record metadata: %#v", typ, ref, m) @@ -2379,17 +2431,6 @@ func messageTargets(messages map[ReplicaID]Message) []ReplicaID { return targets } -func recoveryRequireDependencyRefs(t *testing.T, got []InstanceRef, want []InstanceRef) { - t.Helper() - if len(got) != len(want) { - t.Fatalf("dependency refs = %v, want %v", got, want) - } - for i := range want { - if got[i] != want[i] { - t.Fatalf("dependency refs = %v, want %v", got, want) - } - } -} func recoveryRequireNoPrepareRefs(t *testing.T, messages []Message, refs []InstanceRef) { t.Helper() @@ -2455,11 +2496,11 @@ func TestMaxUint64DependencyStartsBoundedExactRecovery(t *testing.T) { if err != nil { t.Fatal(err) } - max := ^InstanceNum(0) - dependentRef := InstanceRef{Conf: 1, Replica: 7, Instance: max} + maxInst := ^InstanceNum(0) + dependentRef := InstanceRef{Conf: 1, Replica: 7, Instance: maxInst} deps := make([]InstanceNum, 7) for i := range deps { - deps[i] = max + deps[i] = maxInst } commit := Message{ Type: MsgCommit, @@ -2470,7 +2511,7 @@ func TestMaxUint64DependencyStartsBoundedExactRecovery(t *testing.T) { RecordBallot: Ballot{Replica: 7}, Seq: 9, Deps: deps, - Command: Command{ID: CommandID{Client: 700, Sequence: 1}, Payload: []byte("max-dependent")}, + Command: Command{ID: CommandID{Client: 700, Sequence: 1}, Payload: []byte("maxInst-dependent")}, } if err := rn.Step(commit); err != nil { t.Fatalf("MaxUint64 dependency commit failed: %v", err) diff --git a/epaxos/remaining_test.go b/epaxos/remaining_test.go index f79e478..4e53cd7 100644 --- a/epaxos/remaining_test.go +++ b/epaxos/remaining_test.go @@ -196,7 +196,9 @@ func TestRemainingReadyAndProposalBranches(t *testing.T) { inst := &instance{rec: InstanceRecord{Ref: InstanceRef{Replica: 1, Instance: 9, Conf: 1}, Status: StatusCommitted, Deps: zr.q.deps(), Command: Command{Kind: CommandNoop}}, phase: phaseCommitted} zr.startAccept(inst, inst.rec.Attributes()) zr.commit(inst, inst.rec.Attributes()) - zr.schedule(inst, timerAccept, 0) + if err := zr.schedule(inst, timerAccept, 0); err != nil { + panic(err) + } } func TestProposeRejectsNonEmptyNoopWithoutDurableMutation(t *testing.T) { @@ -544,14 +546,18 @@ func TestTimeOptimizationDelaysSlowAcceptUntilFastWaitTick(t *testing.T) { } for tick := uint64(1); tick < 3; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } if inst.phase != phasePreAccept { t.Fatalf("phase after tick %d = %d, want preaccept before fast-wait deadline", tick, inst.phase) } remainingApplyHardStateOnly(t, rn, tick) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } if inst.phase != phaseAccept { t.Fatalf("phase at fast-wait deadline = %d, want accept", inst.phase) } @@ -614,7 +620,9 @@ func TestTimeOptimizationRetryCannotBypassFastWaitDeadline(t *testing.T) { } for tick := uint64(1); tick < 3; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } if inst.phase != phasePreAccept { t.Fatalf("phase after retry tick %d = %d, want preaccept", tick, inst.phase) } @@ -629,7 +637,9 @@ func TestTimeOptimizationRetryCannotBypassFastWaitDeadline(t *testing.T) { advanceOK(t, rn, retry) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } if inst.phase != phaseAccept { t.Fatalf("phase at fast-wait deadline = %d, want accept", inst.phase) } @@ -699,7 +709,9 @@ func TestTimeOptimizationLateFastQuorumCommitsWithoutAccept(t *testing.T) { } for tick := uint64(1); tick < 3; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } if inst.phase != phasePreAccept { t.Fatalf("phase after tick %d = %d, want preaccept before fast-wait deadline", tick, inst.phase) } @@ -1166,9 +1178,13 @@ func TestOutboundPreAcceptProcessAtCarriesCreatedTickOnRetries(t *testing.T) { } advanceOK(t, rn, rd) - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } remainingApplyHardStateOnly(t, rn, 1) - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } retry := rn.Ready() if len(retry.Records) != 0 || len(retry.Committed) != 0 { t.Fatalf("preaccept retry changed durable/application work: records=%#v committed=%#v", retry.Records, retry.Committed) @@ -1192,11 +1208,15 @@ func TestInboundFuturePreAcceptTimingWaitsForProcessAt(t *testing.T) { t.Fatalf("future preaccept produced ready work before ProcessAt: %#v", rn.Ready()) } for tick := uint64(1); tick < msg.ProcessAt; tick++ { - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } remainingApplyHardStateOnly(t, rn, tick) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } rd := rn.Ready() if len(rd.Records) != 1 || rd.Records[0].Ref != ref || rd.Records[0].Status != StatusPreAccepted { t.Fatalf("due preaccept records = %#v, want one pre-accepted record for %s", rd.Records, ref) @@ -1299,10 +1319,14 @@ func TestDuePreAcceptTimingOrdersByProcessAtAndRef(t *testing.T) { if rn.HasReady() { t.Fatalf("queued future preaccepts produced ready before due tick: %#v", rn.Ready()) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } remainingApplyHardStateOnly(t, rn, 1) - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } rd := rn.Ready() if got, want := recordRefs(rd.Records), []InstanceRef{first, second}; !slices.Equal(got, want) { t.Fatalf("records due at same ProcessAt arrived in order %v, want deterministic ref order %v; records=%#v", got, want, rd.Records) @@ -1315,7 +1339,9 @@ func TestDuePreAcceptTimingOrdersByProcessAtAndRef(t *testing.T) { } advanceOK(t, rn, rd) - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } laterReady := rn.Ready() if got, want := recordRefs(laterReady.Records), []InstanceRef{later}; !slices.Equal(got, want) { t.Fatalf("later ProcessAt records = %v, want only %s; records=%#v", got, later, laterReady.Records) @@ -1348,9 +1374,13 @@ func TestDeferredPreAcceptTimingClonesCommandBuffers(t *testing.T) { msg.Command.Payload[1] = 'Z' msg.Command.ConflictKeys[0][1] = 'W' - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } remainingApplyHardStateOnly(t, rn, 1) - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } rd := rn.Ready() if len(rd.Records) != 1 { t.Fatalf("deferred preaccept records = %#v, want one record", rd.Records) @@ -1801,9 +1831,13 @@ func TestRemainingResponseBranches(t *testing.T) { putAttrVoteSet(inst.preOK) inst.preOK = nil resp := Message{Type: MsgPreAcceptResp, From: 2, To: 1, Ref: ref, Seq: inst.rec.Seq, Deps: inst.rec.Deps} - rn.handlePreAcceptResp(resp) + if err := rn.handlePreAcceptResp(resp); err != nil { + panic(err) + } before := inst.preOK.len() - rn.handlePreAcceptResp(resp) + if err := rn.handlePreAcceptResp(resp); err != nil { + panic(err) + } if inst.preOK.len() != before { t.Fatal("duplicate preaccept response changed votes") } @@ -1818,7 +1852,9 @@ func TestRemainingResponseBranches(t *testing.T) { } inst := &instance{rec: rec} rn.instances[rec.Ref] = inst - rn.startPrepare(inst) + if err := rn.startPrepare(inst); err != nil { + panic(err) + } return rn, inst, inst.rec.Ballot } @@ -1831,12 +1867,16 @@ func TestRemainingResponseBranches(t *testing.T) { Command: Command{Payload: []byte("local")}, }) resp := Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: prepRef, RecordStatus: StatusAccepted, Ballot: ballot, RecordBallot: Ballot{Replica: 2}, Seq: 2, Deps: []InstanceNum{0, 2, 0, 0, 0}, Command: Command{Payload: []byte("accepted-2")}} - rn.handlePrepareResp(resp) + if err := rn.handlePrepareResp(resp); err != nil { + t.Fatal(err) + } if prep.phase != phasePrepare { t.Fatalf("single prepare response formed quorum: phase=%d", prep.phase) } votes := prep.prepareOK.len() - rn.handlePrepareResp(resp) + if err := rn.handlePrepareResp(resp); err != nil { + t.Fatal(err) + } if prep.phase != phasePrepare || prep.prepareOK.len() != votes { t.Fatalf("duplicate prepare response changed recovery state: phase=%d votes=%d want phase=%d votes=%d", prep.phase, prep.prepareOK.len(), phasePrepare, votes) } @@ -1844,7 +1884,9 @@ func TestRemainingResponseBranches(t *testing.T) { resp.Seq = 3 resp.Deps = []InstanceNum{0, 0, 3, 0, 0} resp.Command = Command{Payload: []byte("accepted-3")} - rn.handlePrepareResp(resp) + if err := rn.handlePrepareResp(resp); err != nil { + t.Fatal(err) + } if prep.phase != phaseAccept { t.Fatalf("distinct prepare quorum did not start accept: phase=%d", prep.phase) } @@ -1860,7 +1902,9 @@ func TestRemainingResponseBranches(t *testing.T) { Deps: []InstanceNum{7, 0, 0}, Command: Command{Payload: []byte("accepted-local")}, }) - rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: committedRef, RecordStatus: StatusCommitted, Ballot: ballot, RecordBallot: Ballot{Replica: 2}, Seq: 4, Deps: []InstanceNum{0, 0, 4}, Command: committedCommand}) + if err := rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: committedRef, RecordStatus: StatusCommitted, Ballot: ballot, RecordBallot: Ballot{Replica: 2}, Seq: 4, Deps: []InstanceNum{0, 0, 4}, Command: committedCommand}); err != nil { + t.Fatal(err) + } if prep.phase != phaseCommitted || prep.rec.Status != StatusCommitted || prep.rec.Seq != 4 || !bytes.Equal(prep.rec.Command.Payload, committedCommand.Payload) { t.Fatalf("committed prepare response not chosen: phase=%d record=%#v", prep.phase, prep.rec) } @@ -1876,7 +1920,9 @@ func TestRemainingResponseBranches(t *testing.T) { Deps: []InstanceNum{5, 0, 0}, Command: Command{Payload: []byte("preaccepted")}, }) - rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: prepRef, RecordStatus: StatusAccepted, Ballot: ballot, RecordBallot: Ballot{Replica: 2}, Seq: 4, Deps: []InstanceNum{0, 7, 0}, Command: acceptedCommand}) + if err := rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: prepRef, RecordStatus: StatusAccepted, Ballot: ballot, RecordBallot: Ballot{Replica: 2}, Seq: 4, Deps: []InstanceNum{0, 7, 0}, Command: acceptedCommand}); err != nil { + t.Fatal(err) + } if prep.phase != phaseAccept || prep.rec.Status != StatusAccepted || !bytes.Equal(prep.rec.Command.Payload, acceptedCommand.Payload) { t.Fatalf("accepted prepare response not selected over preaccepted: phase=%d record=%#v", prep.phase, prep.rec) } @@ -1888,7 +1934,9 @@ func TestRemainingResponseBranches(t *testing.T) { t.Run("non-owner none quorum starts noop accept", func(t *testing.T) { nonOwnerRef := InstanceRef{Replica: 2, Instance: 25, Conf: 1} rn, prep, ballot := startPrepared(t, 3, InstanceRecord{Ref: nonOwnerRef, Status: StatusNone}) - rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: nonOwnerRef, RecordStatus: StatusNone, Ballot: ballot, Deps: rn.q.deps()}) + if err := rn.handlePrepareResp(Message{Type: MsgPrepareResp, From: 2, To: 1, Ref: nonOwnerRef, RecordStatus: StatusNone, Ballot: ballot, Deps: rn.q.deps()}); err != nil { + t.Fatal(err) + } if prep.phase != phaseAccept || prep.rec.Status != StatusAccepted { t.Fatalf("non-owner StatusNone quorum did not start accept: phase=%d record=%#v", prep.phase, prep.rec) } @@ -2534,10 +2582,14 @@ func TestRestartRetransmitsLocalUncommittedInstances(t *testing.T) { } remainingApplyHardStateOnly(t, restarted, 0) for tick := uint64(1); tick < tt.ticks; tick++ { - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } remainingApplyHardStateOnly(t, restarted, tick) } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } rd := restarted.Ready() if len(rd.Messages) != 2 { t.Fatalf("retry messages = %#v", rd.Messages) @@ -2623,16 +2675,16 @@ func TestExecutedTrackerKeepsSparseExactHolesAndCheckedMax(t *testing.T) { one := InstanceRef{Conf: lane.conf, Replica: lane.replica, Instance: 1} two := InstanceRef{Conf: lane.conf, Replica: lane.replica, Instance: 2} three := InstanceRef{Conf: lane.conf, Replica: lane.replica, Instance: 3} - max := InstanceRef{Conf: lane.conf, Replica: lane.replica, Instance: ^InstanceNum(0)} + maxRef := InstanceRef{Conf: lane.conf, Replica: lane.replica, Instance: ^InstanceNum(0)} tracker.add(one) tracker.add(three) - tracker.add(max) + tracker.add(maxRef) if got := tracker.prefix(lane); got != 1 { t.Fatalf("sparse executed frontier = %d, want 1", got) } - if !tracker.contains(three) || !tracker.contains(max) || tracker.contains(two) { - t.Fatalf("sparse exact membership lost: one=%v two=%v three=%v max=%v", tracker.contains(one), tracker.contains(two), tracker.contains(three), tracker.contains(max)) + if !tracker.contains(three) || !tracker.contains(maxRef) || tracker.contains(two) { + t.Fatalf("sparse exact membership lost: one=%v two=%v three=%v maxRef=%v", tracker.contains(one), tracker.contains(two), tracker.contains(three), tracker.contains(maxRef)) } tracker.add(two) if got := tracker.prefix(lane); got != 3 { diff --git a/epaxos/revisited_test.go b/epaxos/revisited_test.go index 7f90714..53a5d2a 100644 --- a/epaxos/revisited_test.go +++ b/epaxos/revisited_test.go @@ -221,10 +221,10 @@ func TestRevisitedChainPruningDoesNotBypassUnknownDependency(t *testing.T) { func TestRevisitedSparseMaxPrefixPrunesOnlyExactWitness(t *testing.T) { rn := revisitedRawNode(t) - max := ^InstanceNum(0) + maxInstance := ^InstanceNum(0) base := InstanceRef{Conf: 1, Replica: 1, Instance: 1} - witness := InstanceRef{Conf: 1, Replica: 2, Instance: max} - revisitedInstall(rn, InstanceRecord{Ref: base, Status: StatusCommitted, Seq: 1, Deps: []InstanceNum{0, max, 0}, Command: Command{Kind: CommandNoop}}) + witness := InstanceRef{Conf: 1, Replica: 2, Instance: maxInstance} + revisitedInstall(rn, InstanceRecord{Ref: base, Status: StatusCommitted, Seq: 1, Deps: []InstanceNum{0, maxInstance, 0}, Command: Command{Kind: CommandNoop}}) revisitedInstall(rn, InstanceRecord{Ref: witness, Status: StatusCommitted, Seq: 2, Deps: []InstanceNum{1, 0, 0}, Command: Command{Kind: CommandNoop}}) view := rn.newExecutionView() component := revisitedComponentContaining(rn.executionComponents(&view), base) diff --git a/epaxos/sim_test.go b/epaxos/sim_test.go index be3b10d..b09390d 100644 --- a/epaxos/sim_test.go +++ b/epaxos/sim_test.go @@ -44,7 +44,7 @@ func newSimCluster(t *testing.T, n int, opt bool) *simCluster { func newCertifiedBootstrapSimCluster(t *testing.T, voters int) (*simCluster, ClusterID, []VoterIdentity) { t.Helper() ids := makeIDs(voters) - cluster := ClusterID{0x51, byte(voters)} + cluster := ClusterID{0x51, byte(voters)} //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. identities := make([]VoterIdentity, voters+1) for i := range identities { identities[i] = VoterIdentity{Replica: ReplicaID(i + 1), Incarnation: 1} @@ -140,9 +140,7 @@ func (s *simCluster) drain() { s.t.Fatalf("apply ready %d: %v", id, err) } s.committedCommands += uint64(len(rd.Committed)) - for _, c := range rd.Committed { - s.apps[id] = append(s.apps[id], c) - } + s.apps[id] = append(s.apps[id], rd.Committed...) for _, m := range rd.Messages { if !s.deliver(m) { s.delayed = append(s.delayed, m) @@ -182,7 +180,9 @@ func (s *simCluster) tickAll(n int) { for range n { for _, id := range s.ids() { if !s.paused[id] { - s.nodes[id].Tick() + if err := s.nodes[id].Tick(); err != nil { + panic(err) + } s.logicalTicks++ } } @@ -196,7 +196,9 @@ func (s *simCluster) tickOnly(id ReplicaID, n int) { s.t.Fatalf("tickOnly called on paused node %d", id) } for range n { - s.nodes[id].Tick() + if err := s.nodes[id].Tick(); err != nil { + panic(err) + } s.drain() } } @@ -207,7 +209,9 @@ func (s *simCluster) tickBurst(id ReplicaID, n int) { s.t.Fatalf("tickBurst called on paused node %d", id) } for range n { - s.nodes[id].Tick() + if err := s.nodes[id].Tick(); err != nil { + s.t.Fatal(err) + } } s.drain() } @@ -275,7 +279,7 @@ func TestClusterSizesOneThroughSevenCommit(t *testing.T) { for size := 1; size <= 7; size++ { t.Run(fmt.Sprintf("n=%d", size), func(t *testing.T) { s := newSimCluster(t, size, false) - _, err := s.nodes[1].Propose(Command{ID: CommandID{Client: 1, Sequence: uint64(size)}, Payload: []byte("set"), ConflictKeys: [][]byte{[]byte("k")}}) + _, err := s.nodes[1].Propose(Command{ID: CommandID{Client: 1, Sequence: uint64(size)}, Payload: []byte("set"), ConflictKeys: [][]byte{[]byte("k")}}) //nolint:gosec // G115: test harness converts bounded int index/count if err != nil { t.Fatal(err) } @@ -1226,8 +1230,12 @@ func TestPausedClockDoesNotTickOrProcessReadyUntilResume(t *testing.T) { t.Fatal(err) } for range 7 { - s.nodes[1].Tick() - s.nodes[3].Tick() + if err := s.nodes[1].Tick(); err != nil { + t.Fatal(err) + } + if err := s.nodes[3].Tick(); err != nil { + t.Fatal(err) + } s.drain() } if got := s.nodes[2].Status().Tick; got != pausedTick { @@ -1565,10 +1573,10 @@ func TestSparseMaxPrefixBuildsMaterializedSCCButBlocksOnFirstHole(t *testing.T) if err != nil { t.Fatal(err) } - max := ^InstanceNum(0) + maxInst := ^InstanceNum(0) left := InstanceRef{Conf: 1, Replica: 1, Instance: 1} - right := InstanceRef{Conf: 1, Replica: 2, Instance: max} - rn.installInstance(&instance{rec: InstanceRecord{Ref: left, Status: StatusCommitted, Seq: 1, Deps: []InstanceNum{0, max, 0}, Command: Command{Kind: CommandNoop}}}) + right := InstanceRef{Conf: 1, Replica: 2, Instance: maxInst} + rn.installInstance(&instance{rec: InstanceRecord{Ref: left, Status: StatusCommitted, Seq: 1, Deps: []InstanceNum{0, maxInst, 0}, Command: Command{Kind: CommandNoop}}}) rn.installInstance(&instance{rec: InstanceRecord{Ref: right, Status: StatusCommitted, Seq: 1, Deps: []InstanceNum{1, 0, 0}, Command: Command{Kind: CommandNoop}}}) view := rn.newExecutionView() components := rn.executionComponents(&view) diff --git a/epaxos/sparse_progress.go b/epaxos/sparse_progress.go index d39fde2..3d94849 100644 --- a/epaxos/sparse_progress.go +++ b/epaxos/sparse_progress.go @@ -397,9 +397,7 @@ func (n *RawNode) firstPrefixBlocker(base InstanceRef, lane instanceLane, throug if inst == nil || inst.rec.Status < StatusCommitted { return true, ref } - if n.dependencyKnownAfter(base, ref, StatusCommitted) { - discharged = true - } else { + if !n.dependencyKnownAfter(base, ref, StatusCommitted) { return true, InstanceRef{} } } @@ -675,6 +673,8 @@ func (n *RawNode) tryExecute() { inst.rec.MembershipResult, inst.rec.ConfChangeResult = n.applyMembershipControl(ref, inst.rec.Command) inst.rec.Checksum = ChecksumRecord(inst.rec) n.enqueueRecord(inst.rec) + case CommandNoop: + fallthrough default: inst.rec.Checksum = ChecksumRecord(inst.rec) n.enqueueRecord(inst.rec) diff --git a/epaxos/storage.go b/epaxos/storage.go index 3561a76..667235e 100644 --- a/epaxos/storage.go +++ b/epaxos/storage.go @@ -523,6 +523,7 @@ func validateLocalVoterState(state LocalVoterState) error { if contains { return fmt.Errorf("%w: ineligible voter remains present", ErrInvalidConfig) } + case LocalVoterStatusUnspecified: } return nil } diff --git a/epaxos/stress_test.go b/epaxos/stress_test.go index 951bfaf..a99868a 100644 --- a/epaxos/stress_test.go +++ b/epaxos/stress_test.go @@ -21,7 +21,7 @@ func (r *stressRNG) next() uint64 { } func (r *stressRNG) intn(n int) int { - return int(r.next() % uint64(n)) + return int(r.next() % uint64(n)) //nolint:gosec // G115: test harness converts bounded int index/count } type stressProposal struct { @@ -90,9 +90,7 @@ func (s *stressTransportCluster) captureReady(id ReplicaID) { if err := s.stores[id].ApplyReady(rd); err != nil { s.t.Fatalf("apply ready %d: %v", id, err) } - for _, c := range rd.Committed { - s.apps[id] = append(s.apps[id], c) - } + s.apps[id] = append(s.apps[id], rd.Committed...) s.pending = append(s.pending, rd.Messages...) if err := rn.Advance(rd); err != nil { s.t.Fatalf("advance %d: %v", id, err) @@ -155,11 +153,15 @@ func (s *stressTransportCluster) randomTransportStep() bool { func (s *stressTransportCluster) tickSome() { if s.rng.intn(3) == 0 { for _, id := range s.ids { - s.nodes[id].Tick() + if err := s.nodes[id].Tick(); err != nil { + s.t.Fatal(err) + } } return } - s.nodes[s.ids[s.rng.intn(len(s.ids))]].Tick() + if err := s.nodes[s.ids[s.rng.intn(len(s.ids))]].Tick(); err != nil { + s.t.Fatal(err) + } } func (s *stressTransportCluster) restart(id ReplicaID) { @@ -210,7 +212,9 @@ func (s *stressTransportCluster) driveUntilResolved(proposals []stressProposal) continue } for _, id := range s.ids { - s.nodes[id].Tick() + if err := s.nodes[id].Tick(); err != nil { + s.t.Fatal(err) + } } } s.t.Fatalf("cluster did not resolve %d proposed instances: counts=%#v pending=%d", len(proposals), s.appCounts(), len(s.pending)) @@ -275,7 +279,7 @@ func stressCommand(size, index int, rng *stressRNG) Command { keys = append(keys, []byte(fmt.Sprintf("pair-%d", index%2))) } return Command{ - ID: CommandID{Client: uint64(100 + index%size), Sequence: uint64(index + 1)}, + ID: CommandID{Client: uint64(100 + index%size), Sequence: uint64(index + 1)}, //nolint:gosec // G115: test harness converts bounded int index/count Payload: []byte(fmt.Sprintf("cmd-%02d", index)), ConflictKeys: keys, } diff --git a/epaxos/toq_test.go b/epaxos/toq_test.go index 8473290..4c47e9a 100644 --- a/epaxos/toq_test.go +++ b/epaxos/toq_test.go @@ -311,9 +311,13 @@ func TestTOQPendingRetryCannotStartAcceptBeforeProcessAt(t *testing.T) { t.Fatalf("preaccept votes = %d, want slow quorum %d", got, want) } - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } applyTOQHardStateOnly(t, store, rn, 1) - rn.Tick() + if err := rn.Tick(); err != nil { + t.Fatal(err) + } retry := rn.Ready() for _, msg := range retry.Messages { if msg.Type == MsgAccept { @@ -378,9 +382,13 @@ func TestTOQPendingLocalRestartWaitsAndAssignsAtProcessAt(t *testing.T) { t.Fatalf("restart conflict index before ProcessAt = %#v, want only preexisting conflict %s", byLane, conflictRef) } - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } applyTOQHardStateOnly(t, store, restarted, 1) - restarted.Tick() + if err := restarted.Tick(); err != nil { + t.Fatal(err) + } beforeProcessAtRetry := restarted.Ready() if len(beforeProcessAtRetry.Records) != 0 || len(beforeProcessAtRetry.Committed) != 0 { t.Fatalf("TOQ retry before ProcessAt produced durable/application work: records=%#v committed=%#v", beforeProcessAtRetry.Records, beforeProcessAtRetry.Committed) @@ -1603,6 +1611,8 @@ func rnDepsForValidation(typ MessageType, conf ConfState) []InstanceNum { switch typ { case MsgPreAcceptResp, MsgAccept, MsgCommit, MsgPrepareResp, MsgTryPreAccept, MsgTryPreAcceptResp: return make([]InstanceNum, len(conf.Voters)) + case MsgPreAccept, MsgAcceptResp, MsgPrepare, MsgEvidence, MsgEvidenceResp: + fallthrough default: return nil } diff --git a/epaxos/types.go b/epaxos/types.go index e98621b..0c001d4 100644 --- a/epaxos/types.go +++ b/epaxos/types.go @@ -36,7 +36,7 @@ func slicesSameStorageAndShape[T any](left, right []T) bool { if cap(left) == 0 { return true } - return unsafe.SliceData(left[:cap(left)]) == unsafe.SliceData(right[:cap(right)]) + return unsafe.SliceData(left[:cap(left)]) == unsafe.SliceData(right[:cap(right)]) //nolint:gosec // G103: unsafe pointer arithmetic is confined to overlap detection. } func slicesPartiallyOverlap[T any](dst, src []T) bool { @@ -47,8 +47,8 @@ func slicesPartiallyOverlap[T any](dst, src []T) bool { if size == 0 { return false } - dstStart := uintptr(unsafe.Pointer(unsafe.SliceData(dst[:cap(dst)]))) - srcStart := uintptr(unsafe.Pointer(unsafe.SliceData(src))) + dstStart := uintptr(unsafe.Pointer(unsafe.SliceData(dst[:cap(dst)]))) //nolint:gosec // G103: unsafe pointer arithmetic is confined to overlap detection. + srcStart := uintptr(unsafe.Pointer(unsafe.SliceData(src))) //nolint:gosec // G103: unsafe pointer arithmetic is confined to overlap detection. if dstStart == srcStart { return false } diff --git a/epaxos/vote_set.go b/epaxos/vote_set.go index daedeaf..d0fcbd7 100644 --- a/epaxos/vote_set.go +++ b/epaxos/vote_set.go @@ -46,7 +46,7 @@ func newAttrVote(conf ConfState, seq uint64, deps []InstanceNum, depsCommitted u if len(conf.Voters) < 1 || len(conf.Voters) > 7 || len(deps) != len(conf.Voters) { return attrVote{}, false } - vote := attrVote{seq: seq, width: uint8(len(deps)), depsCommitted: depsCommitted, fastPathEligible: fastPathEligible} + vote := attrVote{seq: seq, width: uint8(len(deps)), depsCommitted: depsCommitted, fastPathEligible: fastPathEligible} //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. copy(vote.deps[:], deps) return vote, true } diff --git a/epaxos/vote_set_test.go b/epaxos/vote_set_test.go index 98b0625..e532a8a 100644 --- a/epaxos/vote_set_test.go +++ b/epaxos/vote_set_test.go @@ -4,8 +4,8 @@ import "testing" func TestFixedWidthVoteSetsForAllSupportedConfigurations(t *testing.T) { for voters := 1; voters <= 7; voters++ { - t.Run(string(rune('0'+voters)), func(t *testing.T) { - conf := ConfState{ID: ConfID(10 + voters), Voters: makeIDs(voters)} + t.Run(string(rune('0'+voters)), func(t *testing.T) { //nolint:gosec // G115: test harness converts bounded int to rune + conf := ConfState{ID: ConfID(10 + voters), Voters: makeIDs(voters)} //nolint:gosec // G115: test harness converts bounded int index/count var attrs attrVoteSet var records recordVoteSet var mask voterMask From 4c54e3930293459f6a6db0d7edf9c996f7a159a3 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:36:38 +0900 Subject: [PATCH 04/12] style: remediate faultcampaign golangci findings --- tests/faultcampaign/artifacts.go | 13 +++- tests/faultcampaign/artifacts_test.go | 6 ++ tests/faultcampaign/campaign.go | 91 +++++++++++++++++++-------- tests/faultcampaign/history.go | 1 + tests/faultcampaign/invariants.go | 1 + tests/faultcampaign/main.go | 7 ++- tests/faultcampaign/native_darwin.go | 15 ----- tests/faultcampaign/native_other.go | 10 +-- tests/faultcampaign/process.go | 13 ++-- tests/faultcampaign/proxy.go | 17 ++--- tests/faultcampaign/proxy_test.go | 16 +++-- tests/faultcampaign/trace.go | 5 +- tests/faultcampaign/trace_test.go | 1 + 13 files changed, 126 insertions(+), 70 deletions(-) diff --git a/tests/faultcampaign/artifacts.go b/tests/faultcampaign/artifacts.go index 973a2a3..f41894a 100644 --- a/tests/faultcampaign/artifacts.go +++ b/tests/faultcampaign/artifacts.go @@ -1,3 +1,4 @@ +// Package main implements the fault campaign tests. package main import ( @@ -18,6 +19,7 @@ func ensureArtifactDir(path string) (string, error) { if err != nil { return "", err } + //nolint:gosec // G302: directory requires execute bit if err := os.Chmod(created, 0o700); err != nil { return "", err } @@ -39,6 +41,7 @@ func ensureArtifactDir(path string) (string, error) { } else if err := os.MkdirAll(cleaned, 0o700); err != nil { return "", err } + //nolint:gosec // G302: directory requires execute bit if err := os.Chmod(cleaned, 0o700); err != nil { return "", err } @@ -97,11 +100,14 @@ func writeBytesDurable(path string, payload []byte) error { return err } committed = true + //nolint:gosec // G304: dir is controlled artifact path directory, err := os.Open(dir) if err != nil { return err } - defer directory.Close() + defer func() { + _ = directory.Close() + }() return directory.Sync() } @@ -113,11 +119,14 @@ func fileSHA256(path string) (string, error) { if !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 { return "", fmt.Errorf("hash target %s must be a regular non-symlink file", path) } + //nolint:gosec // G304: path is controlled artifact path file, err := os.Open(path) if err != nil { return "", err } - defer file.Close() + defer func() { + _ = file.Close() + }() hasher := sha256.New() if _, err := io.Copy(hasher, file); err != nil { return "", err diff --git a/tests/faultcampaign/artifacts_test.go b/tests/faultcampaign/artifacts_test.go index 83cea37..55cc07f 100644 --- a/tests/faultcampaign/artifacts_test.go +++ b/tests/faultcampaign/artifacts_test.go @@ -1,3 +1,4 @@ +// Package main implements the fault campaign tests. package main import ( @@ -16,6 +17,7 @@ func TestDurableJSONRetainsPriorArtifactOnEncodingFailure(t *testing.T) { if err := writeJSONDurable(path, map[string]string{"status": "retained"}); err != nil { t.Fatal(err) } + //nolint:gosec // G304: path is controlled test path before, err := os.ReadFile(path) if err != nil { t.Fatal(err) @@ -23,6 +25,7 @@ func TestDurableJSONRetainsPriorArtifactOnEncodingFailure(t *testing.T) { if err := writeJSONDurable(path, map[string]any{"unsupported": make(chan int)}); err == nil { t.Fatal("encoding failure was not reported") } + //nolint:gosec // G304: path is controlled test path after, err := os.ReadFile(path) if err != nil { t.Fatalf("prior artifact was deleted after failure: %v", err) @@ -90,6 +93,7 @@ func TestChecksumsAreSortedDeterministicAndDetectContentChanges(t *testing.T) { if err := writeChecksums(root, []string{"b.log", "a.json"}); err != nil { t.Fatal(err) } + //nolint:gosec // G304: path is controlled test path first, err := os.ReadFile(filepath.Join(root, "checksums.sha256")) if err != nil { t.Fatal(err) @@ -101,6 +105,7 @@ func TestChecksumsAreSortedDeterministicAndDetectContentChanges(t *testing.T) { if err := writeChecksums(root, []string{"a.json", "b.log"}); err != nil { t.Fatal(err) } + //nolint:gosec // G304: path is controlled test path second, _ := os.ReadFile(filepath.Join(root, "checksums.sha256")) if string(first) != string(second) { t.Fatalf("deterministic checksum file changed:\nfirst=%s\nsecond=%s", first, second) @@ -111,6 +116,7 @@ func TestChecksumsAreSortedDeterministicAndDetectContentChanges(t *testing.T) { if err := writeChecksums(root, []string{"a.json", "b.log"}); err != nil { t.Fatal(err) } + //nolint:gosec // G304: path is controlled test path third, _ := os.ReadFile(filepath.Join(root, "checksums.sha256")) if string(first) == string(third) { t.Fatal("checksum evidence did not change after artifact tampering") diff --git a/tests/faultcampaign/campaign.go b/tests/faultcampaign/campaign.go index 6a1ea28..f993cbb 100644 --- a/tests/faultcampaign/campaign.go +++ b/tests/faultcampaign/campaign.go @@ -89,7 +89,9 @@ func runCampaigns(cfg runnerConfig, stdout io.Writer) error { if len(entries) != 0 { return fmt.Errorf("refusing to overwrite nonempty campaign artifact root %s", artifactRoot) } - fmt.Fprintf(stdout, "faultcampaign phase=artifacts path=%s\n", artifactRoot) + if _, err := fmt.Fprintf(stdout, "faultcampaign phase=artifacts path=%s\n", artifactRoot); err != nil { + return fmt.Errorf("write campaign status: %w", err) + } buildContext, cancel := context.WithTimeout(context.Background(), 3*time.Minute) build, err := buildCampaignBinaries(buildContext, artifactRoot) cancel() @@ -145,7 +147,9 @@ func runCampaigns(cfg runnerConfig, stdout io.Writer) error { if status != "reproduced" { return fmt.Errorf("replay explicitly reported nondeterminism: %s", reason) } - fmt.Fprintf(stdout, "faultcampaign replay=pass digest=%s artifacts=%s\n", observed.TerminalDigest, caseDir) + if _, err := fmt.Fprintf(stdout, "faultcampaign replay=pass digest=%s artifacts=%s\n", observed.TerminalDigest, caseDir); err != nil { + return fmt.Errorf("write campaign status: %w", err) + } return nil } @@ -155,15 +159,21 @@ func runCampaigns(cfg runnerConfig, stdout io.Writer) error { if err != nil { return err } - fmt.Fprintf(stdout, "faultcampaign phase=case size=%d profile=%s artifacts=%s\n", size, profile, caseDir) + if _, err := fmt.Fprintf(stdout, "faultcampaign phase=case size=%d profile=%s artifacts=%s\n", size, profile, caseDir); err != nil { + return fmt.Errorf("write campaign status: %w", err) + } trace, err := runOneCampaign(cfg, build, size, profile, caseDir, nil) if err != nil { - return fmt.Errorf("N=%d profile=%s: %w", size, profile, err) + return fmt.Errorf("n=%d profile=%s: %w", size, profile, err) + } + if _, err := fmt.Fprintf(stdout, "faultcampaign case=pass size=%d profile=%s digest=%s\n", size, profile, trace.TerminalDigest); err != nil { + return fmt.Errorf("write campaign status: %w", err) } - fmt.Fprintf(stdout, "faultcampaign case=pass size=%d profile=%s digest=%s\n", size, profile, trace.TerminalDigest) } } - fmt.Fprintf(stdout, "faultcampaign status=pass artifacts=%s\n", artifactRoot) + if _, err := fmt.Fprintf(stdout, "faultcampaign status=pass artifacts=%s\n", artifactRoot); err != nil { + return fmt.Errorf("write campaign status: %w", err) + } return nil } @@ -188,6 +198,7 @@ func runOneCampaign(cfg runnerConfig, build buildArtifacts, size int, profile, c cfg: cfg, build: build, size: size, profile: profile, caseDir: caseDir, recorder: newHistoryRecorder(cfg.requestTimeout), prefix: fmt.Sprintf("fc-%d-%s-%x", size, strings.ReplaceAll(profile, "-", "_"), cfg.seed), + //nolint:gosec // G404: weak random is fine for test generation rng: rand.New(rand.NewPCG(cfg.seed, uint64(size)<<32|uint64(len(profile)))), } manifest := state.manifest("starting", "") @@ -362,18 +373,19 @@ func (r *historyRecorder) execute(node int, baseURL string, operation HistoryEve } else { switch operation.Kind { case OpGet: - if status == http.StatusOK { + switch status { + case http.StatusOK: operation.Result = ResultOK operation.Found = true operation.Value = string(responseBody) - } else if status == http.StatusNotFound { + case http.StatusNotFound: operation.Result = ResultOK operation.Found = false operation.Value = "" - } else { + default: operation.Result = ResultFail } - default: + case OpPut, OpDelete, OpTxn: if status == http.StatusNoContent { operation.Result = ResultOK } else { @@ -446,23 +458,29 @@ func (s *campaignState) runBaseline() error { return err } read := s.get(1, alpha) - if err := requireAcknowledged(read, "baseline read"); err != nil || !read.Found || read.Value != "one" { - return fmt.Errorf("baseline read mismatch: event=%#v error=%v", read, err) + if err := requireAcknowledged(read, "baseline read"); err != nil { + return fmt.Errorf("baseline read mismatch: event=%#v error=%w", read, err) + } else if !read.Found || read.Value != "one" { + return fmt.Errorf("baseline read mismatch: event=%#v", read) } transaction := []TxnOperation{{Key: beta, Value: "two"}, {Key: gamma, Value: "three"}} if err := requireAcknowledged(s.txn(1, transaction), "baseline transaction"); err != nil { return err } read = s.get(1, beta) - if err := requireAcknowledged(read, "baseline transaction read"); err != nil || !read.Found || read.Value != "two" { - return fmt.Errorf("baseline transaction read mismatch: event=%#v error=%v", read, err) + if err := requireAcknowledged(read, "baseline transaction read"); err != nil { + return fmt.Errorf("baseline transaction read mismatch: event=%#v error=%w", read, err) + } else if !read.Found || read.Value != "two" { + return fmt.Errorf("baseline transaction read mismatch: event=%#v", read) } if err := requireAcknowledged(s.delete(1, alpha), "baseline delete"); err != nil { return err } read = s.get(1, alpha) - if err := requireAcknowledged(read, "baseline deleted read"); err != nil || read.Found { - return fmt.Errorf("baseline delete was not observable: event=%#v error=%v", read, err) + if err := requireAcknowledged(read, "baseline deleted read"); err != nil { + return fmt.Errorf("baseline delete was not observable: event=%#v error=%w", read, err) + } else if read.Found { + return fmt.Errorf("baseline delete was not observable: event=%#v", read) } return nil } @@ -614,6 +632,7 @@ func (s *campaignState) runAsymmetricPartition() error { f := s.size - slow if f > 0 { for offset := range f { + //nolint:gosec // G115: size conversion is safe from := uint64(s.size - offset) if err := s.setDirectedDrop(1, from, 1, true, "asymmetric-partition-progress"); err != nil { return err @@ -627,6 +646,7 @@ func (s *campaignState) runAsymmetricPartition() error { } } for offset := range s.size - 1 { + //nolint:gosec // G115: size conversion is safe from := uint64(s.size - offset) if err := s.setDirectedDrop(1, from, 1, true, "asymmetric-partition-fail-closed"); err != nil { return err @@ -865,6 +885,7 @@ func (s *campaignState) runMalformedFrames() error { {kind: "oversized-frame", body: bytes.Repeat([]byte{0xff}, (2<<20)+1), want: http.StatusRequestEntityTooLarge}, } for _, testCase := range cases { + //nolint:gosec // G115: safe conversion action := s.planAction(testCase.kind, target.id, 0, uint64(target.id), true, "direct peer listener hardening probe") status, response, err := rawHTTPRequest(s.recorder.client, http.MethodPost, target.peerURL+"/epaxos/message", testCase.body, "application/octet-stream") if err == nil && status != testCase.want { @@ -908,6 +929,7 @@ func (s *campaignState) runOverload() error { results := make(chan HistoryEvent, operations) var workers sync.WaitGroup for operation := range operations { + //nolint:gosec // G115: s.size is positive node := 1 + int(s.rng.Uint64N(uint64(s.size))) workers.Add(1) go func(index, nodeID int) { @@ -1006,8 +1028,11 @@ func (s *campaignState) runAdmissionPressure() error { snapshots := make([]admissionSnapshot, 0, len(s.cluster.nodes)) for _, node := range s.cluster.nodes { status, body, err := rawHTTPRequest(s.recorder.client, http.MethodGet, node.adminURL+"/metrics", nil, "") - if err != nil || status != http.StatusOK { - return nil, fmt.Errorf("node %d admission metrics status=%d error=%v", node.id, status, err) + if err != nil { + return nil, fmt.Errorf("node %d admission metrics: %w", node.id, err) + } + if status != http.StatusOK { + return nil, fmt.Errorf("node %d admission metrics status=%d", node.id, status) } metrics := string(body) queued, err := prometheusUint(metrics, "kvnode_transport_queued_frames") @@ -1027,6 +1052,7 @@ func (s *campaignState) runAdmissionPressure() error { return nil, err } snapshots = append(snapshots, admissionSnapshot{blocked: blocked, queued: queued, inflight: inflight, retrying: retrying}) + //nolint:gosec // G115: capacity is positive if queued+inflight > uint64(capacity) || retrying > uint64(capacity) { return nil, fmt.Errorf("node %d exceeded transport ownership bounds queued=%d inflight=%d retry=%d", node.id, queued, inflight, retrying) } @@ -1116,8 +1142,11 @@ func (s *campaignState) runRetentionPressure() error { return fmt.Errorf("retention pressure write failed unexpectedly: %#v", event) } status, body, err := rawHTTPRequest(s.recorder.client, http.MethodGet, s.cluster.nodes[0].adminURL+"/metrics", nil, "") - if err != nil || status != http.StatusOK { - return fmt.Errorf("retention metrics status=%d error=%v", status, err) + if err != nil { + return fmt.Errorf("retention metrics error: %w", err) + } + if status != http.StatusOK { + return fmt.Errorf("retention metrics status=%d", status) } level, err := prometheusUint(string(body), "kvnode_retention_level") if err != nil { @@ -1142,8 +1171,11 @@ func (s *campaignState) runRetentionPressure() error { return fmt.Errorf("retention pressure accepted new proposal: %#v", rejected) } status, afterBody, err := rawHTTPRequest(s.recorder.client, http.MethodGet, s.cluster.nodes[0].adminURL+"/metrics", nil, "") - if err != nil || status != http.StatusOK { - return fmt.Errorf("retention post-rejection metrics status=%d error=%v", status, err) + if err != nil { + return fmt.Errorf("retention post-rejection metrics error: %w", err) + } + if status != http.StatusOK { + return fmt.Errorf("retention post-rejection metrics status=%d", status) } after, err := prometheusUint(string(afterBody), "kvnode_storage_durable_instance_records") if err != nil { @@ -1153,8 +1185,11 @@ func (s *campaignState) runRetentionPressure() error { return fmt.Errorf("durable instance count decreased under retention pressure: before=%d after=%d", before, after) } status, _, err = rawHTTPRequest(s.recorder.client, http.MethodGet, s.cluster.nodes[0].adminURL+"/livez", nil, "") - if err != nil || status != http.StatusOK { - return fmt.Errorf("retention pressure liveness status=%d error=%v", status, err) + if err != nil { + return fmt.Errorf("retention pressure liveness error: %w", err) + } + if status != http.StatusOK { + return fmt.Errorf("retention pressure liveness status=%d", status) } s.receipt(action, "bounded-retention-pressure", uint64(operation+1), true, "") return nil @@ -1215,7 +1250,10 @@ func (s *campaignState) postHealCanary() error { cancel() if err != nil { status, body, readyErr := rawHTTPRequest(s.recorder.client, http.MethodGet, s.cluster.nodes[0].adminURL+"/readyz", nil, "") - return fmt.Errorf("%w; readiness status=%d body=%q error=%v", err, status, strings.TrimSpace(string(body)), readyErr) + if readyErr != nil { + return fmt.Errorf("%w; readiness error: %w", err, readyErr) + } + return fmt.Errorf("%w; readiness status=%d body=%q", err, status, strings.TrimSpace(string(body))) } for _, node := range s.cluster.nodes { var event HistoryEvent @@ -1248,6 +1286,7 @@ func (s *campaignState) observeUnknownMutations() error { for _, operation := range event.Txn { keys[operation.Key] = struct{}{} } + case OpGet: } } ordered := make([]string, 0, len(keys)) @@ -1273,6 +1312,7 @@ func (s *campaignState) verifyTerminalOnAll(terminal map[string]string) error { for _, operation := range event.Txn { keys[operation.Key] = struct{}{} } + case OpGet: } } for _, node := range s.cluster.nodes { @@ -1340,6 +1380,7 @@ func (s *campaignState) runCheckpointCommand(label string, arguments ...string) } context, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() + //nolint:gosec // G204: command execution is controlled and validated command := exec.CommandContext(context, argv[0], argv[1:]...) command.Env = append(os.Environ(), "KVNODE_CHECKPOINT_REPORT="+reportPath) output, err := command.CombinedOutput() diff --git a/tests/faultcampaign/history.go b/tests/faultcampaign/history.go index 2b1d7a3..9308614 100644 --- a/tests/faultcampaign/history.go +++ b/tests/faultcampaign/history.go @@ -241,6 +241,7 @@ func applyMutation(e HistoryEvent, state map[string]string) { state[op.Key] = op.Value } } + case OpGet: } } diff --git a/tests/faultcampaign/invariants.go b/tests/faultcampaign/invariants.go index 8985c7c..1f5d349 100644 --- a/tests/faultcampaign/invariants.go +++ b/tests/faultcampaign/invariants.go @@ -125,6 +125,7 @@ func inspectDurableCluster(nodes []*nodeProcess, expectedAcknowledgedMutations i } for nodeIndex := 1; nodeIndex < len(allConfigs); nodeIndex++ { if !sameDurableConfigurations(allConfigs[0], allConfigs[nodeIndex]) { + //nolint:gosec // G602: length of allConfigs is len(nodes) result.Error = fmt.Sprintf("node %d historical voter ordering diverges", nodes[nodeIndex].id) return result } diff --git a/tests/faultcampaign/main.go b/tests/faultcampaign/main.go index a9800d3..c2d6504 100644 --- a/tests/faultcampaign/main.go +++ b/tests/faultcampaign/main.go @@ -55,16 +55,16 @@ func run(args []string, stdout, stderr io.Writer) int { return 0 } if err != nil { - fmt.Fprintf(stderr, "faultcampaign: %v\n", err) + _, _ = fmt.Fprintf(stderr, "faultcampaign: %v\n", err) return 2 } host := hostEnvironment{GOOS: runtime.GOOS, EUID: os.Geteuid(), Lookup: os.Getenv} if err := validateNativeDarwinHost(host); err != nil { - fmt.Fprintf(stderr, "faultcampaign: %v\n", err) + _, _ = fmt.Fprintf(stderr, "faultcampaign: %v\n", err) return 2 } if err := runCampaigns(cfg, stdout); err != nil { - fmt.Fprintf(stderr, "faultcampaign status=fail error=%v\n", err) + _, _ = fmt.Fprintf(stderr, "faultcampaign status=fail error=%v\n", err) return 1 } return 0 @@ -216,6 +216,7 @@ func runExternalOutput(ctx context.Context, argv []string, dir string) ([]byte, if err := validateExternalCommand(argv); err != nil { return nil, err } + //nolint:gosec // G204: command is validated via validateExternalCommand command := exec.CommandContext(ctx, argv[0], argv[1:]...) command.Dir = dir output, err := command.CombinedOutput() diff --git a/tests/faultcampaign/native_darwin.go b/tests/faultcampaign/native_darwin.go index cfd4d8d..5e9ec74 100644 --- a/tests/faultcampaign/native_darwin.go +++ b/tests/faultcampaign/native_darwin.go @@ -9,23 +9,8 @@ import ( "os/exec" "strconv" "strings" - "syscall" ) -func pausePID(pid int) error { - if pid <= 0 { - return fmt.Errorf("invalid PID %d", pid) - } - return syscall.Kill(pid, syscall.SIGSTOP) -} - -func resumePID(pid int) error { - if pid <= 0 { - return fmt.Errorf("invalid PID %d", pid) - } - return syscall.Kill(pid, syscall.SIGCONT) -} - func sampleDarwinProcess(ctx context.Context, pid int) (rssKiB uint64, fdCount int, err error) { if pid <= 0 { return 0, 0, fmt.Errorf("invalid PID %d", pid) diff --git a/tests/faultcampaign/native_other.go b/tests/faultcampaign/native_other.go index 6cfd0d9..1044d68 100644 --- a/tests/faultcampaign/native_other.go +++ b/tests/faultcampaign/native_other.go @@ -7,14 +7,6 @@ import ( "fmt" ) -func pausePID(pid int) error { - return fmt.Errorf("process pause is available only on native Darwin") -} - -func resumePID(pid int) error { - return fmt.Errorf("process resume is available only on native Darwin") -} - -func sampleDarwinProcess(ctx context.Context, pid int) (uint64, int, error) { +func sampleDarwinProcess(_ context.Context, _ int) (uint64, int, error) { return 0, 0, fmt.Errorf("process resource sampling is available only on native Darwin") } diff --git a/tests/faultcampaign/process.go b/tests/faultcampaign/process.go index 1f23f3a..8ec9e04 100644 --- a/tests/faultcampaign/process.go +++ b/tests/faultcampaign/process.go @@ -167,6 +167,7 @@ func sourceTreeSHA256(roots []string) (string, error) { sort.Strings(files) hasher := sha256.New() for _, path := range files { + //nolint:gosec // G304: path is controlled test path payload, err := os.ReadFile(path) if err != nil { return "", err @@ -231,6 +232,7 @@ func startNativeCluster(caseDir string, size int, binary string, timeout time.Du } upstream, _ := url.Parse("http://" + ports.addresses[size+id-1]) proxyLogPath := filepath.Join(logDir, fmt.Sprintf("proxy-%d.jsonl", id)) + //nolint:gosec // G304: path is controlled test path proxyLog, err := os.OpenFile(proxyLogPath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600) if err != nil { _ = proxyListener.Close() @@ -312,6 +314,7 @@ func startNodeProcess(node *nodeProcess) error { if err != nil { return err } + //nolint:gosec // G204: binary and args are validated via validateExternalCommand command := exec.Command(node.binary, node.args...) command.Stdout = logFile command.Stderr = logFile @@ -365,11 +368,11 @@ func waitCondition(ctx context.Context, interval time.Duration, condition func() defer ticker.Stop() var last error for { - if err := condition(); err == nil { + err := condition() + if err == nil { return nil - } else { - last = err } + last = err select { case <-ctx.Done(): return fmt.Errorf("condition timeout: %w", last) @@ -514,7 +517,9 @@ func rawHTTPRequest(client *http.Client, method, target string, body []byte, con if err != nil { return 0, nil, err } - defer response.Body.Close() + defer func() { + _ = response.Body.Close() + }() responseBody, err := ioReadAllBounded(response.Body, 4<<20) return response.StatusCode, responseBody, err } diff --git a/tests/faultcampaign/proxy.go b/tests/faultcampaign/proxy.go index cc8ea36..2895f73 100644 --- a/tests/faultcampaign/proxy.go +++ b/tests/faultcampaign/proxy.go @@ -11,6 +11,7 @@ import ( "net/url" "sort" "sync" + "time" "gosuda.org/moreconsensus/epaxos" ) @@ -82,7 +83,10 @@ func newFaultProxy(listener net.Listener, upstream *url.URL, logWriter io.Writer actionIDs: make(map[uint64]struct{}), held: make(map[uint64]heldRequest), } - proxy.server = &http.Server{Handler: proxy} + proxy.server = &http.Server{ + Handler: proxy, + ReadHeaderTimeout: 5 * time.Second, + } return proxy } @@ -344,6 +348,7 @@ func (p *FaultProxy) ServeHTTP(w http.ResponseWriter, request *http.Request) { } copyHTTPHeader(w.Header(), firstHeader) w.WriteHeader(firstStatus) + //nolint:gosec // G705: XSS is expected in fault proxy forwarding _, _ = w.Write(firstBody) } @@ -360,11 +365,6 @@ func (p *FaultProxy) takeMatchingActionLocked(decoded bool, from, to uint64) (Pr return ProxyAction{}, false } -func (p *FaultProxy) forward(method, requestURI string, header http.Header, body []byte) (int, []byte, error) { - status, _, responseBody, err := p.forwardWithHeader(method, requestURI, header, body) - return status, responseBody, err -} - func (p *FaultProxy) forwardWithHeader(method, requestURI string, header http.Header, body []byte) (int, http.Header, []byte, error) { target := *p.upstream relative, err := url.Parse(requestURI) @@ -379,11 +379,14 @@ func (p *FaultProxy) forwardWithHeader(method, requestURI string, header http.He return 0, nil, nil, err } copyHTTPHeader(forwardRequest.Header, header) + //nolint:gosec // G704: SSRF is intentional in proxy forwarding response, err := p.client.Do(forwardRequest) if err != nil { return 0, nil, nil, err } - defer response.Body.Close() + defer func() { + _ = response.Body.Close() + }() responseBody, err := io.ReadAll(response.Body) if err != nil { return response.StatusCode, response.Header.Clone(), nil, err diff --git a/tests/faultcampaign/proxy_test.go b/tests/faultcampaign/proxy_test.go index e73d687..9ce8e29 100644 --- a/tests/faultcampaign/proxy_test.go +++ b/tests/faultcampaign/proxy_test.go @@ -76,7 +76,9 @@ func (f *proxyFixture) post(t *testing.T, body []byte) int { if err != nil { t.Fatal(err) } - defer response.Body.Close() + defer func() { + _ = response.Body.Close() + }() _, _ = io.Copy(io.Discard, response.Body) return response.StatusCode } @@ -167,7 +169,9 @@ func TestFaultProxyBlockKeepsAttemptOpenUntilRelease(t *testing.T) { result <- postResult{err: err} return } - defer response.Body.Close() + defer func() { + _ = response.Body.Close() + }() _, _ = io.Copy(io.Discard, response.Body) result <- postResult{status: response.StatusCode} }() @@ -287,7 +291,9 @@ func TestFaultProxyDuplicateFailsIfEitherForwardFails(t *testing.T) { if err := proxy.Start(); err != nil { t.Fatal(err) } - defer proxy.Close() + defer func() { + _ = proxy.Close() + }() if err := proxy.Schedule(ProxyAction{ID: 1, Kind: "duplicate", From: 1, To: 2}); err != nil { t.Fatal(err) } @@ -295,7 +301,9 @@ func TestFaultProxyDuplicateFailsIfEitherForwardFails(t *testing.T) { if err != nil { t.Fatal(err) } - defer response.Body.Close() + defer func() { + _ = response.Body.Close() + }() if response.StatusCode != http.StatusBadGateway || calls != 2 { t.Fatalf("duplicate status=%d calls=%d, want 502 and two attempts", response.StatusCode, calls) } diff --git a/tests/faultcampaign/trace.go b/tests/faultcampaign/trace.go index 993f87d..acb38f9 100644 --- a/tests/faultcampaign/trace.go +++ b/tests/faultcampaign/trace.go @@ -207,11 +207,14 @@ func isPeerFault(kind string) bool { } func readTrace(path string) (FaultTrace, error) { + //nolint:gosec // G304: path is controlled trace path file, err := os.Open(path) if err != nil { return FaultTrace{}, err } - defer file.Close() + defer func() { + _ = file.Close() + }() decoder := json.NewDecoder(file) decoder.DisallowUnknownFields() var trace FaultTrace diff --git a/tests/faultcampaign/trace_test.go b/tests/faultcampaign/trace_test.go index 4b2fc94..de67d1d 100644 --- a/tests/faultcampaign/trace_test.go +++ b/tests/faultcampaign/trace_test.go @@ -146,6 +146,7 @@ func TestReadTraceRejectsTrailingJSON(t *testing.T) { if err := writeTraceDurable(path, validTestTrace(3)); err != nil { t.Fatal(err) } + //nolint:gosec // G304: path is controlled test path file, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0) if err != nil { t.Fatal(err) From 03722117946648e9f26018a823cf7124eb70ab21 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:24:00 +0900 Subject: [PATCH 05/12] style: remediate lifecyclecollector golangci findings --- tests/lifecyclecollector/collect.go | 39 +++++++++++++++++----- tests/lifecyclecollector/collector_test.go | 12 ++----- tests/lifecyclecollector/finalize.go | 35 ++++++++++++++----- tests/lifecyclecollector/main.go | 9 ----- tests/lifecyclecollector/model.go | 18 ++++++---- 5 files changed, 72 insertions(+), 41 deletions(-) diff --git a/tests/lifecyclecollector/collect.go b/tests/lifecyclecollector/collect.go index c5036a3..6a88c41 100644 --- a/tests/lifecyclecollector/collect.go +++ b/tests/lifecyclecollector/collect.go @@ -456,10 +456,12 @@ func (controller *processController) startDirect(index int) error { return err } logPath := filepath.Join(logDir, fmt.Sprintf("node%d.log", index+1)) + //nolint:gosec // G304: test staging log path under controller control log, err := os.OpenFile(logPath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600) if err != nil { return err } + //nolint:gosec // G204: subprocess launched with controlled argv command := exec.Command(argv[0], argv[1:]...) command.Stdout, command.Stderr = log, log if err := command.Start(); err != nil { @@ -729,12 +731,16 @@ func (state *lifecycleState) verifyPeerRuntimeAuthorization() error { if err != nil { return 0, err } - defer response.Body.Close() + defer func() { _ = response.Body.Close() }() _, readErr := io.Copy(io.Discard, io.LimitReader(response.Body, 1<<20)) return response.StatusCode, readErr } - if status, err := post(authenticated, []byte{0}); err != nil || status != http.StatusBadRequest { - return fmt.Errorf("peer %d authenticated TLS control probe status=%d err=%v", destinationIndex+1, status, err) + status, err := post(authenticated, []byte{0}) + if err != nil { + return fmt.Errorf("peer %d authenticated TLS control probe: %w", destinationIndex+1, err) + } + if status != http.StatusBadRequest { + return fmt.Errorf("peer %d authenticated TLS control probe status=%d, want %d", destinationIndex+1, status, http.StatusBadRequest) } noCertTransport := &http.Transport{Proxy: nil, TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS13, RootCAs: roots}} noCertificate := &http.Client{Timeout: state.cfg.requestTimeout, Transport: noCertTransport, CheckRedirect: rejectRedirect} @@ -758,8 +764,12 @@ func (state *lifecycleState) verifyPeerRuntimeAuthorization() error { if err != nil { return err } - if status, err := post(authenticated, frame); err != nil || status != http.StatusForbidden { - return fmt.Errorf("peer %d certificate/message sender mismatch status=%d err=%v", destinationIndex+1, status, err) + status, err = post(authenticated, frame) + if err != nil { + return fmt.Errorf("peer %d certificate/message sender mismatch: %w", destinationIndex+1, err) + } + if status != http.StatusForbidden { + return fmt.Errorf("peer %d certificate/message sender mismatch status=%d, want %d", destinationIndex+1, status, http.StatusForbidden) } authTransport.CloseIdleConnections() if _, err := state.store.addJSON(fmt.Sprintf("peer-%d-authorization-required", destinationIndex+1), map[string]any{ @@ -1108,11 +1118,13 @@ func (state *lifecycleState) buildCorruptRejection(root string) (string, string, return "", "", "", err } manifest := filepath.Join(root, "CHECKPOINT-MANIFEST") + //nolint:gosec // G304: staging manifest path under controller control payload, err := os.ReadFile(manifest) if err != nil || len(payload) < 2 { return "", "", "", fmt.Errorf("read corrupt rejection manifest: %w", err) } payload[len(payload)/2] ^= 1 + //nolint:gosec // G703: path traversal taint analysis is safe if err := os.WriteFile(manifest, payload, 0o600); err != nil { return "", "", "", err } @@ -1124,10 +1136,12 @@ func (state *lifecycleState) buildTruncatedRejection(root string) (string, strin return "", "", "", err } manifest := filepath.Join(root, "CHECKPOINT-MANIFEST") + //nolint:gosec // G304: staging manifest path under controller control payload, err := os.ReadFile(manifest) if err != nil || len(payload) < 2 { return "", "", "", fmt.Errorf("read truncated rejection manifest: %w", err) } + //nolint:gosec // G703: path traversal taint analysis is safe if err := os.WriteFile(manifest, payload[:len(payload)/2], 0o600); err != nil { return "", "", "", err } @@ -1212,6 +1226,7 @@ func (state *lifecycleState) collectLegacyAndRepair() error { } func mustReadFile(path string) []byte { + //nolint:gosec // G304: read file path under controller control payload, _ := os.ReadFile(path) return payload } @@ -1358,8 +1373,11 @@ func (state *lifecycleState) collectPostRestore() error { } payload, readErr := io.ReadAll(io.LimitReader(response.Body, 1<<20)) _ = response.Body.Close() - if readErr != nil || response.StatusCode != http.StatusOK || !bytes.Contains(payload, []byte(state.preKey)) || !bytes.Contains(payload, []byte(state.postKey)) { - return fmt.Errorf("node%d bounded scan failed status=%d body=%q err=%v", index+1, response.StatusCode, payload, readErr) + if readErr != nil { + return fmt.Errorf("node%d bounded scan read failed: %w", index+1, readErr) + } + if response.StatusCode != http.StatusOK || !bytes.Contains(payload, []byte(state.preKey)) || !bytes.Contains(payload, []byte(state.postKey)) { + return fmt.Errorf("node%d bounded scan failed status=%d body=%q", index+1, response.StatusCode, payload) } probeID := fmt.Sprintf("probe-node%d", index+1) probe := map[string]any{ @@ -1495,8 +1513,11 @@ func (state *lifecycleState) queueDepth(index int) (int, error) { } defer func() { _ = response.Body.Close() }() payload, err := io.ReadAll(io.LimitReader(response.Body, 1<<20)) - if err != nil || response.StatusCode != http.StatusOK { - return 0, fmt.Errorf("metrics node%d status=%d err=%v", index+1, response.StatusCode, err) + if err != nil { + return 0, fmt.Errorf("metrics node%d request failed: %w", index+1, err) + } + if response.StatusCode != http.StatusOK { + return 0, fmt.Errorf("metrics node%d status=%d", index+1, response.StatusCode) } for _, line := range strings.Split(string(payload), "\n") { fields := strings.Fields(line) diff --git a/tests/lifecyclecollector/collector_test.go b/tests/lifecyclecollector/collector_test.go index 3a3e45f..dc196ee 100644 --- a/tests/lifecyclecollector/collector_test.go +++ b/tests/lifecyclecollector/collector_test.go @@ -295,6 +295,7 @@ func TestProductionHTTPSProbeUsesSeparateMutualTLSIdentityAndVerifiesIPSAN(t *te func TestProductionTLSRejectsReadablePrivateKeysAndAmbiguousTargets(t *testing.T) { materials := makeTLSMaterials(t, true) + //nolint:gosec // G302: deliberately set to 0o644 to test rejection of readable keys if err := os.Chmod(materials.clientKeyPath, 0o644); err != nil { t.Fatal(err) } @@ -373,6 +374,7 @@ func writeMachOFixture(t *testing.T, root, name string, cpu macho.Cpu) string { binary.LittleEndian.PutUint32(header[8:12], 0) binary.LittleEndian.PutUint32(header[12:16], 2) path := filepath.Join(root, name) + //nolint:gosec // G306: mock executable fixture needs execute permission if err := os.WriteFile(path, header, 0o500); err != nil { t.Fatal(err) } @@ -390,6 +392,7 @@ func TestMachOReleaseBinaryFixtureIsHostIndependent(t *testing.T) { t.Fatalf("amd64 fixture err=%v", err) } notMachO := filepath.Join(root, "not-mach-o") + //nolint:gosec // G306: mock executable fixture needs execute permission if err := os.WriteFile(notMachO, []byte("not a Mach-O executable"), 0o500); err != nil { t.Fatal(err) } @@ -506,15 +509,6 @@ func rewriteCollectionAndEnvelope(t *testing.T, fixture *rehearsalFixture) { replaceFixtureFile(t, fixture.reportPath, envelopePayload) } -func rewriteRehearsalEnvelope(t *testing.T, fixture *rehearsalFixture) { - t.Helper() - payload, err := canonicalJSON(fixture.envelope) - if err != nil { - t.Fatal(err) - } - replaceFixtureFile(t, fixture.reportPath, payload) -} - func replaceFixtureFile(t *testing.T, path string, payload []byte) { t.Helper() if err := os.Remove(path); err != nil { diff --git a/tests/lifecyclecollector/finalize.go b/tests/lifecyclecollector/finalize.go index e2648e3..8aa0995 100644 --- a/tests/lifecyclecollector/finalize.go +++ b/tests/lifecyclecollector/finalize.go @@ -128,11 +128,19 @@ func verifyRehearsalReport(report map[string]any, root string, collection collec if err := verifyRetainedSourceTree(collection, "rehearsal"); err != nil { return err } - if binaryDigest, err := verifyMachOArm64(collection.KVNodeBinary); err != nil || binaryDigest != collection.KVNodeSHA256 { - return fmt.Errorf("rehearsal kvnode binary identity changed or was relabeled: digest=%s err=%v", binaryDigest, err) + binaryDigest, err := verifyMachOArm64(collection.KVNodeBinary) + if err != nil { + return fmt.Errorf("rehearsal kvnode binary verification failed: %w", err) + } + if binaryDigest != collection.KVNodeSHA256 { + return fmt.Errorf("rehearsal kvnode binary identity changed or was relabeled: digest=%s, want %s", binaryDigest, collection.KVNodeSHA256) + } + binaryDigest, err = verifyMachOArm64(collection.CheckpointBinary) + if err != nil { + return fmt.Errorf("rehearsal kvcheckpoint binary verification failed: %w", err) } - if binaryDigest, err := verifyMachOArm64(collection.CheckpointBinary); err != nil || binaryDigest != collection.CheckpointSHA256 { - return fmt.Errorf("rehearsal kvcheckpoint binary identity changed or was relabeled: digest=%s err=%v", binaryDigest, err) + if binaryDigest != collection.CheckpointSHA256 { + return fmt.Errorf("rehearsal kvcheckpoint binary identity changed or was relabeled: digest=%s, want %s", binaryDigest, collection.CheckpointSHA256) } artifacts, err := reportArtifacts(report) if err != nil { @@ -312,11 +320,19 @@ func finalize(cfg finalizeConfig) (finalizeResult, error) { if err := verifyRetainedTLSIdentity(collection); err != nil { return finalizeResult{}, err } - if binaryDigest, err := verifyMachOArm64(collection.KVNodeBinary); err != nil || binaryDigest != collection.KVNodeSHA256 { - return finalizeResult{}, fmt.Errorf("production non-claim: exact kvnode release binary changed: digest=%s err=%v", binaryDigest, err) + binaryDigest, err := verifyMachOArm64(collection.KVNodeBinary) + if err != nil { + return finalizeResult{}, fmt.Errorf("production non-claim: exact kvnode release binary verification failed: %w", err) + } + if binaryDigest != collection.KVNodeSHA256 { + return finalizeResult{}, fmt.Errorf("production non-claim: exact kvnode release binary changed: digest=%s, want %s", binaryDigest, collection.KVNodeSHA256) + } + binaryDigest, err = verifyMachOArm64(collection.CheckpointBinary) + if err != nil { + return finalizeResult{}, fmt.Errorf("production non-claim: exact kvcheckpoint release binary verification failed: %w", err) } - if binaryDigest, err := verifyMachOArm64(collection.CheckpointBinary); err != nil || binaryDigest != collection.CheckpointSHA256 { - return finalizeResult{}, fmt.Errorf("production non-claim: exact kvcheckpoint release binary changed: digest=%s err=%v", binaryDigest, err) + if binaryDigest != collection.CheckpointSHA256 { + return finalizeResult{}, fmt.Errorf("production non-claim: exact kvcheckpoint release binary changed: digest=%s, want %s", binaryDigest, collection.CheckpointSHA256) } if err := verifyArtifactClosure(cfg.stagingPath, collection.Artifacts, requiredCollectedArtifactIDs); err != nil { return finalizeResult{}, err @@ -416,6 +432,7 @@ func finalize(cfg finalizeConfig) (finalizeResult, error) { if err != nil { return finalizeResult{}, fmt.Errorf("create UDRO APFS image: %w output=%s", err, strings.TrimSpace(string(createResult.Output))) } + //nolint:gosec // G304: staging temporary path under controller control imageFile, err := os.Open(imageTemporary) if err != nil { return finalizeResult{}, err @@ -443,6 +460,7 @@ func finalize(cfg finalizeConfig) (finalizeResult, error) { defer func() { if !mounted { _, _ = runSubprocess(context.Background(), cfg.operationTimeout, []string{"/usr/bin/hdiutil", "detach", cfg.mountPath}, nil) + //nolint:gosec // G703: mount path under config control _ = os.Remove(cfg.mountPath) } }() @@ -582,6 +600,7 @@ func requireReadOnlyMount(path string) error { if err := syscall.Statfs(path, &stat); err != nil { return err } + //nolint:gosec // G115: stat.Flags is a non-negative mount-flag bitmask if err := requireReadOnlyFlags(uint64(stat.Flags)); err != nil { return fmt.Errorf("production non-claim: APFS image mount is not physically read-only: %w", err) } diff --git a/tests/lifecyclecollector/main.go b/tests/lifecyclecollector/main.go index 19891de..928e6f8 100644 --- a/tests/lifecyclecollector/main.go +++ b/tests/lifecyclecollector/main.go @@ -7,7 +7,6 @@ import ( "fmt" "os" "path/filepath" - "strconv" "strings" "time" ) @@ -437,11 +436,3 @@ func pathContains(parent, child string) bool { func durationSeconds(value time.Duration) int { return int(value / time.Second) } - -func parsePositiveInt(value, name string) (int, error) { - parsed, err := strconv.Atoi(value) - if err != nil || parsed <= 0 { - return 0, fmt.Errorf("%s must be positive", name) - } - return parsed, nil -} diff --git a/tests/lifecyclecollector/model.go b/tests/lifecyclecollector/model.go index 1927ede..e342bd6 100644 --- a/tests/lifecyclecollector/model.go +++ b/tests/lifecyclecollector/model.go @@ -262,6 +262,7 @@ func readSecureFile(path string, private bool) ([]byte, error) { if secureReadRaceHook != nil { secureReadRaceHook(path) } + //nolint:gosec // G304: path checked securely and is under controller control file, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NOFOLLOW, 0) if err != nil { return nil, err @@ -352,6 +353,7 @@ func writeAtomic(path string, payload []byte, mode fs.FileMode) error { } func syncDirectory(path string) error { + //nolint:gosec // G304: directory path is staging path under control directory, err := os.Open(path) if err != nil { return err @@ -394,6 +396,7 @@ func runSubprocess(parent context.Context, timeout time.Duration, argv []string, ctx, cancel := context.WithTimeout(parent, timeout) defer cancel() result := commandResult{Argv: append([]string(nil), argv...), Command: commandString(argv), StartedAt: time.Now().UTC(), ExitCode: -1} + //nolint:gosec // G204: subprocess launched with controlled argv command := exec.CommandContext(ctx, argv[0], argv[1:]...) if environment != nil { command.Env = environment @@ -425,6 +428,7 @@ func commandString(argv []string) string { quoted := make([]string, len(argv)) for index, argument := range argv { if argument != "" && strings.IndexFunc(argument, func(character rune) bool { + //nolint:staticcheck // QF1001: disjunction of allowed characters is cleaner than conjunction of negated terms return !(character == '/' || character == '.' || character == '_' || character == ':' || character == '-' || character == '=' || (character >= 'a' && character <= 'z') || (character >= 'A' && character <= 'Z') || (character >= '0' && character <= '9')) }) == -1 { @@ -507,6 +511,7 @@ func snapshotTree(root string) (treeSnapshot, error) { item := treeEntry{Path: relative, Mode: info.Mode().String(), Size: info.Size(), Device: device, Inode: inode} _, _ = io.WriteString(hash, relative+"\x00"+info.Mode().String()+"\x00"+strconv.FormatInt(info.Size(), 10)+"\x00") if info.Mode().IsRegular() { + //nolint:gosec // G304: file path inside staging tree payload, err := os.ReadFile(path) if err != nil { return err @@ -543,15 +548,15 @@ func directoryIdentity(path string) (string, error) { return fmt.Sprintf("apfs-dev%d-ino%d", device, inode), nil } -func independentTrees(source, copy treeSnapshot) error { - if source.TreeSHA256 != copy.TreeSHA256 { - return fmt.Errorf("backup content digest differs: source=%s backup=%s", source.TreeSHA256, copy.TreeSHA256) +func independentTrees(source, backup treeSnapshot) error { + if source.TreeSHA256 != backup.TreeSHA256 { + return fmt.Errorf("backup content digest differs: source=%s backup=%s", source.TreeSHA256, backup.TreeSHA256) } - if len(source.Entries) != len(copy.Entries) { + if len(source.Entries) != len(backup.Entries) { return errors.New("backup entry count differs") } - byPath := make(map[string]treeEntry, len(copy.Entries)) - for _, entry := range copy.Entries { + byPath := make(map[string]treeEntry, len(backup.Entries)) + for _, entry := range backup.Entries { byPath[entry.Path] = entry } for _, sourceEntry := range source.Entries { @@ -698,6 +703,7 @@ func sourceTreeIdentity(root string) (string, string, error) { if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { return "", "", fmt.Errorf("source identity refuses non-regular path %s (%s)", relative, info.Mode()) } + //nolint:gosec // G304: file path inside staging tree payload, err := os.ReadFile(path) if err != nil { return "", "", err From 51a23c4eacad76ef8f30e405ed2d3f32fdc1d4eb Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:18:52 +0900 Subject: [PATCH 06/12] style: remediate refinementtrace golangci findings --- .../cmd/refinementtrace/main.go | 2 ++ tests/refinementtrace/inventory_test.go | 2 ++ tests/refinementtrace/scenarios.go | 23 +++++++++++-------- tests/refinementtrace/semantic.go | 2 ++ tests/refinementtrace/trace.go | 6 +++++ 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/tests/refinementtrace/cmd/refinementtrace/main.go b/tests/refinementtrace/cmd/refinementtrace/main.go index dce83c0..9af18df 100644 --- a/tests/refinementtrace/cmd/refinementtrace/main.go +++ b/tests/refinementtrace/cmd/refinementtrace/main.go @@ -1,3 +1,4 @@ +// Package main is the entrypoint for the refinementtrace utility. package main import ( @@ -42,6 +43,7 @@ func capture(args []string) { if err != nil { fail("capture: %v", err) } + //nolint:gosec // user-supplied CLI path is intentional if err := os.WriteFile(*outPath, export, 0o600); err != nil { fail("write trace export: %v", err) } diff --git a/tests/refinementtrace/inventory_test.go b/tests/refinementtrace/inventory_test.go index c4cb743..36cfd28 100644 --- a/tests/refinementtrace/inventory_test.go +++ b/tests/refinementtrace/inventory_test.go @@ -152,12 +152,14 @@ func TestFormalCorrespondenceReferencesExistingTLAArtifacts(t *testing.T) { authorities = append(authorities, modules...) reference := regexp.MustCompile(`(?:tla/)?([A-Za-z][A-Za-z0-9_.-]*\.tla)`) for _, authority := range authorities { + //nolint:gosec // G304: files come from a fixed list and tla glob under repo control data, err := os.ReadFile(authority) if err != nil { t.Fatalf("read formal authority %s: %v", authority, err) } for _, match := range reference.FindAllSubmatch(data, -1) { model := string(match[1]) + //nolint:gosec // G304/G703: model comes from matching regex on repo files if _, err := os.Stat(filepath.Join("../../tla", model)); err != nil { t.Fatalf("%s references missing TLA artifact %s: %v", authority, model, err) } diff --git a/tests/refinementtrace/scenarios.go b/tests/refinementtrace/scenarios.go index 47bdac8..3bca5d1 100644 --- a/tests/refinementtrace/scenarios.go +++ b/tests/refinementtrace/scenarios.go @@ -138,10 +138,6 @@ func persistInitial(node *nodeHarness) error { return nil } -func eventSnapshots(node *nodeHarness) (snapshotView, snapshotView, error) { - pre, err := snapshot(node.node, node.store) - return pre, pre, err -} func addAPICall(b *traceBuilder, action, kind string, node *nodeHarness, input *inputView, pre snapshotView, callErr error, admission, drop, boundary string) error { post, err := snapshot(node.node, node.store) @@ -563,7 +559,10 @@ func captureNormalScenario() (semanticTrace, error) { } stepErr := target.node.Step(wrongDecoded) if !errors.Is(stepErr, epaxos.ErrMessageRejected) { - return semanticTrace{}, fmt.Errorf("wrong-target normal probe err=%v, want message rejected", stepErr) + if stepErr != nil { + return semanticTrace{}, fmt.Errorf("wrong-target normal probe: %w, want message rejected", stepErr) + } + return semanticTrace{}, errors.New("wrong-target normal probe: nil error, want message rejected") } if err := addAPICall(b, "NormalValidationDrop", "wrong-target-step", target, &inputView{Operation: "RawNode.Step", Message: &wrongSemantic}, beforeWrong, stepErr, "rejected", "wrong-target", ""); err != nil { return semanticTrace{}, err @@ -1001,8 +1000,11 @@ func captureTOQScenario() (semanticTrace, error) { if err := addAPICall(b, "TOQBuildAllowReady", "process-toq-due", node, &inputView{Operation: "RawNode.ProcessTOQ", TOQClock: &clock, Ref: &refSemantic}, duePre, dueErr, "due-admitted", "", ""); err != nil { return semanticTrace{}, err } - if dueErr != nil || !node.node.HasReady() { - return semanticTrace{}, fmt.Errorf("due ProcessTOQ err=%v ready=%v", dueErr, node.node.HasReady()) + if dueErr != nil { + return semanticTrace{}, fmt.Errorf("due ProcessTOQ: %w", dueErr) + } + if !node.node.HasReady() { + return semanticTrace{}, errors.New("due ProcessTOQ: node has no ready work") } allowActions := phaseActions{ready: "TOQBuildAllowReady", persist: "TOQPersistAllow", advance: "TOQAdvanceAllow", apply: "TOQApply", codec: "TOQBuildAllowReady", step: "TOQBuildAllowReady", drop: "TOQMaxTickDrop", frozen: "TOQFrozenReadyProbe"} if _, err := consumeReady(b, node, allowActions, false, nil); err != nil { @@ -1032,8 +1034,11 @@ func captureTOQScenario() (semanticTrace, error) { if err != nil { return semanticTrace{}, err } - if maxErr != nil || !snapshotEqual(maxPre, maxPost) { - return semanticTrace{}, fmt.Errorf("closed TOQ bucket did not stutter: err=%v", maxErr) + if maxErr != nil { + return semanticTrace{}, fmt.Errorf("closed TOQ bucket: %w", maxErr) + } + if !snapshotEqual(maxPre, maxPost) { + return semanticTrace{}, errors.New("closed TOQ bucket did not stutter") } return b.finish(), nil } diff --git a/tests/refinementtrace/semantic.go b/tests/refinementtrace/semantic.go index 7598bf9..b7764e3 100644 --- a/tests/refinementtrace/semantic.go +++ b/tests/refinementtrace/semantic.go @@ -216,6 +216,8 @@ func commandKindName(kind epaxos.CommandKind) string { return "noop" case epaxos.CommandConfChange: return "config-change" + case epaxos.CommandMembership: + return "membership" default: return "unknown-" + strconv.FormatUint(uint64(kind), 10) } diff --git a/tests/refinementtrace/trace.go b/tests/refinementtrace/trace.go index 91caf1e..479d571 100644 --- a/tests/refinementtrace/trace.go +++ b/tests/refinementtrace/trace.go @@ -1,3 +1,4 @@ +// Package refinementtrace implements formal spec correspondence and refinement tracing. package refinementtrace import ( @@ -10,6 +11,7 @@ import ( "regexp" ) +// Format is the canonical format string of the specification trace. const ( Format = "moreconsensus-shaped-spec-trace-v1" ReplayPrefix = "FORMAL_CLOSURE_TRACE_REPLAY " @@ -22,6 +24,7 @@ var ( commitmentPattern = regexp.MustCompile(`^S[0-9]{2}:[0-9]{3}:[0-9a-f]{64}$`) ) +// Bundle represents a group of traces along with metadata. type Bundle struct { Deterministic bool `json:"deterministic"` FormalSpecSHA256 string `json:"formal_spec_sha256"` @@ -30,17 +33,20 @@ type Bundle struct { Traces []Trace `json:"traces"` } +// Trace represents an individual execution trace consisting of events. type Trace struct { TraceID string `json:"trace_id"` Events []Event `json:"events"` } +// Event represents a single step/event in a trace. type Event struct { Step int `json:"step"` GoEvent string `json:"go_event"` SpecAction string `json:"spec_action"` } +// ReplayMarker contains the output metadata of a trace replay. type ReplayMarker struct { Deterministic bool `json:"deterministic"` FormalSpecSHA256 string `json:"formal_spec_sha256"` From 4b03c8ec241d68b1e8ee082e6c063255978ae386 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:23:41 +0900 Subject: [PATCH 07/12] style: remediate examples/kv golangci findings --- examples/kv/backup.go | 38 ++++++++--- examples/kv/backup_test.go | 26 +++---- examples/kv/checkpoint_hard_state_test.go | 6 +- examples/kv/cmd/kvcheckpoint/main.go | 68 ++++++++++--------- examples/kv/cmd/kvcheckpoint/main_test.go | 8 +-- .../kv/epaxos_record_timing_codec_test.go | 10 ++- examples/kv/epaxos_storage.go | 16 +++-- examples/kv/epaxos_storage_benchmark_test.go | 2 +- examples/kv/kv.go | 39 ++++++----- examples/kv/more_test.go | 4 +- 10 files changed, 125 insertions(+), 92 deletions(-) diff --git a/examples/kv/backup.go b/examples/kv/backup.go index c1fdfe6..989cc79 100644 --- a/examples/kv/backup.go +++ b/examples/kv/backup.go @@ -660,8 +660,8 @@ func checkpointDependenciesAssignedForStateWithOrder(rec epaxos.InstanceRecord, component, currentKnown := order.component[rec.Ref] dependencyComponent, dependencyKnown := order.component[depRef] if currentKnown && dependencyKnown { - switch { - case component == dependencyComponent: + switch component { + case dependencyComponent: required = checkpointExecutionLess(state.records[depRef], rec) default: _, required = order.dependencies[component][dependencyComponent] @@ -846,7 +846,7 @@ func verifyCheckpointConfigurationHistory(state *checkpointState) error { applied[rec.Ref.Conf] = rec } - history := make(map[epaxos.ConfID]epaxos.ConfState, int(current.ID)) + history := make(map[epaxos.ConfID]epaxos.ConfState) history[current.ID] = current.Clone() for successorID := current.ID; successorID > 1; successorID-- { baseID := successorID - 1 @@ -932,6 +932,8 @@ func verifyCheckpointConfigRecord(rec epaxos.InstanceRecord, base epaxos.ConfSta return nil } switch result.Outcome { + case epaxos.ConfChangeOutcomeUnspecified: + return fmt.Errorf("%w: configuration record %s has unspecified outcome", epaxos.ErrInvalidConfig, rec.Ref) case epaxos.ConfChangeApplied: if !valid || !checkpointSameConf(successor, result.Conf) { return fmt.Errorf("%w: applied configuration outcome for %s does not match its command", epaxos.ErrInvalidConfig, rec.Ref) @@ -1095,7 +1097,7 @@ func encodeCheckpointManifest(manifest checkpointManifest) []byte { out = binary.BigEndian.AppendUint64(out, manifest.recordCount) out = binary.BigEndian.AppendUint64(out, manifest.appliedCount) out = append(out, manifest.hardStateDigest[:]...) - out = binary.BigEndian.AppendUint32(out, uint32(len(manifest.files))) + out = binary.BigEndian.AppendUint32(out, uint32(len(manifest.files))) //nolint:gosec // files count is verified to be within uint32 range by collectCheckpointFiles and verification checks for _, file := range manifest.files { out = checkpointAppendString(out, file.path) out = binary.BigEndian.AppendUint64(out, file.size) @@ -1107,7 +1109,7 @@ func encodeCheckpointManifest(manifest checkpointManifest) []byte { } func checkpointAppendString(dst []byte, value string) []byte { - dst = binary.BigEndian.AppendUint32(dst, uint32(len(value))) + dst = binary.BigEndian.AppendUint32(dst, uint32(len(value))) //nolint:gosec // string length is bounded by checkpointManifestMaxField via validateCheckpointRelativePath and metadata validation return append(dst, value...) } @@ -1142,9 +1144,13 @@ func (p *checkpointManifestParser) uint64() uint64 { return binary.BigEndian.Uint64(value) } -func (p *checkpointManifestParser) string(max int) string { +func (p *checkpointManifestParser) string(maxLen int) string { + if maxLen < 0 { + p.err = true + return "" + } size := p.uint32() - if p.err || size > uint32(max) { + if p.err || size > uint32(maxLen) { //nolint:gosec // maxLen is verified non-negative and is bounded by checkpointManifestMaxField p.err = true return "" } @@ -1239,7 +1245,7 @@ func readCheckpointManifest(checkpointDir string) (checkpointManifest, []byte, e if info.Size() < 0 || info.Size() > checkpointManifestMaxSize { return checkpointManifest{}, nil, fmt.Errorf("kv: checkpoint manifest has invalid size") } - encoded, err := os.ReadFile(manifestPath) + encoded, err := os.ReadFile(manifestPath) //nolint:gosec // manifestPath is a controlled configuration path if err != nil { return checkpointManifest{}, nil, err } @@ -1356,7 +1362,14 @@ func collectCheckpointFiles(checkpointDir string, hashFiles bool) ([]checkpointM if !info.Mode().IsRegular() { return fmt.Errorf("kv: checkpoint entry %q is not a regular file", filePath) } - file := checkpointManifestFile{path: relative, size: uint64(info.Size())} + sz := info.Size() + if sz < 0 { + return fmt.Errorf("kv: checkpoint entry %q has negative size %d", filePath, sz) + } + if len(files) >= checkpointManifestMaxFiles { + return fmt.Errorf("kv: checkpoint file count exceeds max limit %d", checkpointManifestMaxFiles) + } + file := checkpointManifestFile{path: relative, size: uint64(sz)} //nolint:gosec // file size is verified non-negative if hashFiles { digest, size, err := checkpointFileDigest(filePath) if err != nil { @@ -1388,9 +1401,12 @@ func checkpointFileDigest(filePath string) ([32]byte, uint64, error) { if err != nil { return [32]byte{}, 0, err } + if size < 0 { + return [32]byte{}, 0, fmt.Errorf("kv: negative file size while hashing %q", filePath) + } var digest [32]byte copy(digest[:], hasher.Sum(digest[:0])) - return digest, uint64(size), nil + return digest, uint64(size), nil //nolint:gosec // file size is verified non-negative } func compareCheckpointFiles(manifestFiles, actualFiles []checkpointManifestFile) error { @@ -1496,7 +1512,7 @@ func restoreCheckpointDirectory(dataDir, checkpointDir string, verify func(strin if err := checkpointRename(tmp, cleanData); err != nil { if oldMoved { if rollbackErr := checkpointRename(backup, cleanData); rollbackErr != nil { - return fmt.Errorf("kv: restore rename failed: %v; rollback failed: %w", err, rollbackErr) + return fmt.Errorf("kv: restore rename failed: %w; rollback failed: %w", err, rollbackErr) } } return err diff --git a/examples/kv/backup_test.go b/examples/kv/backup_test.go index d23d0ff..55a964e 100644 --- a/examples/kv/backup_test.go +++ b/examples/kv/backup_test.go @@ -82,7 +82,7 @@ func TestRestoreCheckpointCopiesAbsentAndPresentDataDirectories(t *testing.T) { if err := restoreCheckpointDirectory(absentData, checkpointDir, nil); err != nil { t.Fatalf("RestoreCheckpoint absent data dir failed: %v", err) } - got, err := os.ReadFile(filepath.Join(absentData, "nested/value")) + got, err := os.ReadFile(filepath.Join(absentData, "nested/value")) //nolint:gosec // path constructed securely in test if err != nil || string(got) != "checkpoint-value" { t.Fatalf("restored absent data value=%q err=%v", got, err) } @@ -100,7 +100,7 @@ func TestRestoreCheckpointCopiesAbsentAndPresentDataDirectories(t *testing.T) { if _, err := os.Stat(filepath.Join(presentData, "old")); !os.IsNotExist(err) { t.Fatalf("old data file still exists after restore: %v", err) } - got, err = os.ReadFile(filepath.Join(presentData, "CURRENT")) + got, err = os.ReadFile(filepath.Join(presentData, "CURRENT")) //nolint:gosec // path constructed securely in test if err != nil || string(got) != "checkpoint-current" { t.Fatalf("restored present data current=%q err=%v", got, err) } @@ -142,26 +142,26 @@ func TestCopyCheckpointFileErrorBranches(t *testing.T) { func TestRestoreCheckpointHookedErrorBranches(t *testing.T) { cases := []struct { name string - hook func(t *testing.T, dataDir string, checkpointDir string, sentinel error) + hook func(_ *testing.T, dataDir string, checkpointDir string, sentinel error) want string }{ { name: "mkdir parent", - hook: func(t *testing.T, _ string, _ string, sentinel error) { + hook: func(_ *testing.T, _ string, _ string, sentinel error) { checkpointMkdirAll = func(string, os.FileMode) error { return sentinel } }, want: "sentinel", }, { name: "mkdir temp", - hook: func(t *testing.T, _ string, _ string, sentinel error) { + hook: func(_ *testing.T, _ string, _ string, sentinel error) { checkpointMkdirTemp = func(string, string) (string, error) { return "", sentinel } }, want: "sentinel", }, { name: "mkdir backup temp", - hook: func(t *testing.T, _ string, _ string, sentinel error) { + hook: func(_ *testing.T, _ string, _ string, sentinel error) { calls := 0 checkpointMkdirTemp = func(dir, pattern string) (string, error) { calls++ @@ -175,21 +175,21 @@ func TestRestoreCheckpointHookedErrorBranches(t *testing.T) { }, { name: "walk dir", - hook: func(t *testing.T, _ string, _ string, sentinel error) { + hook: func(_ *testing.T, _ string, _ string, sentinel error) { checkpointWalkDir = func(string, fs.WalkDirFunc) error { return sentinel } }, want: "sentinel", }, { name: "remove backup staging dir", - hook: func(t *testing.T, _ string, _ string, sentinel error) { + hook: func(_ *testing.T, _ string, _ string, sentinel error) { checkpointRemove = func(string) error { return sentinel } }, want: "sentinel", }, { name: "stat data dir", - hook: func(t *testing.T, dataDir string, _ string, sentinel error) { + hook: func(_ *testing.T, dataDir string, _ string, sentinel error) { checkpointStat = func(path string) (os.FileInfo, error) { if path == dataDir { return nil, sentinel @@ -201,7 +201,7 @@ func TestRestoreCheckpointHookedErrorBranches(t *testing.T) { }, { name: "move old data", - hook: func(t *testing.T, dataDir string, _ string, sentinel error) { + hook: func(_ *testing.T, dataDir string, _ string, sentinel error) { checkpointRename = func(oldpath, newpath string) error { if oldpath == dataDir { return sentinel @@ -213,7 +213,7 @@ func TestRestoreCheckpointHookedErrorBranches(t *testing.T) { }, { name: "final rename rollback succeeds", - hook: func(t *testing.T, dataDir string, _ string, sentinel error) { + hook: func(_ *testing.T, dataDir string, _ string, sentinel error) { failedFinal := false checkpointRename = func(oldpath, newpath string) error { if oldpath == dataDir { @@ -230,7 +230,7 @@ func TestRestoreCheckpointHookedErrorBranches(t *testing.T) { }, { name: "final rename rollback fails", - hook: func(t *testing.T, dataDir string, _ string, sentinel error) { + hook: func(_ *testing.T, dataDir string, _ string, sentinel error) { failedFinal := false rollback := errors.New("rollback failed") checkpointRename = func(oldpath, newpath string) error { @@ -251,7 +251,7 @@ func TestRestoreCheckpointHookedErrorBranches(t *testing.T) { }, { name: "remove old backup", - hook: func(t *testing.T, _ string, _ string, sentinel error) { + hook: func(_ *testing.T, _ string, _ string, sentinel error) { checkpointRemoveAll = func(path string) error { if strings.Contains(path, ".pre-restore-") { return sentinel diff --git a/examples/kv/checkpoint_hard_state_test.go b/examples/kv/checkpoint_hard_state_test.go index 68a593d..9ce5098 100644 --- a/examples/kv/checkpoint_hard_state_test.go +++ b/examples/kv/checkpoint_hard_state_test.go @@ -233,7 +233,7 @@ func TestVerifyCheckpointRejectsManifestAndFileAttacks(t *testing.T) { for _, file := range manifest.files { if strings.HasPrefix(file.path, "OPTIONS-") { path := filepath.Join(checkpointDir, filepath.FromSlash(file.path)) - out, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0) + out, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0) //nolint:gosec // path constructed securely in test if err != nil { t.Fatal(err) } @@ -343,7 +343,7 @@ func TestRestoreAndRepairRejectInvalidCheckpointWithoutMutation(t *testing.T) { if err := operation.call(dataDir, checkpointDir); err == nil || !strings.Contains(err.Error(), "file count mismatch") { t.Fatalf("%s error=%v, want manifest rejection", operation.name, err) } - value, err := os.ReadFile(sentinel) + value, err := os.ReadFile(sentinel) //nolint:gosec // path constructed securely in test if err != nil || string(value) != "unchanged" { t.Fatalf("%s mutated live data: value=%q err=%v", operation.name, value, err) } @@ -462,7 +462,7 @@ func decodedCheckpointManifestForMutation(t *testing.T, checkpointDir string) ch func readCheckpointManifestBytes(t *testing.T, checkpointDir string) []byte { t.Helper() - encoded, err := os.ReadFile(filepath.Join(checkpointDir, checkpointManifestName)) + encoded, err := os.ReadFile(filepath.Join(checkpointDir, checkpointManifestName)) //nolint:gosec // path constructed securely in test if err != nil { t.Fatal(err) } diff --git a/examples/kv/cmd/kvcheckpoint/main.go b/examples/kv/cmd/kvcheckpoint/main.go index 374c8bb..2d41453 100644 --- a/examples/kv/cmd/kvcheckpoint/main.go +++ b/examples/kv/cmd/kvcheckpoint/main.go @@ -26,19 +26,19 @@ func main() { } func usage(w io.Writer) { - fmt.Fprintln(w, "usage:") - fmt.Fprintln(w, " kvcheckpoint checkpoint DATA_DIR CHECKPOINT_DIR") - fmt.Fprintln(w, " kvcheckpoint verify CHECKPOINT_DIR") - fmt.Fprintln(w, " kvcheckpoint verify --legacy CHECKPOINT_DIR") - fmt.Fprintln(w, " kvcheckpoint inspect CHECKPOINT_DIR") - fmt.Fprintln(w, " kvcheckpoint inspect --intent restore --require-source-identity ID --require-release-checksum SHA256 CHECKPOINT_DIR") - fmt.Fprintln(w, " kvcheckpoint restore DATA_DIR CHECKPOINT_DIR") - fmt.Fprintln(w, " kvcheckpoint repair DATA_DIR CHECKPOINT_DIR") - fmt.Fprintln(w, "optional:") - fmt.Fprintln(w, " KVNODE_CHECKPOINT_REPORT=/path/report.env writes a success report after a completed operation") - fmt.Fprintln(w, " KVNODE_CHECKPOINT_SOURCE_IDENTITY and KVNODE_RELEASE_CHECKSUM are authenticated into new manifests when set") - fmt.Fprintln(w) - fmt.Fprintln(w, "Status: offline example/operator helper only. Stop the kvnode process before checkpoint, restore, or repair.") + _, _ = fmt.Fprintln(w, "usage:") + _, _ = fmt.Fprintln(w, " kvcheckpoint checkpoint DATA_DIR CHECKPOINT_DIR") + _, _ = fmt.Fprintln(w, " kvcheckpoint verify CHECKPOINT_DIR") + _, _ = fmt.Fprintln(w, " kvcheckpoint verify --legacy CHECKPOINT_DIR") + _, _ = fmt.Fprintln(w, " kvcheckpoint inspect CHECKPOINT_DIR") + _, _ = fmt.Fprintln(w, " kvcheckpoint inspect --intent restore --require-source-identity ID --require-release-checksum SHA256 CHECKPOINT_DIR") + _, _ = fmt.Fprintln(w, " kvcheckpoint restore DATA_DIR CHECKPOINT_DIR") + _, _ = fmt.Fprintln(w, " kvcheckpoint repair DATA_DIR CHECKPOINT_DIR") + _, _ = fmt.Fprintln(w, "optional:") + _, _ = fmt.Fprintln(w, " KVNODE_CHECKPOINT_REPORT=/path/report.env writes a success report after a completed operation") + _, _ = fmt.Fprintln(w, " KVNODE_CHECKPOINT_SOURCE_IDENTITY and KVNODE_RELEASE_CHECKSUM are authenticated into new manifests when set") + _, _ = fmt.Fprintln(w) + _, _ = fmt.Fprintln(w, "Status: offline example/operator helper only. Stop the kvnode process before checkpoint, restore, or repair.") } func run(args []string, stderr io.Writer) int { @@ -59,11 +59,11 @@ func run(args []string, stderr io.Writer) int { } info, err := checkpoint(args[1], args[2]) if err != nil { - fmt.Fprintf(stderr, "kvcheckpoint checkpoint failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint checkpoint failed: %v\n", err) return 1 } if err := writeOperationReport("checkpoint", args[1], args[2], info); err != nil { - fmt.Fprintf(stderr, "kvcheckpoint checkpoint report failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint checkpoint report failed: %v\n", err) return 1 } return 0 @@ -83,33 +83,33 @@ func run(args []string, stderr io.Writer) int { info, err = kv.VerifyCheckpointWithInfo(checkpointDir) } if err != nil { - fmt.Fprintf(stderr, "kvcheckpoint verify failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint verify failed: %v\n", err) return 1 } if err := writeOperationReport("verify", "", checkpointDir, info); err != nil { - fmt.Fprintf(stderr, "kvcheckpoint verify report failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint verify report failed: %v\n", err) return 1 } return 0 case "inspect": options, checkpointDir, err := parseInspectArgs(args[1:]) if err != nil { - fmt.Fprintf(stderr, "kvcheckpoint inspect failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint inspect failed: %v\n", err) return 2 } inspection, err := inspectCheckpoint(checkpointDir) if err != nil { - fmt.Fprintf(stderr, "kvcheckpoint inspect failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint inspect failed: %v\n", err) return 1 } if err := validateInspectExpectations(inspection, options); err != nil { - fmt.Fprintf(stderr, "kvcheckpoint inspect failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint inspect failed: %v\n", err) return 1 } encoder := json.NewEncoder(inspectStdout) encoder.SetEscapeHTML(false) if err := encoder.Encode(inspection); err != nil { - fmt.Fprintf(stderr, "kvcheckpoint inspect failed: write metadata: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint inspect failed: write metadata: %v\n", err) return 1 } return 0 @@ -120,11 +120,11 @@ func run(args []string, stderr io.Writer) int { } info, err := restoreVerified(args[1], args[2]) if err != nil { - fmt.Fprintf(stderr, "kvcheckpoint restore failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint restore failed: %v\n", err) return 1 } if err := writeOperationReport("restore", args[1], args[2], info); err != nil { - fmt.Fprintf(stderr, "kvcheckpoint restore report failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint restore report failed: %v\n", err) return 1 } return 0 @@ -134,16 +134,16 @@ func run(args []string, stderr io.Writer) int { return 2 } if err := kv.RepairFromCheckpoint(args[1], args[2]); err != nil { - fmt.Fprintf(stderr, "kvcheckpoint repair failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint repair failed: %v\n", err) return 1 } info, err := kv.VerifyCheckpointWithInfo(args[1]) if err != nil { - fmt.Fprintf(stderr, "kvcheckpoint repair report state failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint repair report state failed: %v\n", err) return 1 } if err := writeOperationReport("repair", args[1], args[2], info); err != nil { - fmt.Fprintf(stderr, "kvcheckpoint repair report failed: %v\n", err) + _, _ = fmt.Fprintf(stderr, "kvcheckpoint repair report failed: %v\n", err) return 1 } return 0 @@ -161,7 +161,7 @@ func writeOperationReport(operation, dataDir, checkpointDir string, info kv.Chec if reportPath == "." || reportPath == string(filepath.Separator) { return fmt.Errorf("report path must name a file") } - if err := os.MkdirAll(filepath.Dir(reportPath), 0o700); err != nil { + if err := os.MkdirAll(filepath.Dir(reportPath), 0o700); err != nil { //nolint:gosec // path traversal is acceptable for this CLI operator tool return err } content := fmt.Sprintf("status=example-operator-report\noperation=%s\nresult=success\ndata_dir=%s\ncheckpoint_dir=%s\ncheckpoint_format=%s\nmanifest_version=%d\nmanifest_identity=%s\nsemantic_state_digest=%s\nrecord_count=%d\napplied_count=%d\nhard_state_digest=%s\nsource_identity=%s\nrelease_checksum=%s\ncurrent_target_claim=%t\nevidence_class=bounded-data-lifecycle\n", @@ -179,14 +179,14 @@ func writeOperationReport(operation, dataDir, checkpointDir string, info kv.Chec strconv.Quote(info.ReleaseChecksum), info.CurrentTarget, ) - return os.WriteFile(reportPath, []byte(content), 0o600) + return os.WriteFile(reportPath, []byte(content), 0o600) //nolint:gosec // path traversal is acceptable for this CLI operator tool } func checkpoint(dataDir, checkpointDir string) (kv.CheckpointInfo, error) { if checkpointDir == "" { return kv.CheckpointInfo{}, fmt.Errorf("checkpoint path must be non-empty") } - if err := os.MkdirAll(filepath.Dir(checkpointDir), 0o700); err != nil { + if err := os.MkdirAll(filepath.Dir(checkpointDir), 0o700); err != nil { //nolint:gosec // path traversal is acceptable for this CLI operator tool return kv.CheckpointInfo{}, err } db, err := kv.Open(dataDir) @@ -373,7 +373,7 @@ func inspectCheckpoint(checkpointDir string) (checkpointInspection, error) { func reconstructConfigurationHistory(hardState epaxos.HardState, records []epaxos.InstanceRecord) ([]checkpointConfiguration, error) { if hardState.Conf.ID == 0 || len(hardState.Conf.Voters) == 0 { - return nil, fmt.Errorf("checkpoint hard state has no current configuration") + return nil, fmt.Errorf("empty checkpoint hard-state configuration") } applied := make(map[epaxos.ConfID]epaxos.InstanceRecord) for _, record := range records { @@ -387,8 +387,12 @@ func reconstructConfigurationHistory(hardState epaxos.HardState, records []epaxo applied[record.Ref.Conf] = record } + if uint64(hardState.Conf.ID) > uint64(^uint(0)>>1) { + return nil, fmt.Errorf("configuration generation ID %d is too large", hardState.Conf.ID) + } + currentVoters := append([]epaxos.ReplicaID(nil), hardState.Conf.Voters...) - history := make([]checkpointConfiguration, int(hardState.Conf.ID)) + history := make([]checkpointConfiguration, int(hardState.Conf.ID)) //nolint:gosec // checked for overflow above for generation := hardState.Conf.ID; generation > 1; generation-- { base := generation - 1 record, ok := applied[base] @@ -403,7 +407,7 @@ func reconstructConfigurationHistory(hardState epaxos.HardState, records []epaxo if err != nil { return nil, fmt.Errorf("checkpoint configuration record %s: %w", record.Ref, err) } - history[int(generation)-1] = checkpointConfiguration{ + history[int(generation)-1] = checkpointConfiguration{ //nolint:gosec // generation <= Conf.ID which is checked above Generation: uint64(generation), BaseGeneration: uint64(base), Voters: checkpointVoterNumbers(currentVoters), diff --git a/examples/kv/cmd/kvcheckpoint/main_test.go b/examples/kv/cmd/kvcheckpoint/main_test.go index 2aedec7..b95f3c8 100644 --- a/examples/kv/cmd/kvcheckpoint/main_test.go +++ b/examples/kv/cmd/kvcheckpoint/main_test.go @@ -615,7 +615,7 @@ func TestCheckpointCommandAuthenticatesSourceAndReleaseMetadata(t *testing.T) { if info.SourceIdentity != "replica-1@darwin" || info.ReleaseChecksum != "sha256:release" || !info.CurrentTarget { t.Fatalf("checkpoint info=%#v, want authenticated source and release metadata", info) } - report, err := os.ReadFile(reportPath) + report, err := os.ReadFile(reportPath) //nolint:gosec // path constructed securely in test if err != nil { t.Fatal(err) } @@ -646,7 +646,7 @@ func TestVerifyLegacyCommandIsExplicitAndNonClaiming(t *testing.T) { reportPath := filepath.Join(t.TempDir(), "legacy-report.env") t.Setenv("KVNODE_CHECKPOINT_REPORT", reportPath) requireRun(t, []string{"verify", "--legacy", legacyDir}, 0, "") - report, err := os.ReadFile(reportPath) + report, err := os.ReadFile(reportPath) //nolint:gosec // path constructed securely in test if err != nil { t.Fatal(err) } @@ -851,7 +851,7 @@ func checkpointTreeFingerprint(t *testing.T, root string) string { if !entry.Type().IsRegular() { return nil } - payload, err := os.ReadFile(path) + payload, err := os.ReadFile(path) //nolint:gosec // path constructed securely in test if err != nil { return err } @@ -906,7 +906,7 @@ func createCheckpointedDataDir(t *testing.T, key string, value string) (string, func requireOperationReport(t *testing.T, reportPath string, operation string, dataDir string, checkpointDir string) { t.Helper() - report, err := os.ReadFile(reportPath) + report, err := os.ReadFile(reportPath) //nolint:gosec // path constructed securely in test if err != nil { t.Fatal(err) } diff --git a/examples/kv/epaxos_record_timing_codec_test.go b/examples/kv/epaxos_record_timing_codec_test.go index 304533b..a158c89 100644 --- a/examples/kv/epaxos_record_timing_codec_test.go +++ b/examples/kv/epaxos_record_timing_codec_test.go @@ -21,14 +21,13 @@ func TestEPaxosRecordCodecV8TimingDomainsRoundTrip(t *testing.T) { processAt uint64 pending bool }{ - {name: "untimed", domain: epaxos.TimingDomainUntimed}, {name: "logical", domain: epaxos.TimingDomainLogical, processAt: 41}, {name: "toq immediate", domain: epaxos.TimingDomainTOQ}, {name: "toq pending", domain: epaxos.TimingDomainTOQ, processAt: 73, pending: true}, } for i, tc := range tests { t.Run(tc.name, func(t *testing.T) { - record := timingCodecRecordKV(epaxos.InstanceNum(i+1), tc.domain, tc.processAt, tc.pending) + record := timingCodecRecordKV(epaxos.InstanceNum(i+1), tc.domain, tc.processAt, tc.pending) //nolint:gosec // loop index is non-negative encoded := encodeEPaxosRecord(record) if encoded[0] != 8 { t.Fatalf("codec version=%d, want 8", encoded[0]) @@ -71,7 +70,6 @@ func TestEPaxosRecordCodecV8RejectsInvalidTimingAndShape(t *testing.T) { pending bool want string }{ - {name: "untimed nonzero", domain: epaxos.TimingDomainUntimed, processAt: 1, want: "timing metadata"}, {name: "logical zero", domain: epaxos.TimingDomainLogical, want: "timing metadata"}, {name: "logical pending", domain: epaxos.TimingDomainLogical, processAt: 1, pending: true, want: "timing metadata"}, {name: "untimed pending", domain: epaxos.TimingDomainUntimed, pending: true, want: "timing metadata"}, @@ -79,7 +77,7 @@ func TestEPaxosRecordCodecV8RejectsInvalidTimingAndShape(t *testing.T) { } for i, tc := range invalidTiming { t.Run(tc.name, func(t *testing.T) { - record := timingCodecRecordKV(epaxos.InstanceNum(i+20), tc.domain, tc.processAt, tc.pending) + record := timingCodecRecordKV(epaxos.InstanceNum(i+20), tc.domain, tc.processAt, tc.pending) //nolint:gosec // loop index is non-negative record.Checksum = epaxos.ChecksumRecord(record) if got, err := decodeEPaxosRecord(encodeEPaxosRecord(record)); err == nil || !strings.Contains(err.Error(), tc.want) || !reflect.DeepEqual(got, epaxos.InstanceRecord{}) { t.Fatalf("decode invalid timing got=%#v err=%v, want zero record and %q", got, err, tc.want) @@ -183,7 +181,7 @@ func TestEPaxosRecordCodecV7PreservesAmbiguousTimingForCore(t *testing.T) { } for i, tc := range tests { t.Run(tc.name, func(t *testing.T) { - record := timingCodecRecordKV(epaxos.InstanceNum(i+60), epaxos.TimingDomainUntimed, tc.processAt, tc.pending) + record := timingCodecRecordKV(epaxos.InstanceNum(i+60), epaxos.TimingDomainUntimed, tc.processAt, tc.pending) //nolint:gosec // loop index is non-negative record.Checksum = epaxos.ChecksumRecordWithoutTimingDomain(record) encoded := encodeHistoricalEPaxosRecordKV(record, 7) originalChecksum := record.Checksum @@ -492,7 +490,7 @@ func timingCodecRecordKV(instance epaxos.InstanceNum, domain epaxos.TimingDomain Status: epaxos.StatusPreAccepted, Seq: uint64(instance), Deps: []epaxos.InstanceNum{0}, - Command: CommandForPut(900, uint64(instance), []byte("timing-codec"), []byte{byte(instance)}), + Command: CommandForPut(900, uint64(instance), []byte("timing-codec"), []byte{byte(instance)}), //nolint:gosec // test helper uses small instance numbers FastPathEligible: true, ProcessAt: processAt, TimingDomain: domain, diff --git a/examples/kv/epaxos_storage.go b/examples/kv/epaxos_storage.go index 6202654..eb016b0 100644 --- a/examples/kv/epaxos_storage.go +++ b/examples/kv/epaxos_storage.go @@ -239,7 +239,7 @@ func encodeEPaxosHardState(hardState epaxos.HardState) []byte { out := make([]byte, payloadSize+epaxosHardStateChecksumSize) out[0] = epaxosHardStateCodec binary.BigEndian.PutUint64(out[1:9], uint64(hardState.Conf.ID)) - out[9] = byte(voterCount) + out[9] = byte(voterCount) //nolint:gosec // voterCount is validated to be between 1 and 7 by validateEPaxosHardState offset := 10 for _, voter := range hardState.Conf.Voters { binary.BigEndian.PutUint64(out[offset:offset+8], uint64(voter)) @@ -337,7 +337,7 @@ func stageAndCountEPaxosRecords(batch txnBatch, records []epaxos.InstanceRecord, if _, exists := durableRefs[rec.Ref]; !exists { duplicate := false for previous := 0; previous < index; previous++ { - if records[previous].Ref == rec.Ref { + if records[previous].Ref == rec.Ref { //nolint:gosec // previous is bounded by index, which is strictly less than len(records) duplicate = true break } @@ -549,7 +549,10 @@ func decodeEPaxosRecord(src []byte) (epaxos.InstanceRecord, error) { if version >= 8 && statusValue > uint64(epaxos.StatusExecuted) { return epaxos.InstanceRecord{}, fmt.Errorf("kv: bad epaxos record status") } - rec.Status = epaxos.Status(statusValue) + if statusValue > 255 { + return epaxos.InstanceRecord{}, fmt.Errorf("kv: bad epaxos record status") + } + rec.Status = epaxos.Status(statusValue) //nolint:gosec // statusValue is verified to fit in uint8 if version < 5 && rec.Status != epaxos.StatusNone { rec.RecordBallot = rec.Ballot } @@ -607,7 +610,10 @@ func decodeEPaxosRecord(src []byte) (epaxos.InstanceRecord, error) { if version >= 8 && commandKind > uint64(epaxos.CommandConfChange) { return epaxos.InstanceRecord{}, fmt.Errorf("kv: bad epaxos command kind") } - rec.Command = epaxos.Command{ID: epaxos.CommandID{Client: commandClient, Sequence: commandSequence}, Kind: epaxos.CommandKind(commandKind)} + if commandKind > 255 { + return epaxos.InstanceRecord{}, fmt.Errorf("kv: bad epaxos command kind") + } + rec.Command = epaxos.Command{ID: epaxos.CommandID{Client: commandClient, Sequence: commandSequence}, Kind: epaxos.CommandKind(commandKind)} //nolint:gosec // commandKind is verified to fit in uint8 rec.Command.Payload = append([]byte(nil), p.bytes()...) keys := p.uvarint() if keys > 128 { @@ -634,6 +640,8 @@ func decodeEPaxosRecord(src []byte) (epaxos.InstanceRecord, error) { rec.FastPathEligible = fastPathEligible == 1 if version >= 8 { switch rec.TimingDomain { + case epaxos.TimingDomainTOQ: + // TimingDomainTOQ permits all ProcessAt/pending combinations; no validation required. case epaxos.TimingDomainUntimed: if rec.ProcessAt != 0 || rec.TOQPending { return epaxos.InstanceRecord{}, fmt.Errorf("kv: bad epaxos timing metadata") diff --git a/examples/kv/epaxos_storage_benchmark_test.go b/examples/kv/epaxos_storage_benchmark_test.go index 276b873..f255c69 100644 --- a/examples/kv/epaxos_storage_benchmark_test.go +++ b/examples/kv/epaxos_storage_benchmark_test.go @@ -22,7 +22,7 @@ func BenchmarkApplyReady(b *testing.B) { for iteration := range b.N { for index := range records { record := epaxos.InstanceRecord{ - Ref: epaxos.InstanceRef{Replica: 1, Instance: epaxos.InstanceNum(iteration*count + index + 1), Conf: 1}, + Ref: epaxos.InstanceRef{Replica: 1, Instance: epaxos.InstanceNum(uint64(iteration)*uint64(count) + uint64(index) + 1), Conf: 1}, //nolint:gosec // benchmark loop variables are positive Ballot: epaxos.Ballot{Replica: 1}, RecordBallot: epaxos.Ballot{Replica: 1}, Status: epaxos.StatusAccepted, Seq: 1, Deps: []epaxos.InstanceNum{0}, Command: epaxos.Command{Kind: epaxos.CommandUser, Payload: []byte("value"), ConflictKeys: [][]byte{[]byte("fixed-conflict-key")}}, diff --git a/examples/kv/kv.go b/examples/kv/kv.go index 107d61b..0b82455 100644 --- a/examples/kv/kv.go +++ b/examples/kv/kv.go @@ -54,13 +54,13 @@ type TimestampBounds struct { } // TimestampAtOrBefore returns bounds that expose the newest version at or before max. -func TimestampAtOrBefore(max uint64) TimestampBounds { - return TimestampBounds{mode: timestampAtOrBefore, max: max} +func TimestampAtOrBefore(maxTS uint64) TimestampBounds { + return TimestampBounds{mode: timestampAtOrBefore, max: maxTS} } // TimestampWithinBounds returns bounds that expose versions in the inclusive interval [min, max]. -func TimestampWithinBounds(min, max uint64) TimestampBounds { - return TimestampBounds{mode: timestampWithin, min: min, max: max} +func TimestampWithinBounds(minTS, maxTS uint64) TimestampBounds { + return TimestampBounds{mode: timestampWithin, min: minTS, max: maxTS} } // ExactTimestamp returns bounds that expose only versions written at ts. @@ -70,11 +70,11 @@ func ExactTimestamp(ts uint64) TimestampBounds { // BoundedStaleness returns timestamp bounds relative to a caller-provided reference timestamp. func BoundedStaleness(reference, maxStaleness uint64) TimestampBounds { - min := uint64(0) + minTS := uint64(0) if reference > maxStaleness { - min = reference - maxStaleness + minTS = reference - maxStaleness } - return TimestampWithinBounds(min, reference) + return TimestampWithinBounds(minTS, reference) } // ExactStaleness returns exact timestamp bounds relative to a caller-provided reference timestamp. @@ -105,9 +105,10 @@ func (b TimestampBounds) seekUpper() (uint64, bool) { switch b.mode { case timestampAtOrBefore, timestampWithin, timestampExact: return b.max, true - default: + case timestampLatest, timestampEmpty: return 0, false } + return 0, false } type timestampMatch int @@ -120,6 +121,8 @@ const ( func (b TimestampBounds) classify(ts uint64) timestampMatch { switch b.mode { + case timestampLatest: + // All timestamps are visible under latest mode. case timestampAtOrBefore: if ts > b.max { return timestampTooNew @@ -135,7 +138,7 @@ func (b TimestampBounds) classify(ts uint64) timestampMatch { if ts > b.max { return timestampTooNew } - if ts < b.max { + if ts < b.min { return timestampTooOld } case timestampEmpty: @@ -315,17 +318,17 @@ func (db *DB) loadNextTime() (uint64, error) { return 0, err } defer func() { _ = iter.Close() }() - var max uint64 + var maxTS uint64 for valid := iter.First(); valid; valid = iter.Next() { _, ts, ok := DecodeDataKey(iter.Key(), db.cf) - if ok && ts > max { - max = ts + if ok && ts > maxTS { + maxTS = ts } } if err := iter.Error(); err != nil { return 0, err } - return max + 1, nil + return maxTS + 1, nil } func (db *DB) nextRecordTime() uint64 { @@ -567,9 +570,9 @@ func (db *DB) GetWithBounds(key []byte, bounds TimestampBounds) ([]byte, bool, e return nil, false, err } defer func() { _ = iter.Close() }() - valid := false - if max, ok := bounds.seekUpper(); ok { - valid = iter.SeekGE(EncodeDataKey(nil, db.cf, key, max)) + var valid bool + if maxTS, ok := bounds.seekUpper(); ok { + valid = iter.SeekGE(EncodeDataKey(nil, db.cf, key, maxTS)) } else { valid = iter.First() } @@ -579,6 +582,8 @@ func (db *DB) GetWithBounds(key []byte, bounds TimestampBounds) ([]byte, bool, e continue } switch bounds.classify(ts) { + case timestampVisible: + // Read the value below. case timestampTooNew: continue case timestampTooOld: @@ -649,6 +654,8 @@ func (db *DB) Scan(opt ScanOptions) ([]KV, error) { continue } switch opt.Bounds.classify(ts) { + case timestampVisible: + // Process the value below. case timestampTooNew: continue case timestampTooOld: diff --git a/examples/kv/more_test.go b/examples/kv/more_test.go index e62e845..94ca9ae 100644 --- a/examples/kv/more_test.go +++ b/examples/kv/more_test.go @@ -483,7 +483,7 @@ func TestVerifyCheckpointAndRepairFromCheckpointRejectMissingCheckpoint(t *testi }{ { name: "verify", - call: func(t *testing.T, checkpoint string) error { + call: func(_ *testing.T, checkpoint string) error { return VerifyCheckpoint(checkpoint) }, }, @@ -2232,7 +2232,7 @@ func checkpointPutRecord(ref epaxos.InstanceRef, key, value string) epaxos.Insta Ballot: epaxos.Ballot{Replica: ref.Replica}, Status: epaxos.StatusCommitted, Seq: uint64(ref.Instance), - Deps: make([]epaxos.InstanceNum, max(int(ref.Replica), 1)), + Deps: make([]epaxos.InstanceNum, max(int(ref.Replica), 1)), //nolint:gosec // replica ID is small in tests Command: CommandForPut(99, uint64(ref.Instance), []byte(key), []byte(value)), }) } From 9c4f4ed159922760a6fcfc09328ecf0b65cadb21 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:57:27 +0900 Subject: [PATCH 08/12] ci: lint nested examples/kv module Root golangci-lint does not load the nested module; add a second CI step and document the local dual-module lint commands in AGENTS.md. --- .github/workflows/ci.yml | 6 ++++++ AGENTS.md | 1 + 2 files changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ab46cf..8c67b2c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,5 +57,11 @@ jobs: with: version: v2.12.2 + - name: Run golangci-lint (examples/kv) + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 + with: + version: v2.12.2 + working-directory: examples/kv + - name: Run repository verification gates run: tests/ci.sh diff --git a/AGENTS.md b/AGENTS.md index 4690c49..c524948 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,6 +6,7 @@ Run these commands before completing relevant changes: go test -race -count=1 ./... go vet ./... golangci-lint run ./... +(cd examples/kv && golangci-lint run ./...) tests/ci.sh ``` From 483790df42066449c607ab907d87324986d30f3a Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 23:10:36 +0900 Subject: [PATCH 09/12] fix: range-check bootstrap message type on decode Mirror parser.uvarint8 for bootstrap envelopes so oversized wire types cannot truncate into the valid BootstrapMessageType band. --- epaxos/bootstrap.go | 11 ++++++++- epaxos/bootstrap_fence_test.go | 43 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/epaxos/bootstrap.go b/epaxos/bootstrap.go index 19cc30c..c2dc360 100644 --- a/epaxos/bootstrap.go +++ b/epaxos/bootstrap.go @@ -1532,7 +1532,7 @@ func DecodeBootstrapMessage(src []byte, message *BootstrapMessage) error { if p.uvarint() != bootstrapWireVersion { return ErrInvalidBootstrapMessage } - message.Type = BootstrapMessageType(p.uvarint()) //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. + message.Type = BootstrapMessageType(p.uvarint8()) p.fixed((*[32]byte)(&message.Cluster)) p.fixed((*[32]byte)(&message.Plan)) message.From = ReplicaID(p.uvarint()) @@ -1666,6 +1666,15 @@ func (p *bootstrapParser) uvarint() uint64 { return value } +func (p *bootstrapParser) uvarint8() uint8 { + v := p.uvarint() + if v > uint64(^uint8(0)) { + p.err = true + return 0 + } + return uint8(v) +} + func (p *bootstrapParser) fixed(dst *[32]byte) { if p.err || len(p.b) < len(dst) { p.err = true diff --git a/epaxos/bootstrap_fence_test.go b/epaxos/bootstrap_fence_test.go index f9738e4..697f4c0 100644 --- a/epaxos/bootstrap_fence_test.go +++ b/epaxos/bootstrap_fence_test.go @@ -1,6 +1,7 @@ package epaxos import ( + "encoding/binary" "bytes" "errors" "reflect" @@ -435,3 +436,45 @@ func TestBootstrapEnvelopeAndChunkValidationIsCanonicalBoundedAndIdempotent(t *t t.Fatalf("same-index total conflict err=%v", err) } } + + +func TestDecodeBootstrapMessageRejectsOversizedType(t *testing.T) { + f := newBootstrapTestFixture(t, 1, 1) + plan := prepareBootstrapPlan(t, f) + message, err := BuildBootstrapMessage(BootstrapMessage{ + Type: BootstrapMsgReadyQuery, Cluster: f.cluster, Plan: plan.Request.Plan, + From: 1, FromIncarnation: 1, To: 1, BaseID: plan.Request.Base.ID, + BaseDigest: plan.RequestDigest, Payload: []byte("{}"), + }) + if err != nil { + t.Fatal(err) + } + encoded, err := EncodeBootstrapMessage(nil, message) + if err != nil { + t.Fatal(err) + } + // Replace the type uvarint (immediately after magic+version) with 257 so it + // would truncate to BootstrapMsgFenceRequest without range checking. + frame := append([]byte(nil), encoded[:len(bootstrapWireMagic)]...) + // re-encode version then oversized type then rest after original version+type + // Parse original after magic: version uvarint then type uvarint. + rest := encoded[len(bootstrapWireMagic):] + _, nVer := binary.Uvarint(rest) + if nVer <= 0 { + t.Fatal("version uvarint") + } + _, nType := binary.Uvarint(rest[nVer:]) + if nType <= 0 { + t.Fatal("type uvarint") + } + frame = append(frame, rest[:nVer]...) + frame = binary.AppendUvarint(frame, 257) + frame = append(frame, rest[nVer+nType:]...) + var got BootstrapMessage + if err := DecodeBootstrapMessage(frame, &got); !errors.Is(err, ErrInvalidBootstrapMessage) { + t.Fatalf("oversized bootstrap type err=%v, want ErrInvalidBootstrapMessage; got=%#v", err, got) + } + if got.Type != 0 || got.From != 0 || len(got.Payload) != 0 { + t.Fatalf("failed decode left residue: %#v", got) + } +} From 1ae5bed6a2653278fea295f9d7a82cb1d8749526 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 23:13:22 +0900 Subject: [PATCH 10/12] fix: bound AcceptEvidence deps on decode second pass Re-check depsCount against maxWireDeps before int conversion and arena slicing so a hostile evidence frame cannot panic the node. --- epaxos/codec.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/epaxos/codec.go b/epaxos/codec.go index b55bd57..52aca3e 100644 --- a/epaxos/codec.go +++ b/epaxos/codec.go @@ -254,9 +254,16 @@ func decodeMessage(src []byte, m *Message, scratch *DecodeScratch) error { for i := range m.AcceptEvidence { m.AcceptEvidence[i].Sender = ReplicaID(p.uvarint()) m.AcceptEvidence[i].Seq = p.uvarint() - deps := int(p.uvarint()) //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. + depsCount := p.uvarint() + if p.err || depsCount > maxWireDeps { + return decodeMessageError(m, scratch, ErrInvalidMessage) + } + deps := int(depsCount) if scratch != nil { end := evidenceOffset + deps + if end > len(evidenceArena) { + return decodeMessageError(m, scratch, ErrInvalidMessage) + } m.AcceptEvidence[i].Deps = evidenceArena[evidenceOffset:end:end] evidenceOffset = end } else { From 492d31585407d07a83c3371c46419eb244fd3eaf Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 23:20:27 +0900 Subject: [PATCH 11/12] style: drop bogus G115 nolints on faultWriteTrace modes MkdirAll/WriteFile already use restrictive 0o750/0o600; the G115 suppressions were false and would hide future mode regressions. --- epaxos/faultsim_trace_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/epaxos/faultsim_trace_test.go b/epaxos/faultsim_trace_test.go index 3df1b0b..addc8dc 100644 --- a/epaxos/faultsim_trace_test.go +++ b/epaxos/faultsim_trace_test.go @@ -217,10 +217,10 @@ func faultWriteTrace(path string, trace faultTrace) error { if err != nil { return err } - if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { return err } - return os.WriteFile(path, data, 0o600) //nolint:gosec // G115: conversion is bounded by protocol or test-fixture limits. + return os.WriteFile(path, data, 0o600) } func faultReadTrace(path string) (faultTrace, error) { From 4295ec2bf84b05b5e5ce01dbdbb66793fc004f30 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 14 Jul 2026 23:43:47 +0900 Subject: [PATCH 12/12] feat(epaxos): conflict engine core with model-equivalence tests Standalone per-lane sparse radix max-seq index and per-key postings with fold summaries; property tests prove oracle equivalence and fold domination. --- epaxos/conflict_engine.go | 893 +++++++++++++++++++++++++++++++++ epaxos/conflict_engine_test.go | 643 ++++++++++++++++++++++++ 2 files changed, 1536 insertions(+) create mode 100644 epaxos/conflict_engine.go create mode 100644 epaxos/conflict_engine_test.go diff --git a/epaxos/conflict_engine.go b/epaxos/conflict_engine.go new file mode 100644 index 0000000..a1dc730 --- /dev/null +++ b/epaxos/conflict_engine.go @@ -0,0 +1,893 @@ +package epaxos + +import "fmt" + +const ( + radixBits = 6 + radixFanout = 1 << radixBits + maxRadixLevel = 10 + retiredSeqBreakpointCap = 64 +) + +type laneSlotFlags uint8 + +const ( + laneSlotStatusNone laneSlotFlags = 1 << iota + laneSlotTOQPending + laneSlotNoop + laneSlotGlobalScope + laneSlotExecuted +) + +type laneSlot struct { + seq uint64 + flags laneSlotFlags +} + +func slotForRecord(rec InstanceRecord) laneSlot { + var flags laneSlotFlags + if rec.Status == StatusNone { + flags |= laneSlotStatusNone + } + if rec.TOQPending { + flags |= laneSlotTOQPending + } + if rec.Command.Kind == CommandNoop { + flags |= laneSlotNoop + } + if commandHasGlobalConflictScope(rec.Command.Kind) { + flags |= laneSlotGlobalScope + } + if rec.Status == StatusExecuted { + flags |= laneSlotExecuted + } + return laneSlot{seq: rec.Seq, flags: flags} +} + +func (s laneSlot) eligible() bool { + const ineligible = laneSlotStatusNone | laneSlotTOQPending | laneSlotNoop + return s.flags&ineligible == 0 +} + +func (s laneSlot) global() bool { return s.flags&laneSlotGlobalScope != 0 } + +type radixLeaf struct { + present uint64 + slots [radixFanout]laneSlot +} + +type radixChildren [radixFanout]*radixNode + +type radixNode struct { + level uint8 + children *radixChildren + leaf *radixLeaf + count int + maxSeq uint64 + maxEligibleAny InstanceNum + globalMax InstanceNum +} + +type laneTree struct { + root *radixNode +} + +func radixLevel(value InstanceNum) uint8 { + for level := uint8(0); level < maxRadixLevel; level++ { + if uint64(value) < uint64(1)<<(radixBits*(level+1)) { + return level + } + } + return maxRadixLevel +} + +func newRadixNode(level uint8) *radixNode { + node := &radixNode{level: level} + if level == 0 { + node.leaf = &radixLeaf{} + } else { + node.children = &radixChildren{} + } + return node +} + +func (t *laneTree) set(instance InstanceNum, slot laneSlot) { + level := radixLevel(instance) + if t.root == nil { + t.root = newRadixNode(level) + } + for t.root.level < level { + root := newRadixNode(t.root.level + 1) + root.children[0] = t.root + recomputeRadixNode(root, 0) + t.root = root + } + setRadixSlot(t.root, instance, slot, 0) +} + +func setRadixSlot(node *radixNode, instance InstanceNum, slot laneSlot, prefix InstanceNum) { + if node.level == 0 { + idx := uint(instance) & (radixFanout - 1) + node.leaf.slots[idx] = slot + node.leaf.present |= uint64(1) << idx + recomputeRadixNode(node, prefix) + return + } + shift := radixBits * node.level + idx := uint(instance>>shift) & (radixFanout - 1) + if node.children[idx] == nil { + node.children[idx] = newRadixNode(node.level - 1) + } + childPrefix := prefix | InstanceNum(idx)< t.root.level { + return false + } + removed := removeRadixSlot(t.root, instance, 0) + if !removed { + return false + } + if t.root.count == 0 { + t.root = nil + return true + } + for t.root.level > 0 && t.root.children[0] != nil && t.root.count == t.root.children[0].count { + t.root = t.root.children[0] + } + return true +} + +func removeRadixSlot(node *radixNode, instance InstanceNum, prefix InstanceNum) bool { + if node.level == 0 { + idx := uint(instance) & (radixFanout - 1) + bit := uint64(1) << idx + if node.leaf.present&bit == 0 { + return false + } + node.leaf.present &^= bit + node.leaf.slots[idx] = laneSlot{} + recomputeRadixNode(node, prefix) + return true + } + shift := radixBits * node.level + idx := uint(instance>>shift) & (radixFanout - 1) + child := node.children[idx] + if child == nil || !removeRadixSlot(child, instance, prefix|InstanceNum(idx)<>shift) & (radixFanout - 1) + var result uint64 + for idx := uint(0); idx <= last; idx++ { + result = max(result, prefixMaxRadix(node.children[idx], through, idx == last)) + } + return result +} + +func (t *laneTree) walkDesc(from InstanceNum, yield func(InstanceNum, laneSlot) bool) { + if t.root == nil { + return + } + walkRadixDesc(t.root, 0, from, radixLevel(from) <= t.root.level, yield) +} + +func walkRadixDesc(node *radixNode, prefix, from InstanceNum, limited bool, yield func(InstanceNum, laneSlot) bool) bool { + if node == nil { + return true + } + if node.level == 0 { + last := uint(radixFanout - 1) + if limited { + last = uint(from) & (radixFanout - 1) + } + for idx := int(last); idx >= 0; idx-- { + if node.leaf.present&(uint64(1)<>shift) & (radixFanout - 1) + } + for idx := int(last); idx >= 0; idx-- { + childLimited := limited && uint(idx) == last + if !walkRadixDesc(node.children[idx], prefix|InstanceNum(idx)< node.level { + return laneSlot{}, false + } + for node.level > 0 { + idx := uint(instance>>(radixBits*node.level)) & (radixFanout - 1) + node = node.children[idx] + if node == nil { + return laneSlot{}, false + } + } + idx := uint(instance) & (radixFanout - 1) + if node.leaf.present&(uint64(1)<>shift) & (radixFanout - 1) + if node.children[idx] == nil { + node.children[idx] = newPostingNode(node.level - 1) + } + insertPosting(node.children[idx], instance, prefix|InstanceNum(idx)< s.root.level || !removePosting(s.root, instance, 0) { + return false + } + if s.root.count == 0 { + s.root = nil + return true + } + for s.root.level > 0 && s.root.children[0] != nil && s.root.count == s.root.children[0].count { + s.root = s.root.children[0] + } + return true +} + +func removePosting(node *postingNode, instance, prefix InstanceNum) bool { + if node.level == 0 { + idx := uint(instance) & (radixFanout - 1) + bit := uint64(1) << idx + if node.leaf.present&bit == 0 { + return false + } + node.leaf.present &^= bit + recomputePostingNode(node, prefix) + return true + } + shift := radixBits * node.level + idx := uint(instance>>shift) & (radixFanout - 1) + child := node.children[idx] + if child == nil || !removePosting(child, instance, prefix|InstanceNum(idx)<= 0; idx-- { + if node.leaf.present&(uint64(1)<>shift) & (radixFanout - 1) + } + for idx := int(last); idx >= 0; idx-- { + if !walkPostingDesc(node.children[idx], prefix|InstanceNum(idx)<> 1) + if i.retiredSeq[mid].through <= through { + lo = mid + 1 + } else { + hi = mid + } + } + return i.retiredSeq[lo-1].maxSeq +} + +func (e *conflictEngine) walkDesc(lane instanceLane, from InstanceNum, yield func(InstanceNum, laneSlot) bool) { + if index := e.laneIndex[lane]; index != nil { + index.resident.walkDesc(from, yield) + } +} + +func (e *conflictEngine) keyMax(conf ConfID, key []byte, lane instanceLane) (resident, retired InstanceNum) { + keys := e.byKey[conf] + if keys == nil { + return 0, 0 + } + lanes := keys[string(key)] + if lanes == nil { + return 0, 0 + } + entry := (*lanes)[lane] + if entry == nil { + return 0, 0 + } + return entry.postings.max(), entry.retiredFloor +} + +func (e *conflictEngine) keyLaneSet(conf ConfID, keys [][]byte, yield func(instanceLane) bool) { + for lane := range e.laneIndex { + if lane.conf != conf { + continue + } + for _, key := range keys { + byConf := e.byKey[conf] + if byConf == nil { + break + } + byLane := byConf[string(key)] + if byLane == nil || (*byLane)[lane] == nil { + continue + } + if !yield(lane) { + return + } + break + } + } +} + +func (e *conflictEngine) lanes(conf ConfID, yield func(instanceLane) bool) { + for lane := range e.laneIndex { + if lane.conf == conf && !yield(lane) { + return + } + } +} + +func (e *conflictEngine) maxEligibleAny(lane instanceLane) InstanceNum { + index := e.laneIndex[lane] + if index == nil { + return 0 + } + resident := InstanceNum(0) + if index.resident.root != nil { + resident = index.resident.root.maxEligibleAny + } + return max(resident, index.retiredEligibleAny) +} + +func (e *conflictEngine) globalMax(lane instanceLane) InstanceNum { + index := e.laneIndex[lane] + if index == nil { + return 0 + } + resident := InstanceNum(0) + if index.resident.root != nil { + resident = index.resident.root.globalMax + } + return max(resident, index.retiredGlobal) +} + +func (e *conflictEngine) foldRecord(rec InstanceRecord) { + lane := laneFor(rec.Ref) + index := e.ensureLane(lane) + e.remove(rec.Ref, rec) + if rec.Ref.Instance <= index.folded { + return + } + index.pendingFold.insert(rec.Ref.Instance) + if !recordConflictEligible(rec) { + return + } + index.retiredEligibleAny = max(index.retiredEligibleAny, rec.Ref.Instance) + if commandHasGlobalConflictScope(rec.Command.Kind) { + index.retiredGlobal = max(index.retiredGlobal, rec.Ref.Instance) + } else { + for _, key := range rec.Command.ConflictKeys { + entry := e.ensureKeyLane(rec.Ref.Conf, key, lane) + entry.retiredFloor = max(entry.retiredFloor, rec.Ref.Instance) + } + } + index.addRetiredSeq(rec.Ref.Instance, rec.Seq) +} + +func (i *laneIndex) addRetiredSeq(through InstanceNum, seq uint64) { + prior := i.retiredPrefixMaxSeq(through) + if seq <= prior { + return + } + pos := 0 + for pos < len(i.retiredSeq) && i.retiredSeq[pos].through < through { + pos++ + } + if pos < len(i.retiredSeq) && i.retiredSeq[pos].through == through { + i.retiredSeq[pos].maxSeq = max(i.retiredSeq[pos].maxSeq, seq) + } else { + i.retiredSeq = append(i.retiredSeq, retiredSeqBreakpoint{}) + copy(i.retiredSeq[pos+1:], i.retiredSeq[pos:]) + i.retiredSeq[pos] = retiredSeqBreakpoint{through: through, maxSeq: seq} + } + end := pos + 1 + for end < len(i.retiredSeq) && i.retiredSeq[end].maxSeq <= i.retiredSeq[pos].maxSeq { + end++ + } + if end > pos+1 { + copy(i.retiredSeq[pos+1:], i.retiredSeq[end:]) + i.retiredSeq = i.retiredSeq[:len(i.retiredSeq)-(end-pos-1)] + } + if len(i.retiredSeq) > retiredSeqBreakpointCap { + drop := len(i.retiredSeq) - retiredSeqBreakpointCap + copy(i.retiredSeq, i.retiredSeq[drop:]) + i.retiredSeq = i.retiredSeq[:retiredSeqBreakpointCap] + i.seqCompressed = true + } +} + +func (e *conflictEngine) advanceFold(lane instanceLane, through InstanceNum) { + index := e.ensureLane(lane) + if through < index.folded { + panic("epaxos: conflict engine fold watermark regression") + } + if through == index.folded { + return + } + for instance := index.folded + 1; ; instance++ { + if !index.pendingFold.contains(instance) { + panic("epaxos: conflict engine non-contiguous fold") + } + if instance == through { + break + } + } + for instance := index.folded + 1; ; instance++ { + if !index.pendingFold.remove(instance) { + panic("epaxos: conflict engine fold preflight changed") + } + if instance == through { + break + } + } + index.folded = through +} + +func (e *conflictEngine) foldedThrough(lane instanceLane) InstanceNum { + if index := e.laneIndex[lane]; index != nil { + return index.folded + } + return 0 +} + +func (e *conflictEngine) residentCount() int { return e.resident } + +func (e *conflictEngine) verify() error { + resident := 0 + for lane, index := range e.laneIndex { + if err := verifyRadixNode(index.resident.root, 0); err != nil { + return fmt.Errorf("lane %v: %w", lane, err) + } + if err := verifyPostingNode(index.pendingFold.root, 0); err != nil { + return fmt.Errorf("lane %v pending fold: %w", lane, err) + } + if index.resident.root != nil { + resident += index.resident.root.count + } + if len(index.retiredSeq) > retiredSeqBreakpointCap { + return fmt.Errorf("lane %v: retired seq breakpoint cap exceeded", lane) + } + for idx := 1; idx < len(index.retiredSeq); idx++ { + if index.retiredSeq[idx-1].through >= index.retiredSeq[idx].through || index.retiredSeq[idx-1].maxSeq >= index.retiredSeq[idx].maxSeq { + return fmt.Errorf("lane %v: retired seq breakpoints are not increasing", lane) + } + } + } + if resident != e.resident { + return fmt.Errorf("resident count: aggregate=%d tracked=%d", resident, e.resident) + } + // Forward: every posting entry must name an eligible non-global resident. + for conf, keys := range e.byKey { + for key, lanes := range keys { + for lane, entry := range *lanes { + if lane.conf != conf { + return fmt.Errorf("key %q: lane configuration mismatch", key) + } + if err := verifyPostingNode(entry.postings.root, 0); err != nil { + return fmt.Errorf("key %q lane %v: %w", key, lane, err) + } + valid := true + walkPostingDesc(entry.postings.root, 0, ^InstanceNum(0), false, func(instance InstanceNum) bool { + slot, ok := e.laneIndex[lane].resident.slot(instance) + if !ok || !slot.eligible() || slot.global() { + valid = false + return false + } + return true + }) + if !valid { + return fmt.Errorf("key %q lane %v: posting lacks eligible resident slot", key, lane) + } + } + } + } + return nil +} + +func verifyRadixNode(node *radixNode, prefix InstanceNum) error { + if node == nil { + return nil + } + copyNode := *node + recomputeRadixNode(©Node, prefix) + if copyNode.count != node.count || copyNode.maxSeq != node.maxSeq || copyNode.maxEligibleAny != node.maxEligibleAny || copyNode.globalMax != node.globalMax { + return fmt.Errorf("radix aggregate mismatch at level %d", node.level) + } + if node.level == 0 { + return nil + } + shift := radixBits * node.level + for idx, child := range node.children { + if child == nil { + continue + } + if child.level+1 != node.level { + return fmt.Errorf("radix level mismatch: parent=%d child=%d", node.level, child.level) + } + if err := verifyRadixNode(child, prefix|InstanceNum(idx)< want[j] }) + if fmt.Sprint(got) != fmt.Sprint(want) { + t.Fatalf("step %d lane %v: walk=%v, want %v", step, lane, got, want) + } +} + +func TestConflictEnginePostFoldDomination(t *testing.T) { + t.Parallel() + + var engine conflictEngine + lane := instanceLane{conf: 7, replica: 2} + records := []InstanceRecord{ + conflictRecord(lane, 1, 3, CommandUser, "a"), + conflictRecord(lane, 2, 9, CommandUser, "b"), + conflictRecord(lane, 3, 5, CommandConfChange), + conflictRecord(lane, 4, 11, CommandUser, "a"), + } + for idx := range records { + engine.apply(nil, records[idx]) + } + queries := []Command{ + {Kind: CommandUser, ConflictKeys: [][]byte{[]byte("a")}}, + {Kind: CommandUser, ConflictKeys: [][]byte{[]byte("b")}}, + {Kind: CommandConfChange}, + } + before := make([]modelAttrs, len(queries)) + for idx, query := range queries { + before[idx] = engineModelAttrs(&engine, lane.conf, query) + } + for _, rec := range records { + engine.foldRecord(rec) + } + if got := engine.foldedThrough(lane); got != 0 { + t.Fatalf("foldRecord published watermark %d before advanceFold", got) + } + engine.advanceFold(lane, 4) + for idx, query := range queries { + after := engineModelAttrs(&engine, lane.conf, query) + beforeAttrs, afterAttrs := before[idx][lane], after[lane] + if afterAttrs.dep < beforeAttrs.dep || afterAttrs.seq < beforeAttrs.seq { + t.Fatalf("query %+v decreased across fold: before=%+v after=%+v", query, beforeAttrs, afterAttrs) + } + } + if got := engine.globalMax(lane); got != 3 { + t.Fatalf("retired global max=%d, want 3", got) + } + if got := engine.maxEligibleAny(lane); got != 4 { + t.Fatalf("retired eligible max=%d, want 4", got) + } + if err := engine.verify(); err != nil { + t.Fatal(err) + } +} + +func TestConflictEngineIdempotence(t *testing.T) { + t.Parallel() + + var engine conflictEngine + lane := instanceLane{conf: 1, replica: 1} + rec := conflictRecord(lane, 1, 7, CommandUser, "a") + engine.apply(nil, rec) + engine.apply(&rec, rec) + if got := engine.residentCount(); got != 1 { + t.Fatalf("resident count after apply(rec, rec)=%d, want 1", got) + } + engine.foldRecord(rec) + engine.foldRecord(rec) + resident, retired := engine.keyMax(1, []byte("a"), lane) + if resident != 0 || retired != 1 { + t.Fatalf("key max after double fold=(%d,%d), want (0,1)", resident, retired) + } + engine.advanceFold(lane, 1) + engine.foldRecord(rec) + if got := engine.foldedThrough(lane); got != 1 { + t.Fatalf("folded through=%d, want 1", got) + } + if err := engine.verify(); err != nil { + t.Fatal(err) + } +} + +func TestConflictEngineNoopMutationDropsMax(t *testing.T) { + t.Parallel() + + var engine conflictEngine + lane := instanceLane{conf: 4, replica: 3} + lower := conflictRecord(lane, 10, 2, CommandUser, "key") + higher := conflictRecord(lane, 20, 8, CommandUser, "key") + engine.apply(nil, lower) + engine.apply(nil, higher) + noop := higher + noop.Command.Kind = CommandNoop + engine.apply(&higher, noop) + resident, retired := engine.keyMax(lane.conf, []byte("key"), lane) + if resident != lower.Ref.Instance || retired != 0 { + t.Fatalf("key max after noop mutation=(%d,%d), want (%d,0)", resident, retired, lower.Ref.Instance) + } + if got := engine.maxEligibleAny(lane); got != lower.Ref.Instance { + t.Fatalf("max eligible after noop mutation=%d, want %d", got, lower.Ref.Instance) + } + if err := engine.verify(); err != nil { + t.Fatal(err) + } +} + +func TestConflictEngineSparseOutlierDepth(t *testing.T) { + t.Parallel() + + var engine conflictEngine + lane := instanceLane{conf: 1, replica: 1} + outlier := InstanceNum(1)<<60 + 17 + rec := conflictRecord(lane, outlier, 99, CommandUser, "far") + engine.apply(nil, rec) + index := engine.laneIndex[lane] + nodes, leaves := countRadixNodes(index.resident.root) + if leaves != 1 || nodes > 11 { + t.Fatalf("outlier allocated nodes=%d leaves=%d, want <=11 nodes and 1 leaf", nodes, leaves) + } + if got := engine.prefixMaxSeq(lane, outlier); got != rec.Seq { + t.Fatalf("prefix max at outlier=%d, want %d", got, rec.Seq) + } + var walked []InstanceNum + engine.walkDesc(lane, ^InstanceNum(0), func(instance InstanceNum, _ laneSlot) bool { + walked = append(walked, instance) + return true + }) + if len(walked) != 1 || walked[0] != outlier { + t.Fatalf("walked=%v, want [%d]", walked, outlier) + } +} + +func countRadixNodes(node *radixNode) (nodes, leaves int) { + if node == nil { + return 0, 0 + } + nodes = 1 + if node.level == 0 { + return nodes, 1 + } + for _, child := range node.children { + childNodes, childLeaves := countRadixNodes(child) + nodes += childNodes + leaves += childLeaves + } + return nodes, leaves +} + +func TestConflictEngineBreakpointCapOvershootsSafely(t *testing.T) { + t.Parallel() + + var engine conflictEngine + lane := instanceLane{conf: 8, replica: 1} + for instance := InstanceNum(1); instance <= 96; instance++ { + rec := conflictRecord(lane, instance, uint64(instance), CommandUser, "key") + engine.apply(nil, rec) + engine.foldRecord(rec) + } + engine.advanceFold(lane, 96) + index := engine.laneIndex[lane] + if got := len(index.retiredSeq); got != retiredSeqBreakpointCap { + t.Fatalf("breakpoint count=%d, want %d", got, retiredSeqBreakpointCap) + } + if got := engine.prefixMaxSeq(lane, 1); got < 1 { + t.Fatalf("compressed prefix undershot: got %d, want >=1", got) + } + if got := engine.prefixMaxSeq(lane, 1); got == 1 { + t.Fatalf("old compressed prefix did not exercise permitted overshoot") + } + if got := engine.prefixMaxSeq(lane, 90); got != 90 { + t.Fatalf("recent prefix=%d, want exact 90", got) + } + if err := engine.verify(); err != nil { + t.Fatal(err) + } +} + +func TestConflictEngineFoldTouchesOnlyRecordKeys(t *testing.T) { + t.Parallel() + + var engine conflictEngine + lane := instanceLane{conf: 9, replica: 1} + const unrelatedKeys = 2_000 + for instance := InstanceNum(2); instance < unrelatedKeys+2; instance++ { + rec := conflictRecord(lane, instance, uint64(instance), CommandUser, fmt.Sprintf("unrelated-%04d", instance-2)) + engine.apply(nil, rec) + } + target := conflictRecord(lane, 1, 1, CommandUser, "target") + engine.apply(nil, target) + unrelated := engine.byKey[lane.conf]["unrelated-1000"] + unrelatedRoot := (*unrelated)[lane].postings.root + engine.foldRecord(target) + if (*unrelated)[lane].postings.root != unrelatedRoot { + t.Fatal("fold rebuilt an unrelated posting tree") + } + if got := (*unrelated)[lane].postings.max(); got != 1_002 { + t.Fatalf("unrelated posting max=%d, want 1002", got) + } + if got := len(engine.byKey[lane.conf]); got != unrelatedKeys+1 { + t.Fatalf("key count=%d, want %d", got, unrelatedKeys+1) + } + if err := engine.verify(); err != nil { + t.Fatal(err) + } +} + +func TestConflictEngineAdvanceRejectsNonContiguousFold(t *testing.T) { + t.Parallel() + + var engine conflictEngine + lane := instanceLane{conf: 1, replica: 1} + rec := conflictRecord(lane, 2, 1, CommandUser, "a") + engine.apply(nil, rec) + engine.foldRecord(rec) + defer func() { + if recover() == nil { + t.Fatal("advanceFold accepted a missing folded instance") + } + if got := engine.foldedThrough(lane); got != 0 { + t.Fatalf("failed advance published watermark %d", got) + } + }() + engine.advanceFold(lane, 2) +} + +func conflictRecord(lane instanceLane, instance InstanceNum, seq uint64, kind CommandKind, keys ...string) InstanceRecord { + conflictKeys := make([][]byte, len(keys)) + for idx, key := range keys { + conflictKeys[idx] = []byte(key) + } + return InstanceRecord{ + Ref: InstanceRef{ + Replica: lane.replica, + Instance: instance, + Conf: lane.conf, + }, + Status: StatusCommitted, + Seq: seq, + Command: Command{Kind: kind, ConflictKeys: conflictKeys}, + } +} + +// assertExactKeyPostings checks the converse of verify()'s posting→slot walk: +// every eligible non-global resident record's conflict keys appear in byKey postings. +// Kept in tests so production state does not clone keys into a reverse map. +func assertExactKeyPostings(t *testing.T, e *conflictEngine, records map[InstanceRef]InstanceRecord) { + t.Helper() + for ref, rec := range records { + if !recordConflictEligible(rec) || commandHasGlobalConflictScope(rec.Command.Kind) { + continue + } + lane := laneFor(ref) + for _, key := range rec.Command.ConflictKeys { + keys := e.byKey[ref.Conf] + if keys == nil { + t.Fatalf("missing byKey conf for %v key %q", ref, key) + } + lanes := keys[string(key)] + if lanes == nil { + t.Fatalf("missing byKey entry for %v key %q", ref, key) + } + entry := (*lanes)[lane] + if entry == nil || !entry.postings.contains(ref.Instance) { + t.Fatalf("missing posting for %v key %q", ref, key) + } + } + } +}