round: Round-scoped VTXO-tree + connector signing keys - #698
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces per-round operator signing keys (TreeCosignKey and ConnectorOperatorKey) to the client state machine and protobuf definitions. These keys allow the client to validate VTXO trees and reconstruct connector trees using round-specific keys instead of the global operator key, ensuring compatibility during operator key rotations. Fallbacks to the global operator key are maintained for older servers. There are no review comments, so I have no feedback to provide.
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.
|
@claude review this |
|
Claude finished @ellemouton's task in 3m 16s —— View job Review — Round-scoped VTXO-tree + connector signing keys
Overall this is a clean, well-scoped change. The proto additions are backward-compatible (new optional fields), the fallback to ✅ Design verification — downstream signing is correctly handledMy main concern on first read was that It doesn't, and the change is correct as written: the nonce/partial-sig sessions are built from the extracted 🔐 Security model — worth a one-line note in the invariant docThis is the crux of the change and I believe it's sound, but it's subtle enough to be worth documenting explicitly. The client previously validated the VTXO tree against What makes it safe is the property called out in the PR description: the leaf spending policy still commits to the global identity key, which the client derives itself via Suggestion: add a sentence to the connector-ancestry invariant in 🧪 Test coverage gap (suggestion)The new A focused FSM test that threads a non-nil round key through and asserts validation uses it (e.g. validation fails when the tree is keyed to a different key than Minor nits
Note: I was unable to run |
117b818 to
41c1398
Compare
| // change the tree key the client must agree on. Nil when talking to a | ||
| // server that predates this field; callers fall back to the global | ||
| // operator key. | ||
| TreeCosignKey *btcec.PublicKey |
There was a problem hiding this comment.
Do we need a 3rd key here? So the vtxo sign key? or the tree sign key is the same for all users in a round?
Add two fields to ClientBatchInfo so the server can hand each round's
signing keys to clients at round time instead of clients assuming the
operator's global GetInfo key:
- tree_cosign_key: the round's VTXO-tree MuSig2 cosigner key, derived
fresh per round and independent of the operator identity key.
- connector_operator_key: the operator key the round used to build its
connector tree.
Delivering both per round lets a client that is still on a previous
operator-key epoch agree with the server on the tree-signing key and
reconstruct the same connector outpoints, so an operator key rotation
no longer breaks in-flight round participation.
Generated code regenerated from the proto change.
The client validated the VTXO tree and reconstructed the connector tree against the operator's global GetInfo key (env.OperatorTerms.PubKey). That breaks the moment the operator rotates its key: a client still on the previous key would verify the tree against the wrong cosigner and rebuild connectors from the wrong key, so it could never complete the round. Carry the per-round tree-cosign key and connector operator key from ClientBatchInfo through CommitmentTxReceivedState and use them for tree ValidatePath and connector-ancestry reconstruction. Fall back to the global operator key when the field is absent (older server), so the change is wire-compatible. The leaf policy template still commits to the global identity key — only the tree cosigner and connector reconstruction move to the round-delivered keys.
96f6cb4 to
33e433c
Compare
The client rebuilt a round's VTXO tree with a per-round cosign key
(delivered in ClientBatchInfo) but a GLOBAL sweep key and delay read
from the operator's GetInfo terms. That asymmetry breaks the moment the
operator rotates its key: a client still on a previous epoch would
reconstruct the sweep branch and compute batch expiry from stale global
values that no longer match the round it joined.
Move the sweep key and delay into the per-round batch info so each round
is fully self-describing for VTXO-tree reconstruction, mirroring how the
tree cosign key and connector operator key are already delivered:
- Add sweep_key (field 7) and sweep_delay (field 8) to
round.v1.ClientBatchInfo.
- Carry them through CommitmentTxBuilt and CommitmentTxReceivedState,
threading sweep_delay through the signing-ceremony states so batch
expiry is computed from the round's value on confirmation.
- Move the sweep-vs-exit-delay security check out of round-actor
construction (which no longer has a global sweep delay) into the
per-round CommitmentTxReceivedState transition.
Remove the now-defunct global sweep terms (flag-day, no backwards
compat): drop sweep_key/sweep_delay from arkrpc GetInfoResponse and the
daemonrpc ServerInfo snapshot (both reserved to prevent reuse), and
remove SweepKey/SweepDelay from lib/types.OperatorTerms, the
GetInfo->OperatorTerms parsing, and the sdk/ark ServerInfo model.
Generated code regenerated from the proto changes.
Today the forfeit-tx penalty output pays to a BIP-86 key-spend of the
operator identity key, advertised globally as forfeit_script via the
operator GetInfo terms. That couples the penalty script to the
operator's connector/identity key and breaks the moment the operator
rotates its key: a client still on a previous epoch would build the
penalty output for the round it joined from a stale global script that
no longer matches the server.
Deliver the operator's dedicated per-round forfeit penalty key in the
batch info so each round is self-describing for forfeit-tx
reconstruction, mirroring how the tree cosign, connector, and sweep keys
are already delivered:
- Add forfeit_key (field 9) to round.v1.ClientBatchInfo.
- Carry it through CommitmentTxBuilt and the signing-ceremony states;
derive the penalty script as PayToTaproot(ComputeTaprootKeyNoScript(
forfeitKey)) at the forfeit-tx build and validate sites instead of
reading the removed global forfeit script.
Remove the now-defunct global forfeit term (flag-day, no backwards
compat): drop forfeit_script from arkrpc GetInfoResponse and the
daemonrpc ServerInfo snapshot (both reserved to prevent reuse), and
remove ForfeitScript from lib/types.OperatorTerms, the
GetInfo->OperatorTerms parsing, and the sdk/ark ServerInfo model.
Generated code regenerated from the proto changes.
33e433c to
5bdf260
Compare
3 participants
Part of the operator-key-rotation epic (lightninglabs/darepo#525). Pairs
with the server PR lightninglabs/darepo#533.
What
Stops the client from assuming the operator's global
GetInfokey whenparticipating in a round. The server now derives a fresh, per-round
ephemeral VTXO-tree MuSig2 signing key (independent of its persistent
identity key) and hands it — plus the operator key its connector tree was
built with — to the client in
ClientBatchInfo. This lets a client thatis still on a previous operator-key epoch agree with the server on the
tree-signing key and reconstruct the same connector outpoints, so an
operator key rotation no longer breaks in-flight round participation.
Changes
roundpb: addtree_cosign_key+connector_operator_keytoClientBatchInfo(proto + regenerated stubs).round: carry both keys fromCommitmentTxBuiltthroughCommitmentTxReceivedStateand use them for VTXO-treeValidatePathand connector-ancestry reconstruction instead of
env.OperatorTerms.PubKey. Fall back to the global operator key when afield is absent (older server), so the change is wire-compatible.
The leaf policy template still commits to the global identity key — only
the tree cosigner and connector reconstruction move to the round-delivered
keys.
Testing
roundunit tests pass, including a newFromPrototest covering keyparsing, the absent-field fallback, and malformed-key rejection. Verified
end-to-end against the server change via systests (boarding, refresh,
leave) and the
TestLeaveAfterOperatorKeyRotationitest.🤖 Generated with Claude Code