Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 52 additions & 16 deletions mobile/swaps.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ package lndmobile
import (
"bytes"
"encoding/hex"
"errors"
"fmt"
"io"
"net/http"

"github.com/BoltzExchange/boltz-client/v2/pkg/boltz"
"github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/mempool"
"github.com/btcsuite/btcd/txscript"
"github.com/decred/dcrd/dcrec/secp256k1/v4"
)
Expand Down Expand Up @@ -166,16 +168,7 @@ func BuildReverseClaimTransaction(endpoint string, id string, claimLeaf string,
return "", fmt.Errorf("error decoding preimage hex string: %w", err)
}

var fee boltz.Fee
if minerFee > 0 {
sats := uint64(minerFee)
fee = boltz.Fee{Sats: &sats}
} else {
satPerVbyte := float64(feeRate)
fee = boltz.Fee{SatsPerVbyte: &satPerVbyte}
}

claimTransaction, _, err := boltz.ConstructTransaction(
claimTransaction, results, err := boltz.ConstructTransaction(
chain,
boltz.CurrencyBtc,
[]boltz.OutputDetails{
Expand All @@ -191,9 +184,12 @@ func BuildReverseClaimTransaction(endpoint string, id string, claimLeaf string,
Cooperative: true,
},
},
fee,
swapFee(feeRate, minerFee),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here: results from ConstructTransaction (line 169) is discarded. If Boltz refuses the cooperative claim, boltz-client falls back to a non-cooperative claim, which can return the same empty-output or 0-value transaction with a nil error when minerFee is at or above the lockup value.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 08c2b5f with the same check. A test claims against a host that refuses to co-sign, with minerFee of 100000 and 150000, and expects an error.

&boltz.Api{URL: endpoint},
)
if err == nil {
err = checkSwapTransaction(id, claimTransaction, results)
}
if err != nil {
return "", fmt.Errorf("could not create claim transaction: %w", err)
}
Expand All @@ -206,9 +202,47 @@ func BuildReverseClaimTransaction(endpoint string, id string, claimLeaf string,
return txHex, nil
}

// swapFee pays an exact miner fee in sats when one is given, and the fee rate
// in sat/vbyte otherwise.
func swapFee(feeRate int32, minerFee int32) boltz.Fee {
if minerFee > 0 {
sats := uint64(minerFee)
return boltz.Fee{Sats: &sats}
}
satPerVbyte := float64(feeRate)
return boltz.Fee{SatsPerVbyte: &satPerVbyte}
}

// checkSwapTransaction returns the error ConstructTransaction only records in
// its results, and rejects an output that would not relay. A fee at or above
// the lockup value otherwise leaves a transaction with no output, or a 0-sat
// one, and no error.
func checkSwapTransaction(id string, transaction boltz.Transaction, results boltz.Results) error {
if err := results[id].Err; err != nil {
return err
}

btcTransaction, ok := transaction.(*boltz.BtcTransaction)
if !ok {
return fmt.Errorf("unexpected transaction type %T", transaction)
}

outputs := btcTransaction.MsgTx().TxOut
if len(outputs) == 0 {
return errors.New("transaction has no outputs")
}
for _, output := range outputs {
if mempool.IsDust(output, mempool.DefaultMinRelayTxFee) {
return fmt.Errorf("output of %d sats is below the dust limit", output.Value)
}
}

return nil
}

// BuildRefundTransaction builds and signs the refund of a submarine swap's
// lockup and returns it as hex without broadcasting it.
func BuildRefundTransaction(endpoint string, id string, claimLeaf string, refundLeaf string, transactionHex string, privateKey string, servicePubKey string, feeRate int32, timeoutBlockHeight int32, destinationAddress string, lockupAddress string, cooperative bool, network string) (string, error) {
func BuildRefundTransaction(endpoint string, id string, claimLeaf string, refundLeaf string, transactionHex string, privateKey string, servicePubKey string, feeRate int32, minerFee int32, timeoutBlockHeight int32, destinationAddress string, lockupAddress string, cooperative bool, network string) (string, error) {
chain, err := swapNetwork(network)
if err != nil {
return "", err
Expand All @@ -229,8 +263,7 @@ func BuildRefundTransaction(endpoint string, id string, claimLeaf string, refund
return "", fmt.Errorf("error finding vout: %w", err)
}

satPerVbyte := float64(feeRate)
refundTransaction, _, err := boltz.ConstructTransaction(
refundTransaction, results, err := boltz.ConstructTransaction(
chain,
boltz.CurrencyBtc,
[]boltz.OutputDetails{
Expand All @@ -247,9 +280,12 @@ func BuildRefundTransaction(endpoint string, id string, claimLeaf string, refund
Cooperative: cooperative,
},
},
boltz.Fee{SatsPerVbyte: &satPerVbyte},
swapFee(feeRate, minerFee),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a sats fee, nothing checks minerFee against the lockup value, and the Results from boltz.ConstructTransaction are discarded on line 234.

In boltz-client (pkg/boltz/transaction.go:134-140, same in v2.9.0 and v2.12.5), when the value is less than the fee, getOutValues only records value less than fee in Results and skips the output. constructBtcTransaction then builds a transaction with no outputs, and for a non-cooperative refund it signs the input and returns err == nil. When the fee equals the value, the check passes and a 0-sat output is added.

Reproduced on this branch with the existing newRefundFixture (100000 sat lockup), non-cooperative:

minerFee err outputs
100000 nil 1 output, value 0
150000 nil none

Neither transaction relays (bad-txns-vout-empty / dust), and CreateRefundTransaction passes the mempool.space rejection back with no hint of the cause. Before this PR the refund only took a fee rate, so this needed an absurd rate.

Suggested fix:

  1. Keep the results (refundTransaction, results, err := ...) and return results[id].Err when it is set.
  2. Reject a minerFee that leaves the output below dust (330 sat for P2TR, 294 for P2WPKH), since Results has no error for the equal-value case.

A test calling BuildRefundTransaction with minerFee of 100000 and 150000 against the fixture, expecting an error, would cover both cases.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 08c2b5f. Both builders now return the error ConstructTransaction records in results, and reject a transaction with no output or one below dust (mempool.IsDust at the default relay fee, so 330/294). Tests cover 99800, 100000 and 150000 against the fixture.

&boltz.Api{URL: endpoint},
)
if err == nil {
err = checkSwapTransaction(id, refundTransaction, results)
}
if err != nil {
return "", fmt.Errorf("could not create refund transaction: %w", err)
}
Expand Down Expand Up @@ -312,7 +348,7 @@ func CreateReverseClaimTransaction(endpoint string, id string, claimLeaf string,
// mempool.space and returns the txid. Kept for app builds that predate
// BuildRefundTransaction.
func CreateRefundTransaction(endpoint string, id string, claimLeaf string, refundLeaf string, transactionHex string, privateKey string, servicePubKey string, feeRate int32, timeoutBlockHeight int32, destinationAddress string, lockupAddress string, cooperative bool, isTestnet bool) (string, error) {
txHex, err := BuildRefundTransaction(endpoint, id, claimLeaf, refundLeaf, transactionHex, privateKey, servicePubKey, feeRate, timeoutBlockHeight, destinationAddress, lockupAddress, cooperative, legacyNetworkName(isTestnet))
txHex, err := BuildRefundTransaction(endpoint, id, claimLeaf, refundLeaf, transactionHex, privateKey, servicePubKey, feeRate, 0, timeoutBlockHeight, destinationAddress, lockupAddress, cooperative, legacyNetworkName(isTestnet))
if err != nil {
return "", err
}
Expand Down
88 changes: 87 additions & 1 deletion mobile/swaps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestBuildRefundTransactionDoesNotBroadcast(t *testing.T) {
hex.EncodeToString(refund.refundLeaf), refund.lockupHex,
hex.EncodeToString(refund.ourKey.Serialize()),
hex.EncodeToString(refund.serverKey.PubKey().SerializeCompressed()),
2, 150, destination.EncodeAddress(), refund.lockupAddress, false,
2, 0, 150, destination.EncodeAddress(), refund.lockupAddress, false,
"regtest",
)
require.NoError(t, err)
Expand Down Expand Up @@ -269,3 +269,89 @@ func newRefundFixture(t *testing.T) swapFixture {
lockupTxid: lockup.TxHash(),
}
}

func TestBuildRefundTransactionPaysExactMinerFee(t *testing.T) {
destination, err := btcutil.NewAddressTaproot(
bytes.Repeat([]byte{3}, 32), boltz.Regtest.Btc,
)
require.NoError(t, err)

refund := newRefundFixture(t)
txHex, err := BuildRefundTransaction(
"http://127.0.0.1:1", "swap", hex.EncodeToString(refund.claimLeaf),
hex.EncodeToString(refund.refundLeaf), refund.lockupHex,
hex.EncodeToString(refund.ourKey.Serialize()),
hex.EncodeToString(refund.serverKey.PubKey().SerializeCompressed()),
2, 250, 150, destination.EncodeAddress(), refund.lockupAddress,
false, "regtest",
)
require.NoError(t, err)

raw, err := hex.DecodeString(txHex)
require.NoError(t, err)
tx := wire.NewMsgTx(2)
require.NoError(t, tx.Deserialize(bytes.NewReader(raw)))

require.Len(t, tx.TxOut, 1)
require.Equal(t, int64(100000-250), tx.TxOut[0].Value)
}

func TestBuildRefundTransactionRejectsFeeAtOrAboveLockup(t *testing.T) {
destination, err := btcutil.NewAddressTaproot(
bytes.Repeat([]byte{3}, 32), boltz.Regtest.Btc,
)
require.NoError(t, err)

refund := newRefundFixture(t)
tests := []struct {
minerFee int32
err string
}{
{minerFee: 99800, err: "below the dust limit"},
{minerFee: 100000, err: "below the dust limit"},
{minerFee: 150000, err: "value less than fee"},
}
for _, test := range tests {
_, err := BuildRefundTransaction(
"http://127.0.0.1:1", "swap",
hex.EncodeToString(refund.claimLeaf),
hex.EncodeToString(refund.refundLeaf), refund.lockupHex,
hex.EncodeToString(refund.ourKey.Serialize()),
hex.EncodeToString(
refund.serverKey.PubKey().SerializeCompressed(),
),
2, test.minerFee, 150, destination.EncodeAddress(),
refund.lockupAddress, false, "regtest",
)
require.ErrorContains(t, err, test.err, "minerFee %d", test.minerFee)
}
}

func TestBuildReverseClaimTransactionRejectsFeeAtOrAboveLockup(t *testing.T) {
fixture := newReverseSwapFixture(t)

// the host refuses to co-sign, so the claim falls back to the script
// path, which is where an oversized fee went unreported
host := httptest.NewServer(http.NotFoundHandler())
defer host.Close()

destination, err := btcutil.NewAddressTaproot(
bytes.Repeat([]byte{2}, 32), boltz.Regtest.Btc,
)
require.NoError(t, err)

for _, minerFee := range []int32{100000, 150000} {
_, err := BuildReverseClaimTransaction(
host.URL, "swap", hex.EncodeToString(fixture.claimLeaf),
hex.EncodeToString(fixture.refundLeaf),
hex.EncodeToString(fixture.ourKey.Serialize()),
hex.EncodeToString(
fixture.serverKey.PubKey().SerializeCompressed(),
),
hex.EncodeToString(fixture.preimage), fixture.lockupHex,
fixture.lockupAddress, destination.EncodeAddress(), 2,
minerFee, "regtest",
)
require.Error(t, err, "minerFee %d", minerFee)
}
}
Loading