amadan
moves log: only a declared target loss is declared_lost (final re-review 1, N1)
Every successful DeclareMoveLoss queued rolling_back-target-lost, and moves.DeclaredLost built a target-loss transition from any move, so a destroyed-local-only declaration for a committed move's retired source was logged as the target's loss. Now: - hostops.DeclaresMoveTargetLoss reads the submission's signed challenge. Only a move-target-lost consequence for the record's own move and incarnation queues the row, on both declaration doors. - hostops.MoveTargetLossOf reads a loss record's signed recovery intent. The drainer confirms an unconfirmed row only from a target loss of that move, never from a destroyed-local-only loss of the same op. - moves.DeclaredLost answers only for a landing or a rollback, the path a target loss is declared on. Regressions: a destroyed-local-only declaration on a committed move's retired source, through the move door and the host operator door, leaves the move committed, queues no row and logs no event. DeclaredLost refuses every state off the rollback path.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7 files changed,
+136
−16
internal/console/apihostclaim.go+1 −1internal/console/apiinstancemove.go+1 −1internal/console/movesrecovery.go+21 −7internal/console/movesrecovery_test.go+55 −0internal/hostops/move_loss_authority.go+32 −0internal/moves/transition.go+10 −7internal/moves/transition_test.go+16 −0
diff --git a/internal/console/apihostclaim.go b/internal/console/apihostclaim.go| index e3f230e1e..83e8c10db 100644 |
| --- a/internal/console/apihostclaim.go |
| +++ b/internal/console/apihostclaim.go |
| @@ -120,7 +120,7 @@ func (s *Server) handleAPIHostResolve(w http.ResponseWriter, r *http.Request) { |
| // is the signed challenge's. |
| if in.Action == "declare-lost" { |
| if account, app, ok := s.claimMoveLossHolder(r, r.PathValue("host")); ok { |
| - if err := s.declareMoveLossLogged(r.Context(), account, app, r.PathValue("host"), func() error { |
| + if err := s.declareMoveLossLogged(r.Context(), account, app, r.PathValue("host"), in.Submission, func() error { |
| return s.cfg.ClaimAuthority.DeclareMoveLoss(r.Context(), actor, account, app, r.PathValue("host"), s.claimChallengeBox(in.Submission), in.Submission) |
| }); err != nil { |
| writeClaimResponseError(w, err) |
diff --git a/internal/console/apiinstancemove.go b/internal/console/apiinstancemove.go| index 9fd2ffc44..9f44c8790 100644 |
| --- a/internal/console/apiinstancemove.go |
| +++ b/internal/console/apiinstancemove.go |
| @@ -1081,7 +1081,7 @@ func (s *Server) claimMoveLoss(w http.ResponseWriter, r *http.Request, acct *Acc |
| writeJSON(w, http.StatusOK, in) |
| return |
| } |
| - if err := s.declareMoveLossLogged(ctx, acct.Sqid, app, host, func() error { |
| + if err := s.declareMoveLossLogged(ctx, acct.Sqid, app, host, *sub, func() error { |
| return a.DeclareMoveLoss(ctx, actor, acct.Sqid, app, host, box, *sub) |
| }); err != nil { |
| claimMoveLossFailed(w, acct, app, host, err) |
diff --git a/internal/console/movesrecovery.go b/internal/console/movesrecovery.go| index d777e43fa..c2ebae1f3 100644 |
| --- a/internal/console/movesrecovery.go |
| +++ b/internal/console/movesrecovery.go |
| @@ -616,6 +616,16 @@ func (s *Server) queueMoveLogErr(o hostops.ConsoleMoveObservation, t moves.Trans |
| return s.store.QueueMoveLog(o.Host, o.OpID, t.Identity, t.Incarnation, body, confirmed, now) |
| } |
| +// claimRootKey is the root public key the claim runtime signs with, which |
| +// the move loss challenges and records verify under; nil without the |
| +// runtime. |
| +func (s *Server) claimRootKey() ed25519.PublicKey { |
| + if a := s.cfg.ClaimAuthority; a != nil { |
| + return a.Signer.RootPublicKey() |
| + } |
| + return nil |
| +} |
| + |
| // confirmMoveLog confirms t's entry once the write it names succeeded. The |
| // row is queued again, confirmed, in case a pass dropped the unconfirmed one |
| // in between (settleDeclaredLost drops a row whose loss is not committed |
| @@ -637,7 +647,7 @@ func (s *Server) confirmMoveLog(o hostops.ConsoleMoveObservation, t moves.Transi |
| // read, or a row that cannot be queued, refuses the declaration: an |
| // operator's action fails closed rather than leave no retry for its entry |
| // (final re-review 1, I4). |
| -func (s *Server) declareMoveLossLogged(ctx context.Context, account, app, host string, declare func() error) error { |
| +func (s *Server) declareMoveLossLogged(ctx context.Context, account, app, host string, sub hostfence.Submission, declare func() error) error { |
| var ( |
| o hostops.ConsoleMoveObservation |
| t moves.Transition |
| @@ -649,7 +659,10 @@ func (s *Server) declareMoveLossLogged(ctx context.Context, account, app, host s |
| // cannot be queued is not made. |
| return fmt.Errorf("moves: the declared loss cannot be queued in the move log: the record cannot be read: %w", err) |
| } |
| - if rec.Move != nil { |
| + // Only a declaration whose signed consequence is the loss of this move's |
| + // target is declared_lost; a destroyed-local-only one changes no move |
| + // state and logs nothing (final re-review 1, N1). |
| + if op, inc, ok := hostops.DeclaresMoveTargetLoss(s.claimRootKey(), sub); ok && rec.Move != nil && op == rec.Move.ID && inc == rec.Move.Incarnation { |
| if t, logged = moves.DeclaredLost(rec.Move); logged { |
| o = hostops.ConsoleMoveObservation{Host: host, OpID: rec.Move.ID, Incarnation: rec.Move.Incarnation} |
| if err := s.queueMoveLogErr(o, t, s.movesNow(), false); err != nil { |
| @@ -753,10 +766,7 @@ func (s *Server) settleDeclaredLost(ctx context.Context, r MoveLogPendingEntry) |
| return drop("the claim no longer holds the instance it was queued for") |
| } |
| if len(o.Claim.LossRefs) > 0 { |
| - var pub ed25519.PublicKey |
| - if a := s.cfg.ClaimAuthority; a != nil { |
| - pub = a.Signer.RootPublicKey() |
| - } |
| + pub := s.claimRootKey() |
| if len(pub) != ed25519.PublicKeySize { |
| return false, errors.New("unconfirmed, and there is no key to read the claim's loss records") |
| } |
| @@ -768,7 +778,11 @@ func (s *Server) settleDeclaredLost(ctx context.Context, r MoveLogPendingEntry) |
| if err != nil { |
| return false, fmt.Errorf("unconfirmed, and a loss record cannot be read: %w", err) |
| } |
| - if l.Record.Host != r.Host || l.Record.OpID != r.Op || l.Record.HolderIncarnation != r.Incarnation { |
| + // Only a loss of the move's target, signed as such, is the |
| + // row's evidence: a destroyed-local-only loss of the same op is |
| + // not (final re-review 1, N1). |
| + op, inc, target := hostops.MoveTargetLossOf(l.Record) |
| + if !target || op != r.Op || inc != r.Incarnation || l.Record.Host != r.Host || l.Record.OpID != r.Op { |
| continue |
| } |
| if err := s.store.ConfirmMoveLog(r.Host, r.Op, r.Name); err != nil { |
diff --git a/internal/console/movesrecovery_test.go b/internal/console/movesrecovery_test.go| index bc588c5c1..bd856c367 100644 |
| --- a/internal/console/movesrecovery_test.go |
| +++ b/internal/console/movesrecovery_test.go |
| @@ -1429,3 +1429,58 @@ func j4HostDoor(t *testing.T, fx *consoleFixture) string { |
| } |
| return fx.ts.URL + "/api/cli/ops/hosts/" + j4Host + "/resolve" |
| } |
| + |
| +// Final re-review 1, N1: only a declaration whose signed consequence is the |
| +// loss of the move's target (the rollback path, move-target-lost) is |
| +// declared_lost. A destroyed-local-only declaration, here for a committed |
| +// move's retired source, changes no move state and logs no move event, |
| +// through either door. |
| +func TestDestroyedLocalOnlyLogsNoMoveEvent(t *testing.T) { |
| + for _, door := range []string{"move", "host"} { |
| + t.Run(door, func(t *testing.T) { |
| + ctx := context.Background() |
| + fx, token, moveURL := j4LandingMove(t, "code-mvlog-localonly-"+door) |
| + w := newMovePassWorld(t, fx, j17MoveState(t, fx).ID) |
| + obs, err := hostclaim.Read(ctx, fx.rel, j4Host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := hostclaim.Update(ctx, fx.rel, obs, func(c *hostclaim.Claim) error { |
| + c.Op.Phase = hostclaim.PhaseCommitted |
| + c.Obligations = append(c.Obligations, hostclaim.Obligation{ID: "local", Op: w.id, Kind: "park-local", Box: "ie-box", Retirement: "ret-local", Owner: "box:ie-box"}) |
| + return nil |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + committed := time.Now().UTC() |
| + j17SetRecord(t, fx, func(r *provision.Record) { r.Move.State, r.Move.CommittedAt = provision.MoveCommitted, &committed }) |
| + inspectURL, inspectBody, declURL := moveURL+"/resolve", `{"declare_lost":"ie-box"}`, moveURL+"/resolve" |
| + if door == "host" { |
| + base := j4HostDoor(t, fx) |
| + inspectURL, inspectBody, declURL = base+"/inspect", `{"action":"declare-lost","box":"ie-box"}`, base |
| + } |
| + c, _, out := rawReq(t, http.MethodPost, inspectURL, token, inspectBody) |
| + if c != http.StatusOK || !strings.Contains(out, "destroyed-local-only") { |
| + t.Fatalf("inspect = %d %s, want a destroyed-local-only challenge", c, out) |
| + } |
| + sub := j4Submission(t, fx, out) |
| + body := j4Body(t, map[string]any{"declare_lost": "ie-box", "submission": sub}) |
| + if door == "host" { |
| + body = j4Body(t, map[string]any{"action": "declare-lost", "submission": sub}) |
| + } |
| + if c, _, out := rawReq(t, http.MethodPost, declURL, token, body); c != http.StatusAccepted && c != http.StatusOK { |
| + t.Fatalf("declare = %d %s", c, out) |
| + } |
| + if m := w.record(t).Move; m.State != provision.MoveCommitted { |
| + t.Fatalf("a local-only declaration changed the move: %+v", m) |
| + } |
| + if p := pendingLog(t, w); len(p) != 0 { |
| + t.Fatalf("a local-only declaration queued %v", p) |
| + } |
| + w.pass(t) |
| + if n := declaredLostEntries(t, w); n != 0 { |
| + t.Fatalf("a local-only declaration logged %d declared_lost entries", n) |
| + } |
| + }) |
| + } |
| +} |
diff --git a/internal/hostops/move_loss_authority.go b/internal/hostops/move_loss_authority.go| index 1d20dd54d..81e824411 100644 |
| --- a/internal/hostops/move_loss_authority.go |
| +++ b/internal/hostops/move_loss_authority.go |
| @@ -2,10 +2,13 @@ package hostops |
| import ( |
| "context" |
| + "crypto/ed25519" |
| "slices" |
| + "github.com/carlosframework/platform/internal/hostclaim" |
| "github.com/carlosframework/platform/internal/hostfence" |
| "github.com/carlosframework/platform/internal/hostresolve" |
| + "github.com/carlosframework/platform/internal/release" |
| ) |
| // MoveLossKind reports a claim op kind whose target loss the moves owner |
| @@ -30,3 +33,32 @@ func (a *ProductionAuthority) DeclareMoveLoss(ctx context.Context, actor, accoun |
| _, err := a.Control.DeclareMoveLoss(ctx, a.Signer, a.Directory.Bucket, actor, account, app, host, box, in) |
| return err |
| } |
| + |
| +// DeclaresMoveTargetLoss reports whether a submitted declaration's signed |
| +// challenge declares the loss of a move's target, the rollback path |
| +// (move-target-lost), and of which move and instance incarnation. A |
| +// destroyed-local-only declaration, which changes no move state, is not |
| +// one, and neither is a challenge that does not verify under pub. The move |
| +// log names only a target loss (Plan 6 final re-review 1, N1). |
| +func DeclaresMoveTargetLoss(pub ed25519.PublicKey, in hostfence.Submission) (op, incarnation string, ok bool) { |
| + var c hostfence.Challenge |
| + if len(pub) != ed25519.PublicKeySize || release.VerifyRootEnvelope(pub, in.Challenge, &c) != nil { |
| + return "", "", false |
| + } |
| + if c.Consequences.Transition != moveLossTransition || c.Op.ID == "" { |
| + return "", "", false |
| + } |
| + return c.Op.ID, c.HolderIncarnation, true |
| +} |
| + |
| +// MoveTargetLossOf reports whether a loss record's signed consequences are |
| +// the loss of a move's target (its recovery intent is move-target-lost), and |
| +// of which move and instance incarnation. A destroyed-local-only record is |
| +// not (Plan 6 final re-review 1, N1). |
| +func MoveTargetLossOf(l hostclaim.LossRecord) (op, incarnation string, ok bool) { |
| + in, err := decodeMoveLossIntent(l.ConfirmedConsequences.RecoveryIntent) |
| + if err != nil { |
| + return "", "", false |
| + } |
| + return in.Move, in.Incarnation, true |
| +} |
diff --git a/internal/moves/transition.go b/internal/moves/transition.go| index 548201780..079679989 100644 |
| --- a/internal/moves/transition.go |
| +++ b/internal/moves/transition.go |
| @@ -81,14 +81,17 @@ const DeclaredLostIdentity = "rolling_back-target-" + provision.RbTargetLost |
| // DeclaredLost is the entry of a declared loss of m's target (§7.10), event |
| // declared_lost, named as TransitionOf names the rollback's target going to |
| -// lost, from whatever state the record is in when the writer names it: the |
| -// console that performs the declaration names it from the record it read |
| -// before (a landing, or a rollback whose target is releasing or returning), |
| -// and a box whose pass sees the target lost names it from that record. The |
| -// identity, DeclaredLostIdentity, is the same for both, so create-only |
| -// storage keeps one (final review I4). ok is false for a move with no id. |
| +// lost. The console that performs a declaration whose signed consequence is |
| +// that loss names it from the record it read before (a landing, which the |
| +// declaration rolls back, or a rollback whose target is releasing or |
| +// returning), and a box whose pass sees the target lost names it from that |
| +// record. The identity, DeclaredLostIdentity, is the same for both, so |
| +// create-only storage keeps one (final review I4). A target loss is declared |
| +// only on that path, so ok is false for a move in any other state (a |
| +// committed move's destroyed-local-only declaration changes no move state: |
| +// final re-review 1, N1) and for a move with no id. |
| func DeclaredLost(m *provision.MoveState) (Transition, bool) { |
| - if m == nil || m.ID == "" { |
| + if m == nil || m.ID == "" || (m.State != provision.MoveLanding && m.State != provision.MoveRollingBack) { |
| return Transition{}, false |
| } |
| var rb provision.MoveRollback |
diff --git a/internal/moves/transition_test.go b/internal/moves/transition_test.go| index 64265e7e4..b0ac4a21e 100644 |
| --- a/internal/moves/transition_test.go |
| +++ b/internal/moves/transition_test.go |
| @@ -34,3 +34,19 @@ func TestStuckNamesTheParticipantThatStopped(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// Final re-review 1, N1: DeclaredLost names a target loss only for a move on |
| +// the rollback path a target loss is declared on: a landing (which the |
| +// declaration rolls back) or a rollback. It fabricates none for any other |
| +// state, a committed move's above all. |
| +func TestDeclaredLostOnlyOnTheRollbackPath(t *testing.T) { |
| + for state, want := range map[string]bool{ |
| + provision.MoveLanding: true, provision.MoveRollingBack: true, |
| + provision.MoveCommitted: false, provision.MovePreparing: false, provision.MoveCutting: false, provision.MoveReverting: false, provision.MoveStuck: false, |
| + } { |
| + e, ok := DeclaredLost(&provision.MoveState{ID: "mv_abcdefghijklmnopqrstuvwxyz", Incarnation: "i", From: "ie", To: "uk", State: state}) |
| + if ok != want || ok && (e.Event != "declared_lost" || e.Identity != DeclaredLostIdentity) { |
| + t.Errorf("%s: %+v %v, want %v", state, e, ok, want) |
| + } |
| + } |
| +} |