Skip to content

Commit

Permalink
multi: add coin selection strategy option to all on-chain rpcs
Browse files Browse the repository at this point in the history
In this commit, we add the coin selection strategy option to the following
on-chain RPCs `fundpsbt`, `batchopenchannel`, `estimatefee`, `sendcoins`,
`sendmany`, and `sendoutputs`.
  • Loading branch information
mohamedawnallah committed Apr 1, 2024
1 parent a0b0e7a commit 1a2d50d
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 40 deletions.
7 changes: 4 additions & 3 deletions funding/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,10 @@ func (b *Batcher) BatchFund(ctx context.Context,
Fees: &walletrpc.FundPsbtRequest_SatPerVbyte{
SatPerVbyte: uint64(feeRateSatPerVByte),
},
MinConfs: firstReq.MinConfs,
SpendUnconfirmed: firstReq.MinConfs == 0,
ChangeType: changeType,
MinConfs: firstReq.MinConfs,
SpendUnconfirmed: firstReq.MinConfs == 0,
ChangeType: changeType,
CoinSelectionStrategy: req.CoinSelectionStrategy,
}
fundPsbtResp, err := b.cfg.WalletKitServer.FundPsbt(ctx, fundPsbtReq)
if err != nil {
Expand Down
37 changes: 26 additions & 11 deletions lnrpc/walletrpc/walletkit_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -799,12 +799,19 @@ func (w *WalletKit) SendOutputs(ctx context.Context,
return nil, err
}

coinSelectionStrategy, err := lnrpc.UnmarshallCoinSelectionStrategy(
req.CoinSelectionStrategy, w.cfg.CoinSelectionStrategy,
)
if err != nil {
return nil, err
}

// Now that we have the outputs mapped and checked for the reserve
// requirement, we can request that the wallet attempts to create this
// transaction.
tx, err := w.cfg.Wallet.SendOutputs(
outputsToCreate, chainfee.SatPerKWeight(req.SatPerKw), minConfs,
label,
label, coinSelectionStrategy,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -1173,12 +1180,15 @@ func (w *WalletKit) LabelTransaction(ctx context.Context,
func (w *WalletKit) FundPsbt(_ context.Context,
req *FundPsbtRequest) (*FundPsbtResponse, error) {

var (
err error
feeSatPerKW chainfee.SatPerKWeight
coinSelectionStrategy, err := lnrpc.UnmarshallCoinSelectionStrategy(
req.CoinSelectionStrategy, w.cfg.CoinSelectionStrategy,
)
if err != nil {
return nil, err
}

// Determine the desired transaction fee.
var feeSatPerKW chainfee.SatPerKWeight
switch {
// Estimate the fee by the target number of blocks to confirmation.
case req.GetTargetConf() != 0:
Expand Down Expand Up @@ -1241,7 +1251,7 @@ func (w *WalletKit) FundPsbt(_ context.Context,
// wallet.
return w.fundPsbtInternalWallet(
account, keyScopeFromChangeAddressType(req.ChangeType),
packet, minConfs, feeSatPerKW,
packet, minConfs, feeSatPerKW, coinSelectionStrategy,
)

// The template is specified as a PSBT with the intention to perform
Expand Down Expand Up @@ -1317,7 +1327,7 @@ func (w *WalletKit) FundPsbt(_ context.Context,
// coin selection algorithm.
return w.fundPsbtCoinSelect(
account, changeIndex, packet, minConfs, changeType,
feeSatPerKW,
feeSatPerKW, coinSelectionStrategy,
)

// The template is specified as a RPC message. We need to create a new
Expand Down Expand Up @@ -1374,7 +1384,7 @@ func (w *WalletKit) FundPsbt(_ context.Context,
// wallet.
return w.fundPsbtInternalWallet(
account, keyScopeFromChangeAddressType(req.ChangeType),
packet, minConfs, feeSatPerKW,
packet, minConfs, feeSatPerKW, coinSelectionStrategy,
)

default:
Expand All @@ -1387,7 +1397,8 @@ func (w *WalletKit) FundPsbt(_ context.Context,
// wallet that does not allow specifying custom inputs while selecting coins.
func (w *WalletKit) fundPsbtInternalWallet(account string,
keyScope *waddrmgr.KeyScope, packet *psbt.Packet, minConfs int32,
feeSatPerKW chainfee.SatPerKWeight) (*FundPsbtResponse, error) {
feeSatPerKW chainfee.SatPerKWeight,
strategy base.CoinSelectionStrategy) (*FundPsbtResponse, error) {

// 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
Expand Down Expand Up @@ -1419,7 +1430,8 @@ func (w *WalletKit) fundPsbtInternalWallet(account string,
// lock any coins but might still change the wallet DB by
// generating a new change address.
changeIndex, err := w.cfg.Wallet.FundPsbt(
packet, minConfs, feeSatPerKW, account, keyScope,
packet, minConfs, feeSatPerKW, account,
keyScope, strategy,
)
if err != nil {
return fmt.Errorf("wallet couldn't fund PSBT: %w", err)
Expand Down Expand Up @@ -1454,10 +1466,13 @@ func (w *WalletKit) fundPsbtInternalWallet(account string,
// fundPsbtCoinSelect uses the "new" PSBT funding method using the channel
// funding coin selection algorithm that allows specifying custom inputs while
// selecting coins.
//
//nolint:funlen
func (w *WalletKit) fundPsbtCoinSelect(account string, changeIndex int32,
packet *psbt.Packet, minConfs int32,
changeType chanfunding.ChangeAddressType,
feeRate chainfee.SatPerKWeight) (*FundPsbtResponse, error) {
feeRate chainfee.SatPerKWeight, strategy base.CoinSelectionStrategy) (
*FundPsbtResponse, error) {

// We want to make sure we don't select any inputs that are already
// specified in the template. To do that, we require those inputs to
Expand Down Expand Up @@ -1603,7 +1618,7 @@ func (w *WalletKit) fundPsbtCoinSelect(account string, changeIndex int32,

selectedCoins, changeAmount, err := chanfunding.CoinSelect(
feeRate, fundingAmount, changeDustLimit, coins,
w.cfg.CoinSelectionStrategy, estimator, changeType,
strategy, estimator, changeType,
)
if err != nil {
return fmt.Errorf("error selecting coins: %w", err)
Expand Down
1 change: 1 addition & 0 deletions lnrpc/walletrpc/walletkit_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
resp, err := rpcServer.fundPsbtCoinSelect(
"", tc.changeIndex, copiedPacket, 0,
tc.changeType, tc.feeRate,
rpcServer.cfg.CoinSelectionStrategy,
)

switch {
Expand Down
8 changes: 5 additions & 3 deletions lntest/mock/walletcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,16 @@ func (w *WalletController) ImportTaprootScript(waddrmgr.KeyScope,

// SendOutputs currently returns dummy values.
func (w *WalletController) SendOutputs([]*wire.TxOut,
chainfee.SatPerKWeight, int32, string) (*wire.MsgTx, error) {
chainfee.SatPerKWeight, int32, string,
base.CoinSelectionStrategy) (*wire.MsgTx, error) {

return nil, nil
}

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

return nil, nil
}
Expand Down Expand Up @@ -206,7 +208,7 @@ func (w *WalletController) ListLeasedOutputs() ([]*base.ListLeasedOutputResult,

// FundPsbt currently does nothing.
func (w *WalletController) FundPsbt(*psbt.Packet, int32, chainfee.SatPerKWeight,
string, *waddrmgr.KeyScope) (int32, error) {
string, *waddrmgr.KeyScope, base.CoinSelectionStrategy) (int32, error) {

return 0, nil
}
Expand Down
9 changes: 5 additions & 4 deletions lnwallet/btcwallet/btcwallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -978,8 +978,8 @@ func (b *BtcWallet) ImportTaprootScript(scope waddrmgr.KeyScope,
//
// This is a part of the WalletController interface.
func (b *BtcWallet) SendOutputs(outputs []*wire.TxOut,
feeRate chainfee.SatPerKWeight, minConfs int32,
label string) (*wire.MsgTx, error) {
feeRate chainfee.SatPerKWeight, minConfs int32, label string,
strategy base.CoinSelectionStrategy) (*wire.MsgTx, error) {

// Convert our fee rate from sat/kw to sat/kb since it's required by
// SendOutputs.
Expand All @@ -997,7 +997,7 @@ func (b *BtcWallet) SendOutputs(outputs []*wire.TxOut,

return b.wallet.SendOutputs(
outputs, nil, defaultAccount, minConfs, feeSatPerKB,
b.cfg.CoinSelectionStrategy, label,
strategy, label,
)
}

Expand All @@ -1016,6 +1016,7 @@ 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, minConfs int32,
strategy base.CoinSelectionStrategy,
dryRun bool) (*txauthor.AuthoredTx, error) {

// The fee rate is passed in using units of sat/kw, so we'll convert
Expand Down Expand Up @@ -1048,7 +1049,7 @@ func (b *BtcWallet) CreateSimpleTx(outputs []*wire.TxOut,

return b.wallet.CreateSimpleTx(
nil, defaultAccount, outputs, minConfs, feeSatPerKB,
b.cfg.CoinSelectionStrategy, dryRun,
strategy, dryRun,
)
}

Expand Down
5 changes: 3 additions & 2 deletions lnwallet/btcwallet/psbt.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ var (
// This is a part of the WalletController interface.
func (b *BtcWallet) FundPsbt(packet *psbt.Packet, minConfs int32,
feeRate chainfee.SatPerKWeight, accountName string,
changeScope *waddrmgr.KeyScope) (int32, error) {
changeScope *waddrmgr.KeyScope,
strategy wallet.CoinSelectionStrategy) (int32, 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 Down Expand Up @@ -134,7 +135,7 @@ func (b *BtcWallet) FundPsbt(packet *psbt.Packet, minConfs int32,
// the partial TX information in the packet.
return b.wallet.FundPsbt(
packet, keyScope, minConfs, accountNum, feeSatPerKB,
b.cfg.CoinSelectionStrategy, opts...,
strategy, opts...,
)
}

Expand Down
9 changes: 6 additions & 3 deletions lnwallet/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,8 @@ type WalletController interface {
//
// NOTE: This method requires the global coin selection lock to be held.
SendOutputs(outputs []*wire.TxOut, feeRate chainfee.SatPerKWeight,
minConfs int32, label string) (*wire.MsgTx, error)
minConfs int32, label string,
strategy base.CoinSelectionStrategy) (*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 @@ -360,7 +361,8 @@ type WalletController interface {
//
// NOTE: This method requires the global coin selection lock to be held.
CreateSimpleTx(outputs []*wire.TxOut, feeRate chainfee.SatPerKWeight,
minConfs int32, dryRun bool) (*txauthor.AuthoredTx, error)
minConfs int32, strategy base.CoinSelectionStrategy,
dryRun bool) (*txauthor.AuthoredTx, error)

// GetTransactionDetails returns a detailed description of a transaction
// given its transaction hash.
Expand Down Expand Up @@ -465,7 +467,8 @@ type WalletController interface {
// to lock the inputs before handing them out.
FundPsbt(packet *psbt.Packet, minConfs int32,
feeRate chainfee.SatPerKWeight, account string,
changeScope *waddrmgr.KeyScope) (int32, error)
changeScope *waddrmgr.KeyScope,
strategy base.CoinSelectionStrategy) (int32, error)

// SignPsbt expects a partial transaction with all inputs and outputs
// fully declared and tries to sign all unsigned inputs that have all
Expand Down
9 changes: 6 additions & 3 deletions lnwallet/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,16 @@ func (w *mockWalletController) ImportTaprootScript(waddrmgr.KeyScope,

// SendOutputs currently returns dummy values.
func (w *mockWalletController) SendOutputs([]*wire.TxOut,
chainfee.SatPerKWeight, int32, string) (*wire.MsgTx, error) {
chainfee.SatPerKWeight, int32, string,
base.CoinSelectionStrategy) (*wire.MsgTx, error) {

return nil, nil
}

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

return nil, nil
}
Expand Down Expand Up @@ -214,7 +216,8 @@ func (w *mockWalletController) ListLeasedOutputs() (

// FundPsbt currently does nothing.
func (w *mockWalletController) FundPsbt(*psbt.Packet, int32,
chainfee.SatPerKWeight, string, *waddrmgr.KeyScope) (int32, error) {
chainfee.SatPerKWeight, string, *waddrmgr.KeyScope,
base.CoinSelectionStrategy) (int32, error) {

return 0, nil
}
Expand Down
6 changes: 3 additions & 3 deletions lnwallet/rpcwallet/rpcwallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ func (r *RPCKeyRing) NewAddress(addrType lnwallet.AddressType, change bool,
//
// NOTE: This method only signs with BIP49/84 keys.
func (r *RPCKeyRing) SendOutputs(outputs []*wire.TxOut,
feeRate chainfee.SatPerKWeight, minConfs int32,
label string) (*wire.MsgTx, error) {
feeRate chainfee.SatPerKWeight, minConfs int32, label string,
strategy basewallet.CoinSelectionStrategy) (*wire.MsgTx, error) {

tx, err := r.WalletController.SendOutputs(
outputs, feeRate, minConfs, label,
outputs, feeRate, minConfs, label, strategy,
)
if err != nil && err != basewallet.ErrTxUnsigned {
return nil, err
Expand Down
12 changes: 11 additions & 1 deletion lnwallet/test/test_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ func sendCoins(t *testing.T, miner *rpctest.Harness,

tx, err := sender.SendOutputs(
[]*wire.TxOut{output}, feeRate, minConf, labels.External,
sender.Cfg.CoinSelectionStrategy,
)
require.NoError(t, err, "unable to send transaction")

Expand Down Expand Up @@ -1189,6 +1190,7 @@ func testListTransactionDetails(miner *rpctest.Harness,
burnOutput := wire.NewTxOut(outputAmt, outputScript)
burnTX, err := alice.SendOutputs(
[]*wire.TxOut{burnOutput}, 2500, 1, labels.External,
alice.Cfg.CoinSelectionStrategy,
)
require.NoError(t, err, "unable to create burn tx")
burnTXID := burnTX.TxHash()
Expand Down Expand Up @@ -1448,6 +1450,7 @@ func testTransactionSubscriptions(miner *rpctest.Harness,
burnOutput := wire.NewTxOut(outputAmt, outputScript)
tx, err := alice.SendOutputs(
[]*wire.TxOut{burnOutput}, 2500, 1, labels.External,
alice.Cfg.CoinSelectionStrategy,
)
require.NoError(t, err, "unable to create tx")
txid := tx.TxHash()
Expand Down Expand Up @@ -1636,6 +1639,7 @@ func newTx(t *testing.T, r *rpctest.Harness, pubKey *btcec.PublicKey,
}
tx, err := alice.SendOutputs(
[]*wire.TxOut{newOutput}, 2500, 1, labels.External,
alice.Cfg.CoinSelectionStrategy,
)
require.NoError(t, err, "unable to create output")

Expand Down Expand Up @@ -1951,6 +1955,7 @@ func testSignOutputUsingTweaks(r *rpctest.Harness,
}
tx, err := alice.SendOutputs(
[]*wire.TxOut{newOutput}, 2500, 1, labels.External,
alice.Cfg.CoinSelectionStrategy,
)
if err != nil {
t.Fatalf("unable to create output: %v", err)
Expand Down Expand Up @@ -2069,6 +2074,7 @@ func testReorgWalletBalance(r *rpctest.Harness, w *lnwallet.LightningWallet,
}
tx, err := w.SendOutputs(
[]*wire.TxOut{output}, 2500, 1, labels.External,
w.Cfg.CoinSelectionStrategy,
)
require.NoError(t, err, "unable to send outputs")
txid := tx.TxHash()
Expand Down Expand Up @@ -2293,6 +2299,7 @@ func testSpendUnconfirmed(miner *rpctest.Harness,
}
_, err = bob.SendOutputs(
[]*wire.TxOut{output}, txFeeRate, 0, labels.External,
bob.Cfg.CoinSelectionStrategy,
)
if err == nil {
t.Fatalf("should have not been able to pay due to insufficient balance: %v", err)
Expand All @@ -2319,6 +2326,7 @@ func testSpendUnconfirmed(miner *rpctest.Harness,
// using confirmed outputs only.
_, err = bob.SendOutputs(
[]*wire.TxOut{output}, txFeeRate, 1, labels.External,
bob.Cfg.CoinSelectionStrategy,
)
if err == nil {
t.Fatalf("should have not been able to pay due to insufficient balance: %v", err)
Expand Down Expand Up @@ -2559,7 +2567,8 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet,

// Now try creating a tx spending to these outputs.
createTx, createErr := w.CreateSimpleTx(
outputs, feeRate, minConfs, true,
outputs, feeRate, minConfs,
w.Cfg.CoinSelectionStrategy, true,
)
switch {
case test.valid && createErr != nil:
Expand All @@ -2578,6 +2587,7 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet,
// that the change output position might be different.
tx, sendErr := w.SendOutputs(
outputs, feeRate, minConfs, labels.External,
w.Cfg.CoinSelectionStrategy,
)
switch {
case test.valid && sendErr != nil:
Expand Down
Loading

0 comments on commit 1a2d50d

Please sign in to comment.