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 reward payment tx fee calculation #427

Merged
merged 6 commits into from
Nov 20, 2023
Merged

Fix reward payment tx fee calculation #427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
be98a5f
pool: Split applyTxFees tests from PaymentMgr test
jholdstock Nov 15, 2023
2942837
pool: Remove unused return value from applyTxFees.
jholdstock Nov 15, 2023
5127213
pool: Remove output total param from applyTxFees.
jholdstock Nov 15, 2023
5072c6c
pool: Simplify and parameterize applyTxFees test.
jholdstock Nov 16, 2023
ffad0c9
pool: Extract fee estimation func.
jholdstock Nov 16, 2023
9b692c8
pool: Fix logic in applyTxFees.
jholdstock Nov 17, 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
74 changes: 54 additions & 20 deletions pool/paymentmgr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"errors"
"fmt"
"math"
"math/big"
"math/rand"
"sync"
Expand Down Expand Up @@ -419,6 +420,21 @@ func (pm *PaymentMgr) pruneOrphanedPayments(ctx context.Context, pmts map[string
return pmts, nil
}

func estimateTxFee(numInputs, numOutputs int) dcrutil.Amount {
inSizes := make([]int, numInputs)
for i := 0; i < numInputs; i++ {
inSizes[i] = txsizes.RedeemP2PKHSigScriptSize
}
outSizes := make([]int, numOutputs)
for i := 0; i < numOutputs; i++ {
outSizes[i] = txsizes.P2PKHOutputSize
}
const changeScriptSize = 0
estSize := txsizes.EstimateSerializeSizeFromScriptSizes(inSizes, outSizes,
changeScriptSize)
return txrules.FeeForSerializeSize(txrules.DefaultRelayFeePerKb, estSize)
}

// applyTxFees determines the transaction fees needed for the payout transaction
// and deducts portions of the fee from outputs of participating accounts
// being paid to.
Expand All @@ -427,44 +443,62 @@ func (pm *PaymentMgr) pruneOrphanedPayments(ctx context.Context, pmts map[string
// the ratio of the amount being paid to the total transaction output minus
// pool fees.
func (pm *PaymentMgr) applyTxFees(inputs []chainjson.TransactionInput, outputs map[string]dcrutil.Amount,
tOut dcrutil.Amount, feeAddr stdaddr.Address) (dcrutil.Amount, dcrutil.Amount, error) {
feeAddr stdaddr.Address) (dcrutil.Amount, error) {
const funcName = "applyTxFees"
if len(inputs) == 0 {
desc := fmt.Sprintf("%s: cannot create a payout transaction "+
"without a tx input", funcName)
return 0, 0, errs.PoolError(errs.TxIn, desc)
return 0, errs.PoolError(errs.TxIn, desc)
}
if len(outputs) == 0 {
desc := fmt.Sprintf("%s: cannot create a payout transaction "+
"without a tx output", funcName)
return 0, 0, errs.PoolError(errs.TxOut, desc)
}
inSizes := make([]int, len(inputs))
for range inputs {
inSizes = append(inSizes, txsizes.RedeemP2PKHSigScriptSize)
return 0, errs.PoolError(errs.TxOut, desc)
}
outSizes := make([]int, len(outputs))
for range outputs {
outSizes = append(outSizes, txsizes.P2PKHOutputSize)

estTxFee := estimateTxFee(len(inputs), len(outputs))

// Determine the total output amount without the pool fee.
var outputAmountSansPoolFee dcrutil.Amount
for addr, v := range outputs {
if addr == feeAddr.String() {
continue
}
outputAmountSansPoolFee += v
}
changeScriptSize := 0
estSize := txsizes.EstimateSerializeSizeFromScriptSizes(inSizes, outSizes,
changeScriptSize)
estFee := txrules.FeeForSerializeSize(txrules.DefaultRelayFeePerKb, estSize)
sansFees := tOut - estFee

// Determine the proportional transaction fees for each output other than
// the pool fee. Note that in order to avoid overpayment of the fee, each
// proportional fee is calculated via a floor. This means there will still
// be a few atoms leftover to account for when the fee is not evenly
// divisible by the number of outputs.
var txFeesPaid dcrutil.Amount
proportionalTxFees := make(map[string]dcrutil.Amount)
for addr, v := range outputs {
// Pool fee payments are excluded from tx fee deductions.
if addr == feeAddr.String() {
continue
}

ratio := float64(int64(sansFees)) / float64(int64(v))
outFee := estFee.MulF64(ratio)
outputs[addr] -= outFee
ratio := float64(v) / float64(outputAmountSansPoolFee)
outFee := dcrutil.Amount(math.Floor(float64(estTxFee) * ratio))
proportionalTxFees[addr] = outFee
txFeesPaid += outFee
}

// Apply the fees to the outputs while accounting for any extra atoms needed
// to reach the required transaction fee by reducing random outputs
// accordingly.
remainingTxFeeAtoms := estTxFee - txFeesPaid
for addr, fee := range proportionalTxFees {
outputs[addr] -= fee
if remainingTxFeeAtoms > 0 {
outputs[addr]--
remainingTxFeeAtoms--
}
}

return sansFees, estFee, nil
return estTxFee, nil
}

// confirmCoinbases ensures the coinbases referenced by the provided
Expand Down Expand Up @@ -800,7 +834,7 @@ func (pm *PaymentMgr) payDividends(ctx context.Context, height uint32, coinbaseI
return err
}

_, estFee, err := pm.applyTxFees(inputs, outputs, tOut, feeAddr)
estFee, err := pm.applyTxFees(inputs, outputs, feeAddr)
if err != nil {
return err
}
Expand Down
229 changes: 172 additions & 57 deletions pool/paymentmgr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,177 @@ func testPaymentMgrMaturity(t *testing.T) {
}
}

func testPaymentMgrApplyTxFees(t *testing.T) {
mgr, _, _ := createPaymentMgr(t, PPS)
feeAddr := poolFeeAddrs.String()

tests := map[string]struct {
// inputs and outputs are the params which are passed into applyTxFees.
inputs []dcrutil.Amount
outputs map[string]dcrutil.Amount
// expectedOutputs are the output amounts which are expected after
// applyTxFees has been run. Note that the fee output should remain
// unchanged - tx fees are only deducted from user outputs.
expectedOutputs map[string]dcrutil.Amount
}{
"Single input, single output": {
inputs: []dcrutil.Amount{
dcrutil.Amount(900000000),
},
outputs: map[string]dcrutil.Amount{
feeAddr: dcrutil.Amount(100000000),
"user1": dcrutil.Amount(800000000),
},
expectedOutputs: map[string]dcrutil.Amount{
feeAddr: dcrutil.Amount(100000000),
"user1": dcrutil.Amount(799997250),
},
},
"Single input, multiple outputs": {
inputs: []dcrutil.Amount{
dcrutil.Amount(500000000),
},
outputs: map[string]dcrutil.Amount{
feeAddr: dcrutil.Amount(50000000),
"user1": dcrutil.Amount(300000000),
"user2": dcrutil.Amount(150000000),
},
expectedOutputs: map[string]dcrutil.Amount{
feeAddr: dcrutil.Amount(50000000),
"user1": dcrutil.Amount(299997854),
"user2": dcrutil.Amount(149998927),
},
},
"Multiple inputs, single output": {
inputs: []dcrutil.Amount{
dcrutil.Amount(500000000),
dcrutil.Amount(300000000),
dcrutil.Amount(200000000),
},
outputs: map[string]dcrutil.Amount{
feeAddr: dcrutil.Amount(100000000),
"user1": dcrutil.Amount(900000000),
},
expectedOutputs: map[string]dcrutil.Amount{
feeAddr: dcrutil.Amount(100000000),
"user1": dcrutil.Amount(899993930),
},
},
"Multiple inputs, multiple outputs": {
inputs: []dcrutil.Amount{
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
dcrutil.Amount(100000000),
},
outputs: map[string]dcrutil.Amount{
feeAddr: dcrutil.Amount(100000000),
"user1": dcrutil.Amount(100000000),
"user2": dcrutil.Amount(100000000),
"user3": dcrutil.Amount(100000000),
"user4": dcrutil.Amount(100000000),
"user5": dcrutil.Amount(100000000),
"user6": dcrutil.Amount(500000000),
},
expectedOutputs: map[string]dcrutil.Amount{
feeAddr: dcrutil.Amount(100000000),
"user1": dcrutil.Amount(99997830),
"user2": dcrutil.Amount(99997830),
"user3": dcrutil.Amount(99997830),
"user4": dcrutil.Amount(99997830),
"user5": dcrutil.Amount(99997830),
"user6": dcrutil.Amount(499989150),
},
},
}

for testName, test := range tests {
t.Run(testName, func(t *testing.T) {
// Create TransactionInputs from the provided input amounts.
txInputs := make([]chainjson.TransactionInput, len(test.inputs))
for i := 0; i < len(test.inputs); i++ {
txInputs[i] = chainjson.TransactionInput{
Amount: float64(test.inputs[i]),
Txid: chainhash.Hash{1}.String(),
Vout: 2,
Tree: wire.TxTreeRegular,
}
}

// Call applyTxFees.
_, err := mgr.applyTxFees(txInputs, test.outputs, poolFeeAddrs)
if err != nil {
t.Fatalf("unexpected applyTxFees error: %v", err)
}

// Validate outputs have been modified as expected.
for addr, amt := range test.outputs {
// If the fee could not be split without rounding, extra atoms
// will be allocated to random outputs.
wantAmt1 := test.expectedOutputs[addr]
wantAmt2 := wantAmt1 - 1
if amt != wantAmt1 && amt != wantAmt2 {
t.Fatalf("expected payment for %s to be %v or %v, got %v",
addr, wantAmt1, wantAmt2, amt)
}
}

// Ensure the fee deduced from outputs matches expectation.
expectedFee := estimateTxFee(len(test.inputs), len(test.outputs))
actualFee := func() dcrutil.Amount {
var tIn dcrutil.Amount
for _, amt := range test.inputs {
tIn += amt
}
var tOut dcrutil.Amount
for _, amt := range test.outputs {
tOut += amt
}
return tIn - tOut
}()

if actualFee != expectedFee {
t.Fatalf("expected total fee to be %v, got %v",
expectedFee, actualFee)
}
})
}
}

// testPaymentMgrApplyTxFeesErrs ensures applyTxFees returns an error if it is
// passed invalid parameters.
func testPaymentMgrApplyTxFeesErrs(t *testing.T) {
mgr, _, _ := createPaymentMgr(t, PPS)

in := chainjson.TransactionInput{
Amount: 100,
Txid: chainhash.Hash{1}.String(),
Vout: 2,
Tree: wire.TxTreeRegular,
}

// Ensure providing no tx inputs triggers an error.
_, err := mgr.applyTxFees([]chainjson.TransactionInput{},
make(map[string]dcrutil.Amount), poolFeeAddrs)
if !errors.Is(err, errs.TxIn) {
t.Fatalf("expected a tx input error, got %v", err)
}

// Ensure providing no tx outputs triggers an error.
_, err = mgr.applyTxFees([]chainjson.TransactionInput{in},
make(map[string]dcrutil.Amount), poolFeeAddrs)
if !errors.Is(err, errs.TxOut) {
t.Fatalf("expected a tx output error, got %v", err)
}
}

func testPaymentMgrPayment(t *testing.T) {
// Insert some test accounts.
accountX := NewAccount(xAddr)
Expand Down Expand Up @@ -561,62 +732,6 @@ func testPaymentMgrPayment(t *testing.T) {
"pruning, got %v", len(pmtSet))
}

// applyTxFee tests.
outV, _ := dcrutil.NewAmount(100)
in := chainjson.TransactionInput{
Amount: float64(outV),
Txid: chainhash.Hash{1}.String(),
Vout: 2,
Tree: wire.TxTreeRegular,
}

poolFeeValue := amt.MulF64(0.1)
xValue := amt.MulF64(0.6)
yValue := amt.MulF64(0.3)

feeAddr := poolFeeAddrs.String()
out := make(map[string]dcrutil.Amount)
out[xAddr] = xValue
out[yAddr] = yValue
out[feeAddr] = poolFeeValue

_, txFee, err := mgr.applyTxFees([]chainjson.TransactionInput{in},
out, outV, poolFeeAddrs)
if err != nil {
t.Fatalf("unexpected applyTxFees error: %v", err)
}

// Ensure the pool fee payment was exempted from tx fee deductions.
if out[feeAddr] != poolFeeValue {
t.Fatalf("expected pool fee payment to be %v, got %v",
poolFeeValue, out[feeAddr])
}

// Ensure the difference between initial account payments and updated
// account payments plus the transaction fee is not more than the
// maximum rounding difference.
initialAccountPayments := xValue + yValue
updatedAccountPaymentsPlusTxFee := out[xAddr] + out[yAddr] + txFee
if initialAccountPayments-updatedAccountPaymentsPlusTxFee <= maxRoundingDiff {
t.Fatalf("initial account payment total %v to be equal to updated "+
"values plus the transaction fee %v", initialAccountPayments,
updatedAccountPaymentsPlusTxFee)
}

// Ensure providing no tx inputs triggers an error.
_, _, err = mgr.applyTxFees([]chainjson.TransactionInput{},
out, outV, poolFeeAddrs)
if !errors.Is(err, errs.TxIn) {
t.Fatalf("expected a tx input error, got %v", err)
}

// Ensure providing no tx outputs triggers an error.
_, _, err = mgr.applyTxFees([]chainjson.TransactionInput{in},
make(map[string]dcrutil.Amount), outV, poolFeeAddrs)
if !errors.Is(err, errs.TxOut) {
t.Fatalf("expected a tx output error, got %v", err)
}

// confirmCoinbases tests.
txHashes := make(map[chainhash.Hash]uint32)
hashA := chainhash.Hash{'a'}
Expand Down Expand Up @@ -875,7 +990,7 @@ func testPaymentMgrPayment(t *testing.T) {

for addr := range outputs {
var match bool
if addr == feeAddr || addr == xAddr || addr == yAddr {
if addr == poolFeeAddrs.String() || addr == xAddr || addr == yAddr {
match = true
}
if !match {
Expand Down
Loading
Loading