From dc0351646ff7b4eb5a6776be3d4f531643f03161 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Jun 2022 21:47:36 -0500 Subject: [PATCH] mempool: Explicitly reject standalone treasurybase. This updates the mempool to explicitly reject standalone treasurybases and adds a test to ensure proper functionality. Note that treasurybases are already rejected because IsCoinBaseTx currently detects a treasurybase as a coinbase as well, but it is safer to be explicit in case the coinbase detection function is updated in the future to exclude treasurybases. Also since treasurybases are rejected early, remove the code that deals with special casing them later given there is no way the transaction is one. --- internal/mempool/error.go | 4 ++ internal/mempool/error_test.go | 1 + internal/mempool/mempool.go | 49 ++++++++++++---------- internal/mempool/mempool_test.go | 72 ++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 23 deletions(-) diff --git a/internal/mempool/error.go b/internal/mempool/error.go index 31c71d80..edc0d6ae 100644 --- a/internal/mempool/error.go +++ b/internal/mempool/error.go @@ -38,6 +38,10 @@ const ( // ErrCoinbase indicates a transaction is a standalone coinbase transaction. ErrCoinbase = ErrorKind("ErrCoinbase") + // ErrTreasurybase indicates a transaction is a standalone treasurybase + // transaction. + ErrTreasurybase = ErrorKind("ErrTreasurybase") + // ErrExpired indicates a transaction will be expired as of the next block. ErrExpired = ErrorKind("ErrExpired") diff --git a/internal/mempool/error_test.go b/internal/mempool/error_test.go index af295f5c..6a5ac038 100644 --- a/internal/mempool/error_test.go +++ b/internal/mempool/error_test.go @@ -22,6 +22,7 @@ func TestErrorKindStringer(t *testing.T) { {ErrAlreadyVoted, "ErrorAlreadyVoted"}, {ErrDuplicate, "ErrDuplicate"}, {ErrCoinbase, "ErrCoinbase"}, + {ErrTreasurybase, "ErrTreasurybase"}, {ErrExpired, "ErrExpired"}, {ErrNonStandard, "ErrNonStandard"}, {ErrDustOutput, "ErrDustOutput"}, diff --git a/internal/mempool/mempool.go b/internal/mempool/mempool.go index 138d579a..2c5015fc 100644 --- a/internal/mempool/mempool.go +++ b/internal/mempool/mempool.go @@ -1247,6 +1247,24 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, isAutoRevocationsEnabled := checkTxFlags.IsAutoRevocationsEnabled() isSubsidyEnabled := checkTxFlags.IsSubsidySplitEnabled() + // Determine the type of transaction (regular or stake) and be sure to set + // the transaction tree correctly as it's possible a user submitted it to + // the network with TxTreeUnknown. + txType := stake.DetermineTxType(msgTx) + tree := wire.TxTreeRegular + if txType != stake.TxTypeRegular { + tree = wire.TxTreeStake + } + tx.SetTree(tree) + + // A standalone transaction must not be a treasurybase transaction. + isTreasurybase := isTreasuryEnabled && txType == stake.TxTypeTreasuryBase + if isTreasurybase { + str := fmt.Sprintf("transaction %v is an individual treasurybase", + txHash) + return nil, txRuleError(ErrTreasurybase, str) + } + // A standalone transaction must not be a coinbase transaction. if standalone.IsCoinBaseTx(msgTx, isTreasuryEnabled) { str := fmt.Sprintf("transaction %v is an individual coinbase", @@ -1267,29 +1285,14 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, return nil, txRuleError(ErrExpired, str) } - // Determine what type of transaction we're dealing with (regular or stake). - // Then, be sure to set the tx tree correctly as it's possible a user submitted - // it to the network with TxTreeUnknown. - txType := stake.DetermineTxType(msgTx) - tree := wire.TxTreeRegular - if txType != stake.TxTypeRegular { - tree = wire.TxTreeStake - } - tx.SetTree(tree) + // Reject votes and treasury spends before stake validation height. isVote := txType == stake.TxTypeSSGen - - var isTreasuryBase, isTSpend bool - if isTreasuryEnabled { - isTSpend = txType == stake.TxTypeTSpend - isTreasuryBase = txType == stake.TxTypeTreasuryBase - } - - // Reject votes before stake validation height. + isTSpend := isTreasuryEnabled && txType == stake.TxTypeTSpend stakeValidationHeight := mp.cfg.ChainParams.StakeValidationHeight if (isVote || isTSpend) && nextBlockHeight < stakeValidationHeight { strType := "votes" if isTSpend { - strType = "tspends" + strType = "treasury spends" } str := fmt.Sprintf("%s are not valid until block height %d (next "+ "block height %d)", strType, stakeValidationHeight, nextBlockHeight) @@ -1428,12 +1431,12 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, utxoView.RemoveEntry(outpoint) } - // Transaction is an orphan if any of the referenced transaction outputs don't - // exist or are already spent. + // Transaction is an orphan if any of the referenced transaction outputs + // don't exist or are already spent. var missingParents []*chainhash.Hash var updateFraudProof bool for i, txIn := range msgTx.TxIn { - if (i == 0 && (isVote || isTreasuryBase)) || isTSpend { + if (i == 0 && isVote) || isTSpend { continue } @@ -1489,8 +1492,8 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, msgTx = tx.MsgTx() for i, txIn := range msgTx.TxIn { - // Skip stakebase inputs, treasury base inputs, and tspends. - if (i == 0 && (isVote || isTreasuryBase)) || isTSpend { + // Skip stakebase inputs and treasury spends. + if (i == 0 && isVote) || isTSpend { continue } diff --git a/internal/mempool/mempool_test.go b/internal/mempool/mempool_test.go index 61968a49..9d903f96 100644 --- a/internal/mempool/mempool_test.go +++ b/internal/mempool/mempool_test.go @@ -2714,6 +2714,78 @@ func TestHandlesTAdds(t *testing.T) { acceptTAdd(tadd) } +// TestRejectTreasurybases ensures that treasurybases are rejected from the pool +// as expected. +func TestRejectTreasurybases(t *testing.T) { + t.Parallel() + + harness, _, err := newPoolHarness(chaincfg.MainNetParams()) + if err != nil { + t.Fatalf("unable to create test pool: %v", err) + } + tc := &testContext{t, harness} + subsidyCache := harness.txPool.cfg.SubsidyCache + + // Setup the harness for the test and activate the treasury agenda. + harness.SetTreasuryAgendaActive(true) + + // createTreasurybase returns a valid treasurybase transaction for the given + // block height. + createTreasurybase := func(blockHeight int64) (*dcrutil.Tx, error) { + // Create provably pruneable script for the output that encodes the + // block height used to ensure a unique overall transaction hash. This + // is necessary because neither the input nor the output that adds to + // the treasury account balance are unique for a treasurybase. + extraNonce := uint64(0) + opRetData := make([]byte, 12) + binary.LittleEndian.PutUint32(opRetData[0:4], uint32(blockHeight)) + binary.LittleEndian.PutUint64(opRetData[4:12], extraNonce) + opReturnTreasury, err := stdscript.ProvablyPruneableScriptV0(opRetData) + if err != nil { + return nil, err + } + + // Calculate subsidy for the block height. + trsySubsidy := subsidyCache.CalcTreasurySubsidy(blockHeight, 5, true) + + tx := wire.NewMsgTx() + tx.Version = wire.TxVersionTreasury + tx.AddTxIn(&wire.TxIn{ + // Treasurybase transactions have no inputs, so previous outpoint is + // zero hash and max index. + PreviousOutPoint: *wire.NewOutPoint(&chainhash.Hash{}, + wire.MaxPrevOutIndex, wire.TxTreeRegular), + Sequence: wire.MaxTxInSequenceNum, + ValueIn: trsySubsidy, + BlockHeight: wire.NullBlockHeight, + BlockIndex: wire.NullBlockIndex, + SignatureScript: nil, // Must be nil by consensus. + }) + tx.AddTxOut(newTxOut(trsySubsidy, 0, []byte{txscript.OP_TADD})) + tx.AddTxOut(newTxOut(0, 0, opReturnTreasury)) + + return dcrutil.NewTx(tx), nil + } + + // Set the block height to something after the first few blocks and create + // a valid treasurybase for it. + blockHeight := int64(10) + harness.chain.SetHeight(blockHeight) + trsybase, err := createTreasurybase(blockHeight) + if err != nil { + t.Fatal(err) + } + + // Attempt to add the treasurybase and ensure it is rejected. Also, ensure + // it is not in the orphan pool, not in the transaction pool, and not + // reported as available. + _, err = harness.txPool.ProcessTransaction(trsybase, false, false, true, 0) + if !errors.Is(err, ErrTreasurybase) { + t.Fatal("Process Transaction: did not get expected ErrTreasurybase") + } + testPoolMembership(tc, trsybase, false, false) +} + // TestStagedTransactionHeight verifies that the height of a transaction // that moves from the stage pool into the main pool is set to the height it // was initially added to the mempool, rather than the height it was unstaged.