oor: Bind OOR checkpoint outputs to the session operator key

[ellemouton]

What

When building an OOR transfer, the client derived the checkpoint output's owner collaborative leaf from the spent input VTXO's operator key. But the checkpoint output — and the Ark tx that spends it cooperatively — is governed by the session checkpoint policy, whose operator key is the operator's current key at session-creation time.

After the operator rotates its key, a VTXO created under an older key (X) would produce a checkpoint output committed to X, while the server (correctly) rebuilds and co-signs that output under the current/session key (Y). The post-rotation submit was rejected at AwaitingSubmitValidationState with "owner leaf policy does not contain operator key", before the server's per-input co-sign path even ran — so any OOR spend of a pre-rotation VTXO failed. Before a rotation X == Y, so this was invisible.

Fix

Rebind each standard input's checkpoint-output owner leaf to the session operator key (StartTransferEvent.Policy.OperatorKey) in the StartTransfer transition, before the deterministic build and before the inputs flow into signing/persistence (normalizeCheckpointOwnerLeaves).

The input spend path is untouched — spending the VTXO still uses its own tapscript, committed to the historical key, which the server resolves and co-signs per input. Custom spends (e.g. vHTLC) carry their own owner leaf and are skipped.

This is the client half of the operator-key-rotation OOR work. The server half (per-input historical co-sign + session-bound checkpoint policy persistence + indexer surfacing) is in darepo (stacked on #525 work).

Testing

• New unit tests (oor/checkpoint_owner_leaf_test.go): owner leaf rebinds to the session key (and not the input's historical key); custom spends untouched; nil policy key rejected.
• End-to-end (server repo itests, now un-skipped): TestOORSendAfterOperatorKeyRotation (single rotated-key OOR send) and TestOORSendMixedHistoricalOperatorKeys (one transfer spending inputs under two different historical keys) both pass.

Part of the operator-key-rotation epic (#525).

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a mechanism to normalize checkpoint owner leaves to the session operator key rather than the spent input VTXO's historical operator key, preventing submission failures after operator key rotation. This logic is integrated into the StartTransferEvent processing, and comprehensive unit tests have been added. The feedback suggests a minor refactoring in the loop of normalizeCheckpointOwnerLeaves to use a pointer to the slice elements for better readability and idiomatic Go code.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[ellemouton]

@claude review this

[claude]

Claude finished @ellemouton's task in 2m 19s —— View job

---

Review: oor — Bind OOR checkpoint outputs to the session operator key

• Read the changed files (oor/transitions.go, oor/checkpoint_owner_leaf_test.go)
• Understand the surrounding StartTransfer transition + helper context
• Analyze correctness, edge cases, style, and the gemini suggestion
• Post review feedback

---

Overall this is a well-targeted, well-documented fix. The core insight — that the checkpoint output owner leaf is governed by the session policy while the input spend stays on the historical key — is correct, and the placement of the normalization (before buildSubmitPackage, before the inputs flow into signing/persistence) is exactly right. The doc comment on normalizeCheckpointOwnerLeaves is excellent.

I traced the fix through the downstream TransferInput.Validate() path to confirm it sticks, and it does (details below). I was not able to run the unit tests in this environment (the go test invocation needs an approval I don't have), so the notes below are from reading.

Correctness — the fix composes cleanly ✅

The one thing worth confirming is that the later TransferInput.Validate() in oor/transfer_inputs.go:108-161 doesn't clobber or reject the rebound leaf. It doesn't:

• Validate derives defaultLeaf/defaultPolicy from the historical i.VTXO.OperatorKey, but only assigns them when the fields are empty. After normalizeCheckpointOwnerLeaves, both OwnerLeafScript and OwnerLeafPolicy are populated, so the auto-derive branches (lines 122-130, 138-139, 150-157) are skipped.
• The consistency check at oor/transfer_inputs.go:141-145 recompiles from OwnerLeafPolicy (the session policy) and compares to OwnerLeafScript (the session leaf). Both come from the same defaultOwnerLeaf(clientKey, sessionKey) call, so they match — no "owner leaf script and policy mismatch" error.
• VTXOPolicyTemplate (the spend path) is left untouched, so the spend stays committed to the historical key. This matches the PR's stated invariant. 👍

Minor: unreachable len(leaf) == 0 branch

leaf, leafPolicy, err := defaultOwnerLeaf(
    inputs[i].VTXO.ClientKey.PubKey, policy.OperatorKey,
)
if err != nil {
    return err
}
if len(leaf) == 0 {
    continue
}

defaultOwnerLeaf only returns an empty leaf (with a nil error) when one of its key args is nil (oor/transfer_inputs.go:373-375). But this loop already guards policy.OperatorKey != nil (top of the function) and inputs[i].VTXO.ClientKey.PubKey != nil (the continue above), so len(leaf) == 0 is effectively unreachable here. It's harmless defensive code, but if you want to keep it, a one-line comment noting it's a belt-and-suspenders guard would help a future reader who'd otherwise wonder when it fires.

Style nit (agree with gemini) — input := &inputs[i]

The reviewer's suggestion to take input := &inputs[i] at the top of the loop body is idiomatic and removes five repetitions of the index expression. Worth applying. Fix this →

Questions / things to confirm (not blockers)

1. In-place mutation of evt.VTXOInputs. normalizeCheckpointOwnerLeaves mutates the event's slice elements in place. The StartTransfer transition already mutates these inputs via buildSubmitPackage, so this is consistent with existing behavior, and because the rebind is deterministic (always the session key) a re-processed/retried event is a no-op. Just flagging it's intentional.

2. Restore/resume of pre-fix sessions. A session created before this fix and restored from a persisted snapshot will carry the old historical-key owner leaf in its TransferInputSnapshot, and the restore path reconstructs from the snapshot rather than re-running this normalization. Given deterministic-resume requires byte-identical PSBTs, I assume that's intentional and such sessions are expected to fail/be drained pre-rotation — but worth a sentence in the PR or a follow-up note confirming there's no in-flight-session migration concern.

3. Test coverage gap (minor). The three new tests cover the happy path, custom-spend skip, and nil-policy-key rejection nicely. Two small additions would round it out:

   • an assertion that the spend side (VTXOPolicyTemplate / the resolved spend path) still commits to the historical key after normalization — this is the other half of the invariant and is currently only verified server-side;
   • a case exercising the VTXO == nil / ClientKey.PubKey == nil skip branch.

   These are nice-to-haves; the existing coverage plus the un-skipped server itests (TestOORSendAfterOperatorKeyRotation, TestOORSendMixedHistoricalOperatorKeys) give good confidence.

Nits

• Per the repo guideline, please run make fmt-changed / make lint-changed-local before merge if not already done — the diff looks compliant (8-space tabs, 80-col, name-prefixed doc comments) from here.

Nothing here is blocking; the logic is sound and the design rationale is clearly captured. 👍
· ellemouton/oor-rotation-keys

[ellemouton]

Thanks for the review! Addressed in the amended commit:

• input := &inputs[i] — applied; the loop body no longer repeats the index expression.
• Unreachable len(leaf) == 0 — kept as a belt-and-suspenders guard with a comment noting both key args are non-nil here, so it can't actually fire.
• Test coverage — added an assertion that the spend side (VTXO.OperatorKey, from which the historical collaborative spend path derives) is untouched after normalization, and a TestNormalizeCheckpointOwnerLeavesSkipsIncompleteInput case for the VTXO == nil / ClientKey.PubKey == nil skip branch.

Re: resume of pre-fix sessions (Q2) — no in-flight migration concern. A session created before this change was necessarily created when the input and current operator keys were equal (no rotation relevant to it had occurred yet), and its point-of-no-return co-sign happened under that key. Deterministic resume reconstructs byte-identically under that same key, so it completes consistently. This fix only changes how new sessions (created after the upgrade) bind the checkpoint-output owner leaf — it never re-derives an already-bound session's leaf.

The end-to-end rotation itests on the server side (TestOORSendAfterOperatorKeyRotation, TestOORSendMixedHistoricalOperatorKeys) pass against this branch.

[Roasbeef]

LGTM 🫐