Skip to content

Commit

Permalink
Merge pull request #4905 from LN-Zap/feat/spend-unconfirmed-estimate
Browse files Browse the repository at this point in the history
Support spend_unconfirmed in EstimateFee and FundPsbt
  • Loading branch information
Roasbeef authored Apr 26, 2021
2 parents 9ef00d9 + 2f80283 commit bfcaf02
Show file tree
Hide file tree
Showing 16 changed files with 1,068 additions and 906 deletions.
1,433 changes: 726 additions & 707 deletions lnrpc/rpc.pb.go

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions lnrpc/rpc.proto
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,13 @@ message EstimateFeeRequest {
// The target number of blocks that this transaction should be confirmed
// by.
int32 target_conf = 2;

// The minimum number of confirmations each one of your outputs used for
// the transaction must satisfy.
int32 min_confs = 3;

// Whether unconfirmed outputs should be used as inputs for the transaction.
bool spend_unconfirmed = 4;
}

message EstimateFeeResponse {
Expand Down
16 changes: 16 additions & 0 deletions lnrpc/rpc.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -2034,6 +2034,22 @@
"required": false,
"type": "integer",
"format": "int32"
},
{
"name": "min_confs",
"description": "The minimum number of confirmations each one of your outputs used for\nthe transaction must satisfy.",
"in": "query",
"required": false,
"type": "integer",
"format": "int32"
},
{
"name": "spend_unconfirmed",
"description": "Whether unconfirmed outputs should be used as inputs for the transaction.",
"in": "query",
"required": false,
"type": "boolean",
"format": "boolean"
}
],
"tags": [
Expand Down
1 change: 0 additions & 1 deletion lnrpc/walletrpc/psbt.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
)

const (
defaultMinConf = 1
defaultMaxConf = math.MaxInt32
)

Expand Down
306 changes: 163 additions & 143 deletions lnrpc/walletrpc/walletkit.pb.go

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions lnrpc/walletrpc/walletkit.proto
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,13 @@ message FundPsbtRequest {
account is used.
*/
string account = 5;

// The minimum number of confirmations each one of your outputs used for
// the transaction must satisfy.
int32 min_confs = 6;

// Whether unconfirmed outputs should be used as inputs for the transaction.
bool spend_unconfirmed = 7;
}
message FundPsbtResponse {
/*
Expand Down
10 changes: 10 additions & 0 deletions lnrpc/walletrpc/walletkit.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,16 @@
"account": {
"type": "string",
"description": "The name of the account to fund the PSBT with. If empty, the default wallet\naccount is used."
},
"min_confs": {
"type": "integer",
"format": "int32",
"description": "The minimum number of confirmations each one of your outputs used for\nthe transaction must satisfy."
},
"spend_unconfirmed": {
"type": "boolean",
"format": "boolean",
"description": "Whether unconfirmed outputs should be used as inputs for the transaction."
}
}
},
Expand Down
11 changes: 10 additions & 1 deletion lnrpc/walletrpc/walletkit_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,15 @@ func (w *WalletKit) FundPsbt(_ context.Context,
"specify either target_conf or set_per_vbyte")
}

// Then, we'll extract the minimum number of confirmations that each
// output we use to fund the transaction should satisfy.
minConfs, err := lnrpc.ExtractMinConfs(
req.GetMinConfs(), req.GetSpendUnconfirmed(),
)
if err != nil {
return nil, err
}

// The RPC parsing part is now over. Several of the following operations
// require us to hold the global coin selection lock so we do the rest
// of the tasks while holding the lock. The result is a list of locked
Expand All @@ -1087,7 +1096,7 @@ func (w *WalletKit) FundPsbt(_ context.Context,
if len(packet.UnsignedTx.TxIn) > 0 {
// Get a list of all unspent witness outputs.
utxos, err := w.cfg.Wallet.ListUnspentWitness(
defaultMinConf, defaultMaxConf, account,
minConfs, defaultMaxConf, account,
)
if err != nil {
return err
Expand Down
6 changes: 3 additions & 3 deletions lntest/mock/walletcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,15 @@ func (w *WalletController) SendOutputs(outputs []*wire.TxOut,

// CreateSimpleTx currently returns dummy values.
func (w *WalletController) CreateSimpleTx(outputs []*wire.TxOut,
_ chainfee.SatPerKWeight, _ bool) (*txauthor.AuthoredTx, error) {
_ chainfee.SatPerKWeight, _ int32, _ bool) (*txauthor.AuthoredTx, error) {

return nil, nil
}

// ListUnspentWitness is called by the wallet when doing coin selection. We just
// need one unspent for the funding transaction.
func (w *WalletController) ListUnspentWitness(minconfirms,
maxconfirms int32, _ string) ([]*lnwallet.Utxo, error) {
func (w *WalletController) ListUnspentWitness(minConfs,
maxConfs int32, _ string) ([]*lnwallet.Utxo, error) {

// If the mock already has a list of utxos, return it.
if w.Utxos != nil {
Expand Down
27 changes: 17 additions & 10 deletions lnwallet/btcwallet/btcwallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ func (b *BtcWallet) ImportPublicKey(pubKey *btcec.PublicKey,
//
// This is a part of the WalletController interface.
func (b *BtcWallet) SendOutputs(outputs []*wire.TxOut,
feeRate chainfee.SatPerKWeight, minconf int32, label string) (*wire.MsgTx, error) {
feeRate chainfee.SatPerKWeight, minConfs int32, label string) (*wire.MsgTx, error) {

// Convert our fee rate from sat/kw to sat/kb since it's required by
// SendOutputs.
Expand All @@ -495,13 +495,13 @@ func (b *BtcWallet) SendOutputs(outputs []*wire.TxOut,
return nil, lnwallet.ErrNoOutputs
}

// Sanity check minconf.
if minconf < 0 {
// Sanity check minConfs.
if minConfs < 0 {
return nil, lnwallet.ErrInvalidMinconf
}

return b.wallet.SendOutputs(
outputs, nil, defaultAccount, minconf, feeSatPerKB, label,
outputs, nil, defaultAccount, minConfs, feeSatPerKB, label,
)
}

Expand All @@ -519,7 +519,8 @@ func (b *BtcWallet) SendOutputs(outputs []*wire.TxOut,
//
// This is a part of the WalletController interface.
func (b *BtcWallet) CreateSimpleTx(outputs []*wire.TxOut,
feeRate chainfee.SatPerKWeight, dryRun bool) (*txauthor.AuthoredTx, error) {
feeRate chainfee.SatPerKWeight, minConfs int32,
dryRun bool) (*txauthor.AuthoredTx, error) {

// The fee rate is passed in using units of sat/kw, so we'll convert
// this to sat/KB as the CreateSimpleTx method requires this unit.
Expand All @@ -529,6 +530,12 @@ func (b *BtcWallet) CreateSimpleTx(outputs []*wire.TxOut,
if len(outputs) < 1 {
return nil, lnwallet.ErrNoOutputs
}

// Sanity check minConfs.
if minConfs < 0 {
return nil, lnwallet.ErrInvalidMinconf
}

for _, output := range outputs {
// When checking an output for things like dusty-ness, we'll
// use the default mempool relay fee rather than the target
Expand All @@ -544,7 +551,7 @@ func (b *BtcWallet) CreateSimpleTx(outputs []*wire.TxOut,
}

return b.wallet.CreateSimpleTx(
nil, defaultAccount, outputs, 1, feeSatPerKB, dryRun,
nil, defaultAccount, outputs, minConfs, feeSatPerKB, dryRun,
)
}

Expand Down Expand Up @@ -608,11 +615,11 @@ func (b *BtcWallet) ReleaseOutput(id wtxmgr.LockID, op wire.OutPoint) error {
}

// ListUnspentWitness returns all unspent outputs which are version 0 witness
// programs. The 'minconfirms' and 'maxconfirms' parameters indicate the minimum
// programs. The 'minConfs' and 'maxConfs' parameters indicate the minimum
// and maximum number of confirmations an output needs in order to be returned
// by this method. Passing -1 as 'minconfirms' indicates that even unconfirmed
// outputs should be returned. Using MaxInt32 as 'maxconfirms' implies returning
// all outputs with at least 'minconfirms'. The account parameter serves as a
// by this method. Passing -1 as 'minConfs' indicates that even unconfirmed
// outputs should be returned. Using MaxInt32 as 'maxConfs' implies returning
// all outputs with at least 'minConfs'. The account parameter serves as a
// filter to retrieve the unspent outputs for a specific account. When empty,
// the unspent outputs of all wallet accounts are returned.
//
Expand Down
16 changes: 8 additions & 8 deletions lnwallet/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ var (
var ErrNoOutputs = errors.New("no outputs")

// ErrInvalidMinconf is returned if we try to create a transaction with
// invalid minconf value.
// invalid minConfs value.
var ErrInvalidMinconf = errors.New("minimum number of confirmations must " +
"be a non-negative number")

Expand Down Expand Up @@ -236,7 +236,7 @@ type WalletController interface {
//
// NOTE: This method requires the global coin selection lock to be held.
SendOutputs(outputs []*wire.TxOut,
feeRate chainfee.SatPerKWeight, minconf int32, label string) (*wire.MsgTx, error)
feeRate chainfee.SatPerKWeight, minConfs int32, label string) (*wire.MsgTx, error)

// CreateSimpleTx creates a Bitcoin transaction paying to the specified
// outputs. The transaction is not broadcasted to the network. In the
Expand All @@ -251,20 +251,20 @@ type WalletController interface {
//
// NOTE: This method requires the global coin selection lock to be held.
CreateSimpleTx(outputs []*wire.TxOut, feeRate chainfee.SatPerKWeight,
dryRun bool) (*txauthor.AuthoredTx, error)
minConfs int32, dryRun bool) (*txauthor.AuthoredTx, error)

// ListUnspentWitness returns all unspent outputs which are version 0
// witness programs. The 'minconfirms' and 'maxconfirms' parameters
// witness programs. The 'minConfs' and 'maxConfs' parameters
// indicate the minimum and maximum number of confirmations an output
// needs in order to be returned by this method. Passing -1 as
// 'minconfirms' indicates that even unconfirmed outputs should be
// returned. Using MaxInt32 as 'maxconfirms' implies returning all
// outputs with at least 'minconfirms'. The account parameter serves as
// 'minConfs' indicates that even unconfirmed outputs should be
// returned. Using MaxInt32 as 'maxConfs' implies returning all
// outputs with at least 'minConfs'. The account parameter serves as
// a filter to retrieve the unspent outputs for a specific account.
// When empty, the unspent outputs of all wallet accounts are returned.
//
// NOTE: This method requires the global coin selection lock to be held.
ListUnspentWitness(minconfirms, maxconfirms int32,
ListUnspentWitness(minConfs, maxConfs int32,
accountFilter string) ([]*Utxo, error)

// ListTransactionDetails returns a list of all transactions which are
Expand Down
109 changes: 84 additions & 25 deletions lnwallet/test/test_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -2547,6 +2547,8 @@ func testLastUnusedAddr(miner *rpctest.Harness,
// testCreateSimpleTx checks that a call to CreateSimpleTx will return a
// transaction that is equal to the one that is being created by SendOutputs in
// a subsequent call.
// All test cases are doubled-up: one for testing unconfirmed inputs,
// one for testing only confirmed inputs.
func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet,
_ *lnwallet.LightningWallet, t *testing.T) {

Expand All @@ -2558,51 +2560,108 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet,

// The test cases we will run through for all backends.
testCases := []struct {
outVals []int64
feeRate chainfee.SatPerKWeight
valid bool
outVals []int64
feeRate chainfee.SatPerKWeight
valid bool
unconfirmed bool
}{
{
outVals: []int64{},
feeRate: 2500,
valid: false, // No outputs.
outVals: []int64{},
feeRate: 2500,
valid: false, // No outputs.
unconfirmed: false,
},
{
outVals: []int64{},
feeRate: 2500,
valid: false, // No outputs.
unconfirmed: true,
},

{
outVals: []int64{200},
feeRate: 2500,
valid: false, // Dust output.
outVals: []int64{200},
feeRate: 2500,
valid: false, // Dust output.
unconfirmed: false,
},
{
outVals: []int64{200},
feeRate: 2500,
valid: false, // Dust output.
unconfirmed: true,
},

{
outVals: []int64{1e8},
feeRate: 2500,
valid: true,
outVals: []int64{1e8},
feeRate: 2500,
valid: true,
unconfirmed: false,
},
{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 2500,
valid: true,
outVals: []int64{1e8},
feeRate: 2500,
valid: true,
unconfirmed: true,
},

{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 12500,
valid: true,
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 2500,
valid: true,
unconfirmed: false,
},
{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 50000,
valid: true,
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 2500,
valid: true,
unconfirmed: true,
},

{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 12500,
valid: true,
unconfirmed: false,
},
{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 12500,
valid: true,
unconfirmed: true,
},

{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 50000,
valid: true,
unconfirmed: false,
},
{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5},
feeRate: 50000,
valid: true,
unconfirmed: true,
},

{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5, 1e8, 2e8,
1e8, 2e7, 3e5},
feeRate: 44250,
valid: true,
feeRate: 44250,
valid: true,
unconfirmed: false,
},
{
outVals: []int64{1e8, 2e8, 1e8, 2e7, 3e5, 1e8, 2e8,
1e8, 2e7, 3e5},
feeRate: 44250,
valid: true,
unconfirmed: true,
},
}

for i, test := range testCases {
var minConfs int32 = 1

feeRate := test.feeRate

// Grab some fresh addresses from the miner that we will send
Expand All @@ -2629,7 +2688,7 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet,

// Now try creating a tx spending to these outputs.
createTx, createErr := w.CreateSimpleTx(
outputs, feeRate, true,
outputs, feeRate, minConfs, true,
)
switch {
case test.valid && createErr != nil:
Expand All @@ -2646,7 +2705,7 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet,
// _very_ similar to the one we just created being sent. The
// only difference is that the dry run tx is not signed, and
// that the change output position might be different.
tx, sendErr := w.SendOutputs(outputs, feeRate, 1, labels.External)
tx, sendErr := w.SendOutputs(outputs, feeRate, minConfs, labels.External)
switch {
case test.valid && sendErr != nil:
t.Fatalf("got unexpected error when sending tx: %v",
Expand Down
Loading

0 comments on commit bfcaf02

Please sign in to comment.