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.

Sign up for GitHub

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

Jump to bottom

Fix Fee-Calculation when Selecting Inputs #844

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix Fee-Calculation when Selecting Inputs #844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78502da
txauthor: new input selection logic
ziggie1984 Feb 13, 2023
a48faa2
wallet: adopting changes for the new input selection
ziggie1984 Feb 13, 2023
40abbc7
cmd: new input selection changes are adopted.
ziggie1984 Feb 13, 2023
f109218
mod: use own txauthor version
ziggie1984 Feb 16, 2023
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
30 changes: 15 additions & 15 deletions cmd/sweepaccount/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/btcsuite/btcwallet/wallet/txauthor"
"github.com/btcsuite/btcwallet/wallet/txrules"
"github.com/btcsuite/btcwallet/wallet/txsizes"
"github.com/btcsuite/btcwallet/wtxmgr"
"github.com/jessevdk/go-flags"
)

Expand Down Expand Up @@ -133,16 +134,11 @@ type noInputValue struct {

func (noInputValue) Error() string { return "no input value" }

// makeInputSource creates an InputSource that creates inputs for every unspent
// output with non-zero output values. The target amount is ignored since every
// output is consumed. The InputSource does not return any previous output
// scripts as they are not needed for creating the unsinged transaction and are
// looked up again by the wallet during the call to signrawtransaction.
func makeInputSource(outputs []btcjson.ListUnspentResult) txauthor.InputSource {
// fetchInputs fetches every unspent output with non-zero output values.
func fetchInputs(outputs []btcjson.ListUnspentResult) ([]wtxmgr.Credit, error) {
var (
totalInputValue btcutil.Amount
inputs = make([]*wire.TxIn, 0, len(outputs))
inputValues = make([]btcutil.Amount, 0, len(outputs))
inputs = make([]wtxmgr.Credit, 0, len(outputs))
sourceErr error
)
for _, output := range outputs {
Expand Down Expand Up @@ -174,17 +170,18 @@ func makeInputSource(outputs []btcjson.ListUnspentResult) txauthor.InputSource {
break
}

inputs = append(inputs, wire.NewTxIn(&previousOutPoint, nil, nil))
inputValues = append(inputValues, outputAmount)
inputs = append(inputs, wtxmgr.Credit{
OutPoint: previousOutPoint,
Amount: outputAmount,
})
}

if sourceErr == nil && totalInputValue == 0 {
sourceErr = noInputValue{}
}

return func(btcutil.Amount) (btcutil.Amount, []*wire.TxIn, []btcutil.Amount, [][]byte, error) {
return totalInputValue, inputs, inputValues, nil, sourceErr
}
return inputs, sourceErr

}

// makeDestinationScriptSource creates a ChangeSource which is used to receive
Expand Down Expand Up @@ -277,10 +274,13 @@ func sweep() error {
numErrors++
}
for _, previousOutputs := range sourceOutputs {
inputSource := makeInputSource(previousOutputs)
inputs, err := fetchInputs(previousOutputs)
if err != nil {
return err
}
destinationSource := makeDestinationScriptSource(rpcClient, opts.DestinationAccount)
tx, err := txauthor.NewUnsignedTransaction(nil, opts.FeeRate.Amount,
Copy link

Choose a reason for hiding this comment

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

Did you test the sweep action to see if you have any regression? (as there is no test for it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point

inputSource, destinationSource)
inputs, txauthor.ConstantSelection, destinationSource)
if err != nil {
if err != (noInputValue{}) {
reportError("Failed to create unsigned transaction: %v", err)
Expand Down
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,5 @@ require (
)

go 1.16

replace github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2 => github.com/ziggie1984/btcwallet/wallet/txauthor v1.2.4-0.20230216141549-78502da359d1
5 changes: 2 additions & 3 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,9 @@ github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA=
github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
github.com/btcsuite/btcwallet/wallet/txauthor v1.2.3/go.mod h1:T2xSiKGpUkSLCh68aF+FMXmKK9mFqNdHl9VaqOr+JjU=
github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2 h1:etuLgGEojecsDOYTII8rYiGHjGyV5xTqsXi+ZQ715UU=
github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2/go.mod h1:Zpk/LOb2sKqwP2lmHjaZT9AdaKsHPSbNLm2Uql5IQ/0=
github.com/btcsuite/btcwallet/wallet/txrules v1.2.0 h1:BtEN5Empw62/RVnZ0VcJaVtVlBijnLlJY+dwjAye2Bg=
github.com/btcsuite/btcwallet/wallet/txrules v1.2.0/go.mod h1:AtkqiL7ccKWxuLYtZm8Bu8G6q82w4yIZdgq6riy60z0=
github.com/btcsuite/btcwallet/wallet/txsizes v1.1.0/go.mod h1:pauEU8UuMFiThe5PB3EO+gO5kx87Me5NvdQDsTuq6cs=
github.com/btcsuite/btcwallet/wallet/txsizes v1.2.2/go.mod h1:q08Rms52VyWyXcp5zDc4tdFRKkFgNsMQrv3/LvE1448=
github.com/btcsuite/btcwallet/wallet/txsizes v1.2.3 h1:PszOub7iXVYbtGybym5TGCp9Dv1h1iX4rIC3HICZGLg=
github.com/btcsuite/btcwallet/wallet/txsizes v1.2.3/go.mod h1:q08Rms52VyWyXcp5zDc4tdFRKkFgNsMQrv3/LvE1448=
github.com/btcsuite/btcwallet/walletdb v1.3.5/go.mod h1:oJDxAEUHVtnmIIBaa22wSBPTVcs6hUp5NKWmI8xDwwU=
Expand Down Expand Up @@ -130,6 +127,8 @@ github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PK
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 h1:epCh84lMvA70Z7CTTCmYQn2CKbY8j86K7/FAIr141uY=
github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7/go.mod h1:q4W45IWZaF22tdD+VEXcAWRA037jwmWEB5VWYORlTpc=
github.com/ziggie1984/btcwallet/wallet/txauthor v1.2.4-0.20230216141549-78502da359d1 h1:JZfmiAtJUf8tmSkAajJ8lyUTIHSQaRfJQwgSO1X8v0I=
github.com/ziggie1984/btcwallet/wallet/txauthor v1.2.4-0.20230216141549-78502da359d1/go.mod h1:k8f2FcgUQUdfpkY98vWvDYpXLb83g17EupcLv1KcNJw=
go.etcd.io/bbolt v1.3.5-0.20200615073812-232d8fc87f50 h1:ASw9n1EHMftwnP3Az4XW6e308+gNsrHzmdhd0Olz9Hs=
go.etcd.io/bbolt v1.3.5-0.20200615073812-232d8fc87f50/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ=
golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
Expand Down
35 changes: 6 additions & 29 deletions wallet/createtx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,6 @@ func (s byAmount) Len() int { return len(s) }
func (s byAmount) Less(i, j int) bool { return s[i].Amount < s[j].Amount }
func (s byAmount) Swap(i, j int) { s[i], s[j] = s[j], s[i] }

func makeInputSource(eligible []wtxmgr.Credit) txauthor.InputSource {
// Current inputs and their total value. These are closed over by the
// returned input source and reused across multiple calls.
currentTotal := btcutil.Amount(0)
currentInputs := make([]*wire.TxIn, 0, len(eligible))
currentScripts := make([][]byte, 0, len(eligible))
currentInputValues := make([]btcutil.Amount, 0, len(eligible))

return func(target btcutil.Amount) (btcutil.Amount, []*wire.TxIn,
[]btcutil.Amount, [][]byte, error) {

for currentTotal < target && len(eligible) != 0 {
nextCredit := &eligible[0]
eligible = eligible[1:]
nextInput := wire.NewTxIn(&nextCredit.OutPoint, nil, nil)
currentTotal += nextCredit.Amount
currentInputs = append(currentInputs, nextInput)
currentScripts = append(currentScripts, nextCredit.PkScript)
currentInputValues = append(currentInputValues, nextCredit.Amount)
}
return currentTotal, currentInputs, currentInputValues, currentScripts, nil
}
}

// secretSource is an implementation of txauthor.SecretSource for the wallet's
// address manager.
type secretSource struct {
Expand Down Expand Up @@ -139,13 +115,14 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut,
return err
}

var inputSource txauthor.InputSource
// We need to define the Selection Strategy
var inputSelectionStrategy txauthor.InputSelectionStrategy

switch coinSelectionStrategy {
// Pick largest outputs first.
case CoinSelectionLargest:
sort.Sort(sort.Reverse(byAmount(eligible)))
inputSource = makeInputSource(eligible)
inputSelectionStrategy = txauthor.PositiveYieldingSelection

// Select coins at random. This prevents the creation of ever
// smaller utxos over time that may never become economical to
Expand All @@ -170,12 +147,12 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut,
positivelyYielding[i], positivelyYielding[j] =
positivelyYielding[j], positivelyYielding[i]
})

inputSource = makeInputSource(positivelyYielding)
inputSelectionStrategy = txauthor.RandomSelection
eligible = positivelyYielding
}

tx, err = txauthor.NewUnsignedTransaction(
outputs, feeSatPerKb, inputSource, changeSource,
outputs, feeSatPerKb, eligible, inputSelectionStrategy, changeSource,
)
if err != nil {
return err
Expand Down
38 changes: 5 additions & 33 deletions wallet/psbt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, keyScope *waddrmgr.KeyScope,
PkScript: utxo.PkScript,
}
}
inputSource := constantInputSource(credits)

// Build the TxCreateOption to retrieve the change scope.
opts := defaultTxCreateOptions()
Expand All @@ -197,25 +196,25 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, keyScope *waddrmgr.KeyScope,
dbtx, opts.changeKeyScope, account,
)
if err != nil {
return err
return fmt.Errorf("could not add change address to "+
"database: %v", err)
}

// Ask the txauthor to create a transaction with our
// selected coins. This will perform fee estimation and
// add a change output if necessary.
tx, err = txauthor.NewUnsignedTransaction(
txOut, feeSatPerKB, inputSource, changeSource,
txOut, feeSatPerKB, credits, txauthor.ConstantSelection, changeSource,
)
if err != nil {
return fmt.Errorf("fee estimation not "+
"successful: %v", err)
"successful: %w", err)
}

return nil
})
if err != nil {
return 0, fmt.Errorf("could not add change address to "+
"database: %v", err)
return 0, err
}
}

Expand Down Expand Up @@ -544,30 +543,3 @@ func PsbtPrevOutputFetcher(packet *psbt.Packet) *txscript.MultiPrevOutFetcher {

return fetcher
}

// constantInputSource creates an input source function that always returns the
// static set of user-selected UTXOs.
func constantInputSource(eligible []wtxmgr.Credit) txauthor.InputSource {
// Current inputs and their total value. These won't change over
// different invocations as we want our inputs to remain static since
// they're selected by the user.
currentTotal := btcutil.Amount(0)
currentInputs := make([]*wire.TxIn, 0, len(eligible))
currentScripts := make([][]byte, 0, len(eligible))
currentInputValues := make([]btcutil.Amount, 0, len(eligible))

for _, credit := range eligible {
nextInput := wire.NewTxIn(&credit.OutPoint, nil, nil)
currentTotal += credit.Amount
currentInputs = append(currentInputs, nextInput)
currentScripts = append(currentScripts, credit.PkScript)
currentInputValues = append(currentInputValues, credit.Amount)
}

return func(target btcutil.Amount) (btcutil.Amount, []*wire.TxIn,
[]btcutil.Amount, [][]byte, error) {

return currentTotal, currentInputs, currentInputValues,
currentScripts, nil
}
}
Loading