-
Notifications
You must be signed in to change notification settings - Fork 9
OOR client 2/4: outgoing/incoming FSM + actor wrapper #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
999cb51
904688d
b01a74c
b5377d2
0a35ed3
35e5650
23dfdae
fce3f1c
b98398d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,304 @@ | ||
| package oor | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "fmt" | ||
| "sort" | ||
|
|
||
| "github.com/btcsuite/btcd/btcutil" | ||
| "github.com/btcsuite/btcd/btcutil/psbt" | ||
| "github.com/btcsuite/btcd/chaincfg/chainhash" | ||
| "github.com/btcsuite/btcd/txscript" | ||
| "github.com/btcsuite/btcd/wire" | ||
| "github.com/lightninglabs/darepo-client/lib/scripts" | ||
| ) | ||
|
|
||
| // CheckpointInput describes the VTXO input being transformed into a checkpoint | ||
| // output for an OOR transfer. | ||
| type CheckpointInput struct { | ||
| // Outpoint is the outpoint of the VTXO output being spent. | ||
| Outpoint wire.OutPoint | ||
|
|
||
| // WitnessUtxo is the previous output being spent (value + pkScript). | ||
| // | ||
| // This must match the server's stored VTXO descriptor later, but at the | ||
| // primitive level we only need it so PSBT has enough material to be | ||
| // signed and validated structurally. | ||
| WitnessUtxo *wire.TxOut | ||
|
|
||
| // OwnerLeafScript is the VTXO-owner collaborative leaf script. | ||
| // | ||
| // "Owner" here means owner of the spent VTXO input, not owner of the | ||
| // checkpoint CSV timeout path. | ||
| // | ||
| // The script should be committed to in the checkpoint output tap tree. | ||
| // | ||
| // This is deliberately a raw script for the draft implementation. Once | ||
| // the closure system is canonical, higher layers should construct this | ||
| // leaf using closure helpers and pass the resulting script bytes here. | ||
| OwnerLeafScript []byte | ||
|
bhandras marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // CheckpointResult is the result of building a checkpoint PSBT. | ||
| type CheckpointResult struct { | ||
| // PSBT is the unsigned checkpoint transaction. | ||
| PSBT *psbt.Packet | ||
|
|
||
| // TapTreeEncoded is the v0 tap tree encoding for the checkpoint output. | ||
| // | ||
| // This is intended to be attached to the Ark tx PSBT inputs under the | ||
| // `taptree` unknown key so finalization can later copy it onto the | ||
| // checkpoint output metadata. | ||
|
Comment on lines
+47
to
+51
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar question on previous PR: when is it encoded in PSBT vs explicitly communicated like this? perhaps worth keeping explicit? 🤷♀️ if not, then perhaps worth having a type that hides the data but that returns a result that defs always has it encoded? then have methods to extract taptree on that type?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now we keep this explicit across the boundary for clarity; once wire types stabilize we can compress payload shape.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated with latest changes: we no longer expose a CheckpointOutput() helper. Callers build typed CheckpointOutput directly from Result{PSBT, TapTreeEncoded}, keeping the primitive API flat while preserving metadata.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i dont get it. the argument on the previous PR was to keep spending metadata in the PSBT but to make session data explicit. not clear to me 🤷♀️
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. personally i think there should be a new type that wraps the two to make it clear that the psbt does or doesnt include the data.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree and implemented: we now return a wrapper artifact (CheckpointArtifact) that carries {PSBT, TapTreeEncoded} explicitly so it's clear when metadata is sidecar vs embedded.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. almost there: the agent is trying to remain backwards compatible but doenst need to:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call — removed. Done in e32d939
|
||
| TapTreeEncoded []byte | ||
| } | ||
|
|
||
| // RecipientOutput describes an Ark tx recipient output. | ||
| type RecipientOutput struct { | ||
| // PkScript is the destination script. | ||
| PkScript []byte | ||
|
|
||
| // Value is the amount to send in satoshis. | ||
| Value btcutil.Amount | ||
| } | ||
|
Comment on lines
+56
to
+62
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why cant we use wire.Txout?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also fixed: we now use wire.TxOut in the checkpoint/spent-vtxo shapes (checkpoint.SpentVTXORef.Output, CheckpointOutput.Output). |
||
|
|
||
| // BuildCheckpointPSBT constructs an unsigned checkpoint PSBT that spends a VTXO | ||
| // input and pays the entire input value to a checkpoint P2TR output. | ||
| // | ||
| // The checkpoint output pkScript is derived deterministically from: | ||
| // | ||
| // - the operator checkpoint policy, and | ||
| // - the caller-provided VTXO-owner collaborative leaf script. | ||
| // | ||
| // This function does not attempt to sign the checkpoint tx. It also does not | ||
| // validate that the owner leaf is a canonical Ark closure (draft phase). | ||
| func BuildCheckpointPSBT(policy scripts.CheckpointPolicy, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. btw do we check somewher that the policy value used by the operator for the checkpoint's CSV is reasonable? we dont want it to be too short. ie, we should have a min acceptable value for this. also: the server should check that the user has used its advertised values. just want to make sure we are doing that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good call. we now enforce a minimum csv delay in checkpoint builder (checkpoint.MinCheckpointCSVDelay). server-side 'matches advertised policy' enforcement is still a follow-up in server OOR hardening.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. has that TODO been tracked?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, tracked here: https://github.com/lightninglabs/darepo/issues/91 That issue covers the server-side follow-up to enforce that submitted checkpoint policy values match what the server advertises. |
||
| in CheckpointInput) (*CheckpointResult, error) { | ||
|
|
||
| switch { | ||
| case in.WitnessUtxo == nil: | ||
| return nil, fmt.Errorf("witness utxo must be provided") | ||
|
|
||
| case in.WitnessUtxo.Value <= 0: | ||
| return nil, fmt.Errorf("witness utxo value must be " + | ||
| "positive") | ||
|
|
||
| case len(in.WitnessUtxo.PkScript) == 0: | ||
| return nil, fmt.Errorf("witness utxo pkScript must be " + | ||
| "provided") | ||
| } | ||
|
|
||
| tapscript, err := scripts.CheckpointTapScript( | ||
| policy, in.OwnerLeafScript, | ||
| ) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| encodedTapTree, err := EncodeTapTree(tapLeafScripts(tapscript.Leaves)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| checkpointPkScript, err := scripts.CheckpointPkScript( | ||
| policy, in.OwnerLeafScript, | ||
| ) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Use v3 to be compatible with package relay policies (TRUC-style | ||
| // constraints) when these txs are eventually submitted as a package. | ||
| tx := wire.NewMsgTx(3) | ||
| tx.AddTxIn(&wire.TxIn{ | ||
| PreviousOutPoint: in.Outpoint, | ||
| Sequence: wire.MaxTxInSequenceNum, | ||
| }) | ||
| tx.AddTxOut(&wire.TxOut{ | ||
| Value: in.WitnessUtxo.Value, | ||
| PkScript: checkpointPkScript, | ||
| }) | ||
|
|
||
| pkt, err := psbt.NewFromUnsignedTx(tx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to create checkpoint psbt: %w", | ||
| err) | ||
| } | ||
|
|
||
| pkt.Inputs[0].WitnessUtxo = in.WitnessUtxo | ||
|
|
||
| return &CheckpointResult{ | ||
| PSBT: pkt, | ||
| TapTreeEncoded: encodedTapTree, | ||
| }, nil | ||
| } | ||
|
|
||
| // CheckpointOutput describes a checkpoint output that will be spent by the Ark | ||
| // transaction. | ||
| type CheckpointOutput struct { | ||
| // Txid is the txid of the checkpoint transaction. | ||
| Txid chainhash.Hash | ||
|
|
||
| // Output is the checkpoint output being spent (value + pkScript). | ||
| Output *wire.TxOut | ||
|
|
||
| // TapTreeEncoded is the v0 tap tree encoding for the checkpoint output. | ||
| TapTreeEncoded []byte | ||
| } | ||
|
|
||
| // BuildArkPSBT constructs a deterministic Ark tx PSBT spending the set of | ||
| // checkpoint outputs and producing the requested recipient outputs plus an | ||
| // anchor output. | ||
| // | ||
| // This is a v0 builder and enforces: | ||
| // | ||
| // - fee-less transfers (sum(inputs) == sum(outputs excluding anchor)), | ||
| // - anchor output is last output (P2A, value 0), and | ||
| // - canonical ordering rules for inputs/outputs (BIP69), | ||
| // | ||
| // It also attaches per-input `taptree` metadata using TapTreePSBTKey so the | ||
| // finalize step can later bind tap tree data onto checkpoint PSBT outputs. | ||
| func BuildArkPSBT(checkpoints []CheckpointOutput, | ||
|
ellemouton marked this conversation as resolved.
|
||
| recipients []RecipientOutput) (*psbt.Packet, error) { | ||
|
|
||
| if len(checkpoints) == 0 { | ||
| return nil, fmt.Errorf("checkpoint outputs must be provided") | ||
| } | ||
|
|
||
| if len(recipients) == 0 { | ||
| return nil, fmt.Errorf("recipient outputs must be provided") | ||
| } | ||
|
|
||
| var sumInputs btcutil.Amount | ||
| for _, cp := range checkpoints { | ||
| if cp.Output == nil { | ||
| return nil, fmt.Errorf( | ||
| "checkpoint output must be provided", | ||
| ) | ||
| } | ||
|
|
||
| if len(cp.Output.PkScript) == 0 { | ||
| return nil, fmt.Errorf("checkpoint pkScript must be " + | ||
| "provided") | ||
| } | ||
|
|
||
| if cp.Output.Value <= 0 { | ||
| return nil, fmt.Errorf("checkpoint output value must " + | ||
| "be positive") | ||
| } | ||
|
|
||
| sumInputs += btcutil.Amount(cp.Output.Value) | ||
| } | ||
|
|
||
| var sumOutputs btcutil.Amount | ||
| for _, out := range recipients { | ||
| if len(out.PkScript) == 0 { | ||
| return nil, fmt.Errorf("recipient pkScript must be " + | ||
| "provided") | ||
| } | ||
|
|
||
| if out.Value <= 0 { | ||
| return nil, fmt.Errorf("recipient value must be " + | ||
| "positive") | ||
| } | ||
|
|
||
| sumOutputs += out.Value | ||
| } | ||
|
|
||
| if sumInputs != sumOutputs { | ||
| return nil, fmt.Errorf("fee-less ark tx requires equal " + | ||
| "input/output sums") | ||
| } | ||
|
|
||
| // Sort checkpoint inputs by outpoint (BIP69-style) to ensure | ||
| // deterministic input order. | ||
| checkpointsSorted := make([]CheckpointOutput, len(checkpoints)) | ||
| copy(checkpointsSorted, checkpoints) | ||
| sort.SliceStable(checkpointsSorted, func(i, j int) bool { | ||
| a := checkpointsSorted[i] | ||
| b := checkpointsSorted[j] | ||
|
|
||
| cmp := bytes.Compare(a.Txid[:], b.Txid[:]) | ||
| if cmp != 0 { | ||
| return cmp < 0 | ||
| } | ||
|
|
||
| // v0 always spends vout=0. | ||
| return false | ||
| }) | ||
|
|
||
| recipientOuts := make([]RecipientOutput, len(recipients)) | ||
| copy(recipientOuts, recipients) | ||
| sort.SliceStable(recipientOuts, func(i, j int) bool { | ||
| a := recipientOuts[i] | ||
| b := recipientOuts[j] | ||
|
|
||
| if a.Value != b.Value { | ||
| return a.Value < b.Value | ||
| } | ||
|
|
||
| return bytes.Compare(a.PkScript, b.PkScript) < 0 | ||
| }) | ||
|
|
||
| // Use v3 to be compatible with package relay policies (TRUC-style | ||
| // constraints) when this tx is submitted as part of a package. | ||
| tx := wire.NewMsgTx(3) | ||
| for _, cp := range checkpointsSorted { | ||
| tx.AddTxIn(&wire.TxIn{ | ||
| PreviousOutPoint: wire.OutPoint{ | ||
| Hash: cp.Txid, | ||
| Index: 0, | ||
| }, | ||
| Sequence: wire.MaxTxInSequenceNum, | ||
| }) | ||
| } | ||
|
|
||
| for _, out := range recipientOuts { | ||
| tx.AddTxOut(&wire.TxOut{ | ||
| Value: int64(out.Value), | ||
| PkScript: out.PkScript, | ||
| }) | ||
| } | ||
|
|
||
| tx.AddTxOut(scripts.AnchorOutput()) | ||
|
|
||
| err := ValidateCanonicalArkTx(tx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("internal: built ark tx is not "+ | ||
| "canonical: %w", err) | ||
| } | ||
|
|
||
| pkt, err := psbt.NewFromUnsignedTx(tx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to create ark psbt: %w", err) | ||
| } | ||
|
|
||
| // Attach witness UTXOs and tap tree metadata in the same order as | ||
| // inputs. | ||
| for i := range checkpointsSorted { | ||
| cp := checkpointsSorted[i] | ||
|
|
||
| pkt.Inputs[i].WitnessUtxo = cp.Output | ||
|
|
||
| if len(cp.TapTreeEncoded) == 0 { | ||
| return nil, fmt.Errorf("checkpoint tap tree must be " + | ||
| "provided") | ||
| } | ||
|
|
||
| err := PutTapTreePSBTInput(pkt, i, cp.TapTreeEncoded) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
| return pkt, nil | ||
| } | ||
|
|
||
| // tapLeafScripts extracts raw script bytes from a list of tap leaves. | ||
| func tapLeafScripts(leaves []txscript.TapLeaf) [][]byte { | ||
| scripts := make([][]byte, 0, len(leaves)) | ||
| for _, leaf := range leaves { | ||
| scripts = append(scripts, leaf.Script) | ||
| } | ||
|
|
||
| return scripts | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package oor | ||
|
|
||
| import ( | ||
| "crypto/rand" | ||
| "testing" | ||
|
|
||
| "github.com/btcsuite/btcd/btcec/v2" | ||
| "github.com/btcsuite/btcd/btcutil/psbt" | ||
| "github.com/btcsuite/btcd/chaincfg/chainhash" | ||
| "github.com/btcsuite/btcd/txscript" | ||
| "github.com/btcsuite/btcd/wire" | ||
| "github.com/lightninglabs/darepo-client/lib/scripts" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // randomP2TRScript returns a P2TR pkScript with a random key. | ||
| func randomP2TRScript(t *testing.T) []byte { | ||
| t.Helper() | ||
|
|
||
| var key [32]byte | ||
| _, err := rand.Read(key[:]) | ||
| require.NoError(t, err) | ||
|
|
||
| return append([]byte{txscript.OP_1, 0x20}, key[:]...) | ||
| } | ||
|
|
||
| // TestBuildCheckpointAndArkPSBT asserts the builders produce a submit package | ||
| // that passes the shared submit validator. | ||
| func TestBuildCheckpointAndArkPSBT(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| // This is an integration-style unit test over the tx builder layer: | ||
| // - BuildCheckpointPSBT produces a checkpoint spend for a single VTXO. | ||
| // - BuildArkPSBT consumes the checkpoint output and adds recipients + | ||
| // anchor output. | ||
| // - ValidateSubmitPackage then enforces the shared structural rules. | ||
| operatorKey, err := btcec.NewPrivateKey() | ||
| require.NoError(t, err) | ||
|
|
||
| policy := scripts.CheckpointPolicy{ | ||
| OperatorKey: operatorKey.PubKey(), | ||
| CSVDelay: 10, | ||
| } | ||
|
|
||
| vtxoWitness := &wire.TxOut{ | ||
| Value: 5000, | ||
| PkScript: randomP2TRScript(t), | ||
| } | ||
|
|
||
| ownerLeafScript := []byte{ | ||
| txscript.OP_1, | ||
| txscript.OP_1, | ||
| txscript.OP_ADD, | ||
| txscript.OP_2, | ||
| txscript.OP_EQUAL, | ||
| } | ||
|
|
||
| cpResult, err := BuildCheckpointPSBT(policy, CheckpointInput{ | ||
| Outpoint: wire.OutPoint{ | ||
| Hash: chainhash.Hash{1}, | ||
| Index: 0, | ||
| }, | ||
| WitnessUtxo: vtxoWitness, | ||
| OwnerLeafScript: ownerLeafScript, | ||
| }) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, cpResult) | ||
|
|
||
| checkpointTx := cpResult.PSBT.UnsignedTx | ||
| require.NotNil(t, checkpointTx) | ||
| require.Len(t, checkpointTx.TxOut, 1) | ||
|
|
||
| arkPsbt, err := BuildArkPSBT([]CheckpointOutput{ | ||
| { | ||
| Txid: checkpointTx.TxHash(), | ||
| Output: checkpointTx.TxOut[0], | ||
| TapTreeEncoded: cpResult.TapTreeEncoded, | ||
| }, | ||
| }, []RecipientOutput{ | ||
| { | ||
| PkScript: randomP2TRScript(t), | ||
| Value: 5000, | ||
| }, | ||
| }) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, arkPsbt) | ||
|
|
||
| _, err = ValidateSubmitPackage(arkPsbt, []*psbt.Packet{cpResult.PSBT}) | ||
| require.NoError(t, err) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we change the name a bit since ideally this never becomes a Utxo?
also wondering if we should like group the wire.outpoint & wire.TxOut together to represent the VTXO we are spending?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated after latest force-push: we dropped the SpentVTXO wrapper and now reuse checkpoint.Input directly (outpoint + witness txout), so OOR uses one canonical checkpoint input shape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure this answers my question. all that has been done is that the type was moved and is now aliased here. but my question still stands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair. to address this, the canonical checkpoint shape is now checkpoint.Input with SpentVTXORef{Outpoint, Output} so identity+witness material stay grouped in one type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okkkk doesnt look addressed in this pr but looks like it is later on somwhere