Skip to content

Commit

Permalink
mempool: Use median time for tx finality checks.
Browse files Browse the repository at this point in the history
This modifies the mempool transaction finality checks to use the past
median time instead of the adjusted network time.  This ensures the
standardness rules match the upcoming consensus rules which reject
transactions that are not after the median time of the last several
blocks.  Without this change, it is possible for transactions which can
never make it into a block to be admitted to the mempool thereby wasting
memory.

The following is an overview of the changes:

- Introduce a new callback to the mempool config named PastMedianTime
  for obtaining the median time for the current best chain so it can
  remain decoupled from the chain
- Update the tx finality standardness check to use the callback
- Remove the now unused TimeSource mempool config parameter
- Update the mempool test harness and mock chain to use a mock median time
- Update server to provide a closure for the mempool config that uses
  the cached median time for the current best chain to negate any
  additional performance impact the new calculations would otherwise
  cause from recalculating the median time for every new candidate tx
  validation
  • Loading branch information
davecgh committed Sep 20, 2017
1 parent a70d73e commit ecf5f0a
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 12 deletions.
11 changes: 7 additions & 4 deletions mempool/mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,17 @@ type Config struct {
// the current best chain.
BestHeight func() int64

// PastMedianTime defines the function to use in order to access the
// median time calculated from the point-of-view of the current chain
// tip within the best chain.
PastMedianTime func() time.Time

// SubsidyCache defines a subsidy cache to use.
SubsidyCache *blockchain.SubsidyCache

// SigCache defines a signature cache to use.
SigCache *txscript.SigCache

// TimeSource defines the timesource to use.
TimeSource blockchain.MedianTimeSource

// AddrIndex defines the optional address index instance to use for
// indexing the unconfirmed transactions in the memory pool.
// This can be nil if the address index is not enabled.
Expand Down Expand Up @@ -830,8 +832,9 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, allow
// Don't allow non-standard transactions if the network parameters
// forbid their relaying.
if !mp.cfg.Policy.RelayNonStd {
medianTime := mp.cfg.PastMedianTime()
err := checkTransactionStandard(tx, txType, nextBlockHeight,
mp.cfg.TimeSource, mp.cfg.Policy.MinRelayTxFee,
medianTime, mp.cfg.Policy.MinRelayTxFee,
mp.cfg.Policy.MaxTxVersion)
if err != nil {
// Attempt to extract a reject code from the error so
Expand Down
22 changes: 21 additions & 1 deletion mempool/mempool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"reflect"
"sync"
"testing"
"time"

"github.com/decred/dcrd/blockchain"
"github.com/decred/dcrd/chaincfg"
Expand All @@ -32,6 +33,7 @@ type fakeChain struct {
blocks map[chainhash.Hash]*dcrutil.Block
currentHash chainhash.Hash
currentHeight int64
medianTime time.Time
scriptFlags txscript.ScriptFlags
}

Expand Down Expand Up @@ -133,6 +135,23 @@ func (s *fakeChain) SetHeight(height int64) {
s.Unlock()
}

// PastMedianTime returns the current median time associated with the fake chain
// instance.
func (s *fakeChain) PastMedianTime() time.Time {
s.RLock()
medianTime := s.medianTime
s.RUnlock()
return medianTime
}

// SetPastMedianTime sets the current median time associated with the fake chain
// instance.
func (s *fakeChain) SetPastMedianTime(medianTime time.Time) {
s.Lock()
s.medianTime = medianTime
s.Unlock()
}

// StandardVerifyFlags returns the standard verification script flags associated
// with the fake chain instance.
func (s *fakeChain) StandardVerifyFlags() (txscript.ScriptFlags, error) {
Expand Down Expand Up @@ -369,9 +388,9 @@ func newPoolHarness(chainParams *chaincfg.Params) (*poolHarness, []spendableOutp
BlockByHash: chain.BlockByHash,
BestHash: chain.BestHash,
BestHeight: chain.BestHeight,
PastMedianTime: chain.PastMedianTime,
SubsidyCache: subsidyCache,
SigCache: nil,
TimeSource: blockchain.NewMedianTime(),
AddrIndex: nil,
ExistsAddrIndex: nil,
}),
Expand All @@ -394,6 +413,7 @@ func newPoolHarness(chainParams *chaincfg.Params) (*poolHarness, []spendableOutp
outputs = append(outputs, txOutToSpendableOut(coinbase, i))
}
harness.chain.SetHeight(int64(chainParams.CoinbaseMaturity) + curHeight)
harness.chain.SetPastMedianTime(time.Now())

return &harness, outputs, nil
}
Expand Down
6 changes: 3 additions & 3 deletions mempool/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package mempool

import (
"fmt"
"time"

"github.com/decred/dcrd/blockchain"
"github.com/decred/dcrd/blockchain/stake"
Expand Down Expand Up @@ -363,7 +364,7 @@ func isDust(txOut *wire.TxOut, minRelayTxFee dcrutil.Amount) bool {
// of recognized forms, and not containing "dust" outputs (those that are
// so small it costs more to process them than they are worth).
func checkTransactionStandard(tx *dcrutil.Tx, txType stake.TxType, height int64,
timeSource blockchain.MedianTimeSource, minRelayTxFee dcrutil.Amount,
medianTime time.Time, minRelayTxFee dcrutil.Amount,
maxTxVersion uint16) error {

// The transaction must be a currently supported version and serialize
Expand All @@ -382,8 +383,7 @@ func checkTransactionStandard(tx *dcrutil.Tx, txType stake.TxType, height int64,

// The transaction must be finalized to be standard and therefore
// considered for inclusion in a block.
adjustedTime := timeSource.AdjustedTime()
if !blockchain.IsFinalizedTransaction(tx, height, adjustedTime) {
if !blockchain.IsFinalizedTransaction(tx, height, medianTime) {
return txRuleError(wire.RejectNonstandard,
"transaction is not finalized")
}
Expand Down
7 changes: 4 additions & 3 deletions mempool/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"bytes"
"math/big"
"testing"
"time"

"github.com/decred/dcrd/blockchain"
"github.com/decred/dcrd/blockchain/stake"
"github.com/decred/dcrd/chaincfg"
"github.com/decred/dcrd/chaincfg/chainec"
Expand Down Expand Up @@ -514,12 +514,13 @@ func TestCheckTransactionStandard(t *testing.T) {
},
}

timeSource := blockchain.NewMedianTime()
medianTime := time.Now()
for _, test := range tests {
// Ensure standardness is as expected.
tx := dcrutil.NewTx(&test.tx)
err := checkTransactionStandard(tx, stake.DetermineTxType(&test.tx),
test.height, timeSource, DefaultMinRelayTxFee, maxTxVersion)
test.height, medianTime, DefaultMinRelayTxFee,
maxTxVersion)
if err == nil && test.isStandard {
// Test passes since function returned standard for a
// transaction which is intended to be standard.
Expand Down
2 changes: 1 addition & 1 deletion server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2446,7 +2446,7 @@ func newServer(listenAddrs []string, db database.DB, chainParams *chaincfg.Param
BestHeight: func() int64 { return bm.chain.BestSnapshot().Height },
SubsidyCache: bm.chain.FetchSubsidyCache(),
SigCache: s.sigCache,
TimeSource: s.timeSource,
PastMedianTime: func() time.Time { return bm.chain.BestSnapshot().MedianTime },
AddrIndex: s.addrIndex,
ExistsAddrIndex: s.existsAddrIndex,
}
Expand Down

0 comments on commit ecf5f0a

Please sign in to comment.