From df32d2299ba6cb850a9717c5dc56de7502deb345 Mon Sep 17 00:00:00 2001 From: Kukks Date: Wed, 2 Sep 2026 09:07:38 +0200 Subject: [PATCH 1/4] fix: enforce sighash policy on counterparty psbts --- pkg/client-lib/offchain-tx/sighash_test.go | 146 +++++++++++++++++++++ pkg/client-lib/offchain-tx/utils.go | 53 +++++++- 2 files changed, 198 insertions(+), 1 deletion(-) create mode 100644 pkg/client-lib/offchain-tx/sighash_test.go diff --git a/pkg/client-lib/offchain-tx/sighash_test.go b/pkg/client-lib/offchain-tx/sighash_test.go new file mode 100644 index 000000000..495e23abb --- /dev/null +++ b/pkg/client-lib/offchain-tx/sighash_test.go @@ -0,0 +1,146 @@ +package offchaintx + +import ( + "context" + "fmt" + "strings" + "testing" + + clientlib "github.com/arkade-os/arkd/pkg/client-lib" + "github.com/btcsuite/btcd/psbt/v2" + "github.com/btcsuite/btcd/txscript/v2" + "github.com/stretchr/testify/require" +) + +// forbiddenSighashTypes are the types a malicious counterparty would declare to +// obtain a signature that doesn't commit to our outputs. +var forbiddenSighashTypes = []txscript.SigHashType{ + txscript.SigHashNone, + txscript.SigHashSingle, + txscript.SigHashNone | txscript.SigHashAnyOneCanPay, + txscript.SigHashSingle | txscript.SigHashAnyOneCanPay, +} + +// withSighashType re-encodes the given base64 psbt with the sighash type set on +// every input, mimicking a counterparty declaring it in its response. +func withSighashType(t *testing.T, tx string, sighashType txscript.SigHashType) string { + t.Helper() + ptx, err := psbt.NewFromRawBytes(strings.NewReader(tx), true) + require.NoError(t, err) + + for i := range ptx.Inputs { + ptx.Inputs[i].SighashType = sighashType + } + + encoded, err := ptx.B64Encode() + require.NoError(t, err) + return encoded +} + +// recordingClient records the txs handed to the signer so a test can assert a +// counterparty psbt never reached it. +type recordingClient struct { + clientlib.Client + signed []string + finalized bool +} + +func (c *recordingClient) FinalizeTx(_ context.Context, _ string, _ []string) error { + c.finalized = true + return nil +} + +func (c *recordingClient) signTx(_ context.Context, tx string) (string, error) { + c.signed = append(c.signed, tx) + return tx, nil +} + +func TestVerifySignedTxSighashPolicy(t *testing.T) { + txid := "1111111111111111111111111111111111111111111111111111111111111111" + original := newTestVerifyPSBT(t, txid, 0, true, true) + signers := getSigners(t) + + t.Run("forbidden", func(t *testing.T) { + for _, sighashType := range forbiddenSighashTypes { + t.Run(fmt.Sprintf("%#x", uint32(sighashType)), func(t *testing.T) { + signed := withSighashType(t, original, sighashType) + + err := VerifySignedTx(original, signed, signers) + require.ErrorContains(t, err, "forbidden sighash type") + }) + } + }) + + // SIGHASH_ALL is allowed by the policy but is not what we built, so a + // counterparty must not be able to swap it in either. + t.Run("mismatch", func(t *testing.T) { + signed := withSighashType(t, original, txscript.SigHashAll) + + err := VerifySignedTx(original, signed, signers) + require.ErrorContains(t, err, "sighash type mismatch") + }) + + // The honest counterparty leaves the sighash type untouched: the policy must + // let it through and fail later, on the missing signature. + t.Run("allowed", func(t *testing.T) { + signed := newTestVerifyPSBT(t, txid, 0, true, true) + + err := VerifySignedTx(original, signed, signers) + require.ErrorContains(t, err, "signer signature not found") + }) +} + +func TestVerifySignedCheckpointTxsSighashPolicy(t *testing.T) { + txid := "1111111111111111111111111111111111111111111111111111111111111111" + original := newTestVerifyPSBT(t, txid, 0, true, true) + signers := getSigners(t) + + for _, sighashType := range forbiddenSighashTypes { + t.Run(fmt.Sprintf("%#x", uint32(sighashType)), func(t *testing.T) { + signed := withSighashType(t, original, sighashType) + + err := VerifySignedCheckpointTxs([]string{original}, []string{signed}, signers) + require.ErrorContains(t, err, "forbidden sighash type") + }) + } +} + +func TestFinalizeTxSighashPolicy(t *testing.T) { + txid := "1111111111111111111111111111111111111111111111111111111111111111" + checkpoint := newTestVerifyPSBT(t, txid, 0, true, true) + + t.Run("forbidden", func(t *testing.T) { + for _, sighashType := range forbiddenSighashTypes { + t.Run(fmt.Sprintf("%#x", uint32(sighashType)), func(t *testing.T) { + client := &recordingClient{} + + _, _, err := finalizeTx( + context.Background(), client, client.signTx, + clientlib.AcceptedOffchainTx{ + Txid: txid, + SignedCheckpointTxs: []string{withSighashType(t, checkpoint, sighashType)}, + }, + ) + require.ErrorContains(t, err, "forbidden sighash type") + require.Empty(t, client.signed) + require.False(t, client.finalized) + }) + } + }) + + t.Run("allowed", func(t *testing.T) { + client := &recordingClient{} + + _, finalCheckpoints, err := finalizeTx( + context.Background(), client, client.signTx, + clientlib.AcceptedOffchainTx{ + Txid: txid, + SignedCheckpointTxs: []string{checkpoint}, + }, + ) + require.NoError(t, err) + require.Equal(t, []string{checkpoint}, finalCheckpoints) + require.Equal(t, []string{checkpoint}, client.signed) + require.True(t, client.finalized) + }) +} diff --git a/pkg/client-lib/offchain-tx/utils.go b/pkg/client-lib/offchain-tx/utils.go index a839bfbb8..1910e7fac 100644 --- a/pkg/client-lib/offchain-tx/utils.go +++ b/pkg/client-lib/offchain-tx/utils.go @@ -5,6 +5,7 @@ import ( "context" "encoding/hex" "fmt" + "strings" arklib "github.com/arkade-os/arkd/pkg/ark-lib" "github.com/arkade-os/arkd/pkg/ark-lib/asset" @@ -266,6 +267,36 @@ func addExtension( return nil } +// checkSighashType rejects the sighash types the honest stack never produces. +// SIGHASH_NONE and SIGHASH_SINGLE don't commit to our outputs, so a signature +// made under them can be replayed on a tx paying the counterparty instead. +func checkSighashType(sighashType txscript.SigHashType) error { + switch sighashType { + case txscript.SigHashDefault, txscript.SigHashAll, + txscript.SigHashAll | txscript.SigHashAnyOneCanPay: + return nil + default: + return fmt.Errorf("forbidden sighash type %#x", uint32(sighashType)) + } +} + +// checkSighashTypes rejects a base64 encoded tx declaring a forbidden sighash +// type on any of its inputs. +func checkSighashTypes(tx string) error { + ptx, err := psbt.NewFromRawBytes(strings.NewReader(tx), true) + if err != nil { + return err + } + + for inputIndex, input := range ptx.Inputs { + if err := checkSighashType(input.SighashType); err != nil { + return fmt.Errorf("input %d: %s", inputIndex, err) + } + } + + return nil +} + // verifyOffchainTx verifies the signer signatures of the given transaction func verifyOffchainTx(original, signed *psbt.Packet, signers map[string]*btcec.PublicKey) error { if original.UnsignedTx.TxID() != signed.UnsignedTx.TxID() { @@ -313,6 +344,20 @@ func verifyOffchainTx(original, signed *psbt.Packet, signers map[string]*btcec.P ) } + // the sighash type is declared by the signer, verifying under it would + // accept a signature committing to nothing, so it must be allowed and + // must match the one of the tx we built. + if err := checkSighashType(signedInput.SighashType); err != nil { + return fmt.Errorf("input %d: %s", inputIndex, err) + } + + if signedInput.SighashType != originalInput.SighashType { + return fmt.Errorf( + "sighash type mismatch for input %d: expected %#x, got %#x", + inputIndex, uint32(originalInput.SighashType), uint32(signedInput.SighashType), + ) + } + // check that every input has the signer's signature var signerSig *psbt.TaprootScriptSpendSig var signerPubkey *btcec.PublicKey @@ -337,7 +382,7 @@ func verifyOffchainTx(original, signed *psbt.Packet, signers map[string]*btcec.P // verify the signature message, err := txscript.CalcTapscriptSignaturehash( txsigHashes, - signedInput.SighashType, + originalInput.SighashType, original.UnsignedTx, inputIndex, prevoutFetcher, @@ -636,6 +681,12 @@ func finalizeTx( finalCheckpoints := make([]string, 0, len(acceptedTx.SignedCheckpointTxs)) for _, checkpoint := range acceptedTx.SignedCheckpointTxs { + // the checkpoints are built by the counterparty and signed as they come, + // a forbidden sighash type would make us sign a blank cheque. + if err := checkSighashTypes(checkpoint); err != nil { + return "", nil, err + } + signedTx, err := signTx(ctx, checkpoint) if err != nil { return "", nil, err From 4fc9bcde535da8f8660f9c5e7a30ef297a3b1b46 Mon Sep 17 00:00:00 2001 From: Kukks Date: Wed, 2 Sep 2026 09:12:05 +0200 Subject: [PATCH 2/4] fix: validate tx tree finality --- pkg/ark-lib/tree/tx_tree.go | 17 ++++++ pkg/ark-lib/tree/tx_tree_finality_test.go | 67 +++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 pkg/ark-lib/tree/tx_tree_finality_test.go diff --git a/pkg/ark-lib/tree/tx_tree.go b/pkg/ark-lib/tree/tx_tree.go index 7f6c04b3d..4d0524001 100644 --- a/pkg/ark-lib/tree/tx_tree.go +++ b/pkg/ark-lib/tree/tx_tree.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/btcsuite/btcd/psbt/v2" + "github.com/btcsuite/btcd/wire/v2" ) // LeafOutput represents the output of a leaf transaction. @@ -199,6 +200,7 @@ func (t *TxTree) SerializeNode() (*TxTreeNode, error) { // It verifies : // - every tx is a valid partial transaction. // - every tx has exactly one input. +// - every tx is final, ie. it can be broadcasted as soon as its parent is confirmed // - the child txs spend the right parent's output // - the sum of the child txs' output amounts matches the parent tx input amount func (t *TxTree) Validate() error { @@ -210,6 +212,12 @@ func (t *TxTree) Validate() error { return fmt.Errorf("unexpected version: %d, expected 3", t.Root.UnsignedTx.Version) } + // A non-final tx can't be broadcasted until its timelock elapses, which would + // hold the unroll path back while the batch output sweep matures. + if t.Root.UnsignedTx.LockTime != 0 { + return fmt.Errorf("unexpected locktime: %d, expected 0", t.Root.UnsignedTx.LockTime) + } + nbOfOutputs := uint32(len(t.Root.UnsignedTx.TxOut)) nbOfInputs := uint32(len(t.Root.UnsignedTx.TxIn)) @@ -217,6 +225,15 @@ func (t *TxTree) Validate() error { return fmt.Errorf("unexpected number of inputs: %d, expected 1", nbOfInputs) } + for inputIndex, input := range t.Root.UnsignedTx.TxIn { + if input.Sequence != wire.MaxTxInSequenceNum { + return fmt.Errorf( + "unexpected sequence for input %d: %d, expected %d", + inputIndex, input.Sequence, uint32(wire.MaxTxInSequenceNum), + ) + } + } + // The children map can't be bigger than the number of outputs (excluding the P2A). // A tx tree can be "partial" and specify only some of the outputs as children, // that's why we allow len(g.Children) to be less than nbOfOutputs-1 diff --git a/pkg/ark-lib/tree/tx_tree_finality_test.go b/pkg/ark-lib/tree/tx_tree_finality_test.go new file mode 100644 index 000000000..c9a8c1fcc --- /dev/null +++ b/pkg/ark-lib/tree/tx_tree_finality_test.go @@ -0,0 +1,67 @@ +package tree_test + +import ( + "testing" + + "github.com/arkade-os/arkd/pkg/ark-lib/tree" + "github.com/btcsuite/btcd/wire/v2" + "github.com/stretchr/testify/require" +) + +// allNodes returns every node of the tree, so a mutation can be applied to the +// deeper ones too and only surface if Validate() really recurses. +func allNodes(txTree *tree.TxTree) []*tree.TxTree { + nodes := []*tree.TxTree{txTree} + for _, child := range txTree.Children { + nodes = append(nodes, allNodes(child)...) + } + return nodes +} + +// TestTxTreeValidateFinality checks that a tree holding a node that bitcoin +// would not accept right away is rejected. A pre-signed unroll path is only +// worth something if it can be broadcasted as soon as its parent confirms. +func TestTxTreeValidateFinality(t *testing.T) { + testVectors, err := makeTestVectors() + require.NoError(t, err) + require.NotEmpty(t, testVectors) + + for _, v := range testVectors { + t.Run(v.name, func(t *testing.T) { + vtxoTree, err := tree.BuildVtxoTree( + rootInput, v.receivers, batchOutSweepClosure[:], vtxoTreeExpiry, + ) + require.NoError(t, err) + + connectorTree, err := tree.BuildConnectorTree(rootInput, v.receivers) + require.NoError(t, err) + + for name, txTree := range map[string]*tree.TxTree{ + "vtxo tree": vtxoTree, "connector tree": connectorTree, + } { + t.Run(name, func(t *testing.T) { + // the tree as built by the honest stack must stay valid + require.NoError(t, txTree.Validate()) + + for i, node := range allNodes(txTree) { + node.Root.UnsignedTx.LockTime = 1 + require.ErrorContainsf( + t, txTree.Validate(), "unexpected locktime", + "locktime not validated on node %d", i, + ) + node.Root.UnsignedTx.LockTime = 0 + require.NoError(t, txTree.Validate()) + + node.Root.UnsignedTx.TxIn[0].Sequence = wire.MaxTxInSequenceNum - 1 + require.ErrorContainsf( + t, txTree.Validate(), "unexpected sequence", + "sequence not validated on node %d", i, + ) + node.Root.UnsignedTx.TxIn[0].Sequence = wire.MaxTxInSequenceNum + require.NoError(t, txTree.Validate()) + } + }) + } + }) + } +} From cdc8e0d7a488c55d84efe8b5394caad6e0e52319 Mon Sep 17 00:00:00 2001 From: Kukks Date: Wed, 2 Sep 2026 09:16:24 +0200 Subject: [PATCH 3/4] fix: validate vtxo tree before signing --- .../batch-session/handler/default_handler.go | 36 ++- .../handler/default_handler_test.go | 218 ++++++++++++++++++ 2 files changed, 244 insertions(+), 10 deletions(-) create mode 100644 pkg/client-lib/batch-session/handler/default_handler_test.go diff --git a/pkg/client-lib/batch-session/handler/default_handler.go b/pkg/client-lib/batch-session/handler/default_handler.go index b8a89bdc6..1225c7b9f 100644 --- a/pkg/client-lib/batch-session/handler/default_handler.go +++ b/pkg/client-lib/batch-session/handler/default_handler.go @@ -179,6 +179,12 @@ func (h *defaultHandler) OnTreeSigningStarted( return false, err } + // the tree has to be verified before we send any nonce or signature for it, + // otherwise we pre-sign unroll paths we haven't checked. + if err := h.validateVtxoTreeAgainstCommitmentTx(commitmentTx, vtxoTree); err != nil { + return false, fmt.Errorf("failed to verify vtxo tree: %s", err) + } + batchOutput := commitmentTx.UnsignedTx.TxOut[0] batchOutputAmount := batchOutput.Value @@ -438,15 +444,11 @@ func (h *defaultHandler) vtxosToForfeit() []clientlib.Vtxo { return withoutRecoverable } -func (h *defaultHandler) validateVtxoTree( - event clientlib.BatchFinalizationEvent, vtxoTree, connectorTree *tree.TxTree, +// validateVtxoTreeAgainstCommitmentTx groups the checks needing nothing but the +// vtxo tree and the commitment tx, so that they can run before the tree is signed. +func (h *defaultHandler) validateVtxoTreeAgainstCommitmentTx( + commitmentPtx *psbt.Packet, vtxoTree *tree.TxTree, ) error { - commitmentTx := event.Tx - commitmentPtx, err := psbt.NewFromRawBytes(strings.NewReader(commitmentTx), true) - if err != nil { - return err - } - // validate the vtxo tree is well formed if !isOnchainOnly(h.Receivers) { if err := tree.ValidateVtxoTree( @@ -476,9 +478,23 @@ func (h *defaultHandler) validateVtxoTree( } // validate it contains our outputs - if err := validateReceivers( + return validateReceivers( h.ServerParams.Network, commitmentPtx, h.Receivers, vtxoTree, - ); err != nil { + ) +} + +func (h *defaultHandler) validateVtxoTree( + event clientlib.BatchFinalizationEvent, vtxoTree, connectorTree *tree.TxTree, +) error { + commitmentTx := event.Tx + commitmentPtx, err := psbt.NewFromRawBytes(strings.NewReader(commitmentTx), true) + if err != nil { + return err + } + + // re-run them against the commitment tx of this event, it may not be the one + // we were given when the signing session started. + if err := h.validateVtxoTreeAgainstCommitmentTx(commitmentPtx, vtxoTree); err != nil { return err } diff --git a/pkg/client-lib/batch-session/handler/default_handler_test.go b/pkg/client-lib/batch-session/handler/default_handler_test.go new file mode 100644 index 000000000..e33fd914a --- /dev/null +++ b/pkg/client-lib/batch-session/handler/default_handler_test.go @@ -0,0 +1,218 @@ +package batchsessionhandler + +import ( + "context" + "encoding/hex" + "testing" + + arklib "github.com/arkade-os/arkd/pkg/ark-lib" + "github.com/arkade-os/arkd/pkg/ark-lib/script" + "github.com/arkade-os/arkd/pkg/ark-lib/tree" + clientlib "github.com/arkade-os/arkd/pkg/client-lib" + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/chainhash/v2" + "github.com/btcsuite/btcd/psbt/v2" + "github.com/btcsuite/btcd/txscript/v2" + "github.com/btcsuite/btcd/wire/v2" + "github.com/stretchr/testify/require" +) + +// recordingSignerSession records whether the tree ever reached the musig2 session. +type recordingSignerSession struct { + tree.SignerSession + pubkey string + initiated bool +} + +func (s *recordingSignerSession) Init(_ []byte, _ int64, _ *tree.TxTree) error { + s.initiated = true + return nil +} + +func (s *recordingSignerSession) GetPublicKey() string { return s.pubkey } + +func (s *recordingSignerSession) GetNonces() (tree.TreeNonces, error) { + return tree.TreeNonces{}, nil +} + +// recordingClient records the nonce submissions so a test can assert nothing was +// sent for a tree that didn't validate. +type recordingClient struct { + clientlib.Client + submittedNonces int +} + +func (c *recordingClient) SubmitTreeNonces( + _ context.Context, _, _ string, _ tree.TreeNonces, +) error { + c.submittedNonces++ + return nil +} + +type treeSigningFixture struct { + handler *defaultHandler + client *recordingClient + session *recordingSignerSession + event clientlib.TreeSigningStartedEvent + vtxoTree *tree.TxTree +} + +// newTreeSigningFixture builds the batch a honest server would propose: a +// commitment tx paying the batch output and the vtxo tree spending it. +func newTreeSigningFixture(t *testing.T) *treeSigningFixture { + t.Helper() + + forfeitPrvkey, err := btcec.NewPrivateKey() + require.NoError(t, err) + cosignerPrvkey, err := btcec.NewPrivateKey() + require.NoError(t, err) + vtxoPrvkey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + batchExpiry := arklib.RelativeLocktime{Type: arklib.LocktimeTypeBlock, Value: 144} + + sweepClosure := script.CSVMultisigClosure{ + MultisigClosure: script.MultisigClosure{ + PubKeys: []*btcec.PublicKey{forfeitPrvkey.PubKey()}, + }, + Locktime: batchExpiry, + } + sweepScript, err := sweepClosure.Script() + require.NoError(t, err) + + sweepRoot := txscript.AssembleTaprootScriptTree( + txscript.NewBaseTapLeaf(sweepScript), + ).RootNode.TapHash() + + vtxoPkScript, err := script.P2TRScript(vtxoPrvkey.PubKey()) + require.NoError(t, err) + + const receiverAmount = 10000 + leaf := tree.Leaf{ + Outputs: []tree.LeafOutput{ + {Amount: receiverAmount, Script: hex.EncodeToString(vtxoPkScript)}, + }, + CosignersPublicKeys: []string{ + hex.EncodeToString(cosignerPrvkey.PubKey().SerializeCompressed()), + }, + } + + batchOutScript, batchOutAmount, err := tree.BuildBatchOutput( + []tree.Leaf{leaf}, sweepRoot[:], + ) + require.NoError(t, err) + + prevoutHash, err := chainhash.NewHashFromStr( + "49f8664acc899be91902f8ade781b7eeb9cbe22bdd9efbc36e56195de21bcd12", + ) + require.NoError(t, err) + + commitmentTx, err := psbt.New( + []*wire.OutPoint{{Hash: *prevoutHash, Index: 0}}, + []*wire.TxOut{{Value: batchOutAmount, PkScript: batchOutScript}}, + 3, 0, []uint32{wire.MaxTxInSequenceNum}, + ) + require.NoError(t, err) + + commitmentTxHash := commitmentTx.UnsignedTx.TxHash() + vtxoTree, err := tree.BuildVtxoTree( + &wire.OutPoint{Hash: commitmentTxHash, Index: 0}, + []tree.Leaf{leaf}, sweepRoot[:], batchExpiry, + ) + require.NoError(t, err) + + encodedCommitmentTx, err := commitmentTx.B64Encode() + require.NoError(t, err) + + addr := arklib.Address{ + HRP: arklib.BitcoinRegTest.Addr, + Signer: forfeitPrvkey.PubKey(), + VtxoTapKey: vtxoPrvkey.PubKey(), + } + encodedAddr, err := addr.EncodeV0() + require.NoError(t, err) + + client := &recordingClient{} + session := &recordingSignerSession{ + pubkey: hex.EncodeToString(cosignerPrvkey.PubKey().SerializeCompressed()), + } + + return &treeSigningFixture{ + handler: &defaultHandler{ + Args: Args{ + Client: client, + ServerParams: clientlib.ServerParams{ + Network: arklib.BitcoinRegTest, + ForfeitPubKey: forfeitPrvkey.PubKey(), + }, + Receivers: []clientlib.Receiver{{To: encodedAddr, Amount: receiverAmount}}, + SignerSessions: []tree.SignerSession{session}, + }, + batchExpiry: batchExpiry, + }, + client: client, + session: session, + event: clientlib.TreeSigningStartedEvent{ + Id: "batch-id", + UnsignedCommitmentTx: encodedCommitmentTx, + CosignersPubkeys: []string{session.pubkey}, + }, + vtxoTree: vtxoTree, + } +} + +func TestOnTreeSigningStartedValidatesBeforeSigning(t *testing.T) { + // the tree of a honest batch must still reach the musig2 session + t.Run("valid", func(t *testing.T) { + f := newTreeSigningFixture(t) + + skip, err := f.handler.OnTreeSigningStarted( + context.Background(), f.event, f.vtxoTree, + ) + require.NoError(t, err) + require.False(t, skip) + require.True(t, f.session.initiated) + require.Equal(t, 1, f.client.submittedNonces) + }) + + // a tree not spending the batch output must be rejected before we contribute + // anything to the signing session + t.Run("not spending the batch output", func(t *testing.T) { + f := newTreeSigningFixture(t) + f.vtxoTree.Root.UnsignedTx.TxIn[0].PreviousOutPoint.Index = 1 + + skip, err := f.handler.OnTreeSigningStarted( + context.Background(), f.event, f.vtxoTree, + ) + require.ErrorContains(t, err, "failed to verify vtxo tree") + require.False(t, skip) + require.False(t, f.session.initiated) + require.Zero(t, f.client.submittedNonces) + }) + + // same for a tree we couldn't broadcast when we need to unroll + t.Run("non final", func(t *testing.T) { + f := newTreeSigningFixture(t) + f.vtxoTree.Root.UnsignedTx.TxIn[0].Sequence = wire.MaxTxInSequenceNum - 1 + + _, err := f.handler.OnTreeSigningStarted( + context.Background(), f.event, f.vtxoTree, + ) + require.ErrorContains(t, err, "unexpected sequence") + require.False(t, f.session.initiated) + require.Zero(t, f.client.submittedNonces) + }) + + // and for a tree not paying what we asked for + t.Run("wrong receiver amount", func(t *testing.T) { + f := newTreeSigningFixture(t) + f.handler.Receivers[0].Amount++ + + _, err := f.handler.OnTreeSigningStarted( + context.Background(), f.event, f.vtxoTree, + ) + require.ErrorContains(t, err, "offchain send output not found") + require.False(t, f.session.initiated) + require.Zero(t, f.client.submittedNonces) + }) +} From 3ffee5cb168042e7d23486295ddbfe90c985b2e2 Mon Sep 17 00:00:00 2001 From: Kukks Date: Wed, 2 Sep 2026 09:22:55 +0200 Subject: [PATCH 4/4] fix: require default sighash on returned checkpoints --- pkg/client-lib/offchain-tx/sighash_test.go | 23 ++++++++++++++++++++++ pkg/client-lib/offchain-tx/utils.go | 17 ++++++++++------ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/pkg/client-lib/offchain-tx/sighash_test.go b/pkg/client-lib/offchain-tx/sighash_test.go index 495e23abb..aa263eda5 100644 --- a/pkg/client-lib/offchain-tx/sighash_test.go +++ b/pkg/client-lib/offchain-tx/sighash_test.go @@ -128,6 +128,29 @@ func TestFinalizeTxSighashPolicy(t *testing.T) { } }) + // unlike the submit path there is no locally built checkpoint to compare + // against here, so anything but SIGHASH_DEFAULT is refused. + t.Run("not default", func(t *testing.T) { + for _, sighashType := range []txscript.SigHashType{ + txscript.SigHashAll, + txscript.SigHashAll | txscript.SigHashAnyOneCanPay, + } { + t.Run(fmt.Sprintf("%#x", uint32(sighashType)), func(t *testing.T) { + client := &recordingClient{} + + _, _, err := finalizeTx( + context.Background(), client, client.signTx, + clientlib.AcceptedOffchainTx{ + Txid: txid, + SignedCheckpointTxs: []string{withSighashType(t, checkpoint, sighashType)}, + }, + ) + require.ErrorContains(t, err, "expected SIGHASH_DEFAULT") + require.Empty(t, client.signed) + require.False(t, client.finalized) + }) + } + }) t.Run("allowed", func(t *testing.T) { client := &recordingClient{} diff --git a/pkg/client-lib/offchain-tx/utils.go b/pkg/client-lib/offchain-tx/utils.go index 1910e7fac..7ff6de52b 100644 --- a/pkg/client-lib/offchain-tx/utils.go +++ b/pkg/client-lib/offchain-tx/utils.go @@ -280,17 +280,22 @@ func checkSighashType(sighashType txscript.SigHashType) error { } } -// checkSighashTypes rejects a base64 encoded tx declaring a forbidden sighash -// type on any of its inputs. -func checkSighashTypes(tx string) error { +// checkCheckpointSighashTypes rejects a base64 encoded checkpoint tx declaring +// anything but SIGHASH_DEFAULT on any of its inputs. Checkpoints are never +// stamped with a sighash type when we build them, and unlike the submit path +// there is no locally built tx here to compare against. +func checkCheckpointSighashTypes(tx string) error { ptx, err := psbt.NewFromRawBytes(strings.NewReader(tx), true) if err != nil { return err } for inputIndex, input := range ptx.Inputs { - if err := checkSighashType(input.SighashType); err != nil { - return fmt.Errorf("input %d: %s", inputIndex, err) + if input.SighashType != txscript.SigHashDefault { + return fmt.Errorf( + "input %d: forbidden sighash type %#x, expected SIGHASH_DEFAULT", + inputIndex, uint32(input.SighashType), + ) } } @@ -683,7 +688,7 @@ func finalizeTx( for _, checkpoint := range acceptedTx.SignedCheckpointTxs { // the checkpoints are built by the counterparty and signed as they come, // a forbidden sighash type would make us sign a blank cheque. - if err := checkSighashTypes(checkpoint); err != nil { + if err := checkCheckpointSighashTypes(checkpoint); err != nil { return "", nil, err }