Skip to content
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

Support spend_unconfirmed in EstimateFee and FundPsbt #4905

Merged
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
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;
mrfelton marked this conversation as resolved.
Show resolved Hide resolved

// 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,
mrfelton marked this conversation as resolved.
Show resolved Hide resolved
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)
mrfelton marked this conversation as resolved.
Show resolved Hide resolved

// 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