From a82f67b538d50501741a3322238218577fd62669 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 23 Aug 2016 11:48:03 -0700 Subject: [PATCH 1/3] mempool: add closure to compute median time past to config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds an additional closure function to the mempool’s config which computes the median time past from the point of view of the best node in the chain. The mempool test harness has also been updated to allow setting a mock median time past for testing purposes. In addition to increasing the testability of the mempool, this commit should also speed up transaction and block validation for BIP 113 as the MTP no longer needs to be re-calculated each time from scratch. --- mempool/mempool.go | 5 +++++ mempool/mempool_test.go | 35 +++++++++++++++++++++++++++-------- server.go | 12 ++++++------ 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/mempool/mempool.go b/mempool/mempool.go index 9d232897..d8ce31e7 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -59,6 +59,11 @@ type Config struct { // the current best chain. BestHeight func() int32 + // MedianTimePast defines the function to use in order to access the + // median time past calculated from the point-of-view of the current + // chain tip within the best chain. + MedianTimePast func() time.Time + // SigCache defines a signature cache to use. SigCache *txscript.SigCache diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index 114ae769..4b8f577a 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -9,6 +9,7 @@ import ( "reflect" "sync" "testing" + "time" "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcec" @@ -24,8 +25,9 @@ import ( // transations to be appear as though they are spending completely valid utxos. type fakeChain struct { sync.RWMutex - utxos *blockchain.UtxoViewpoint - currentHeight int32 + utxos *blockchain.UtxoViewpoint + currentHeight int32 + medianTimePast time.Time } // FetchUtxoView loads utxo details about the input transactions referenced by @@ -72,6 +74,23 @@ func (s *fakeChain) SetHeight(height int32) { s.Unlock() } +// MedianTimePast returns the current median time past associated with the fake +// chain instance. +func (s *fakeChain) MedianTimePast() time.Time { + s.RLock() + mtp := s.medianTimePast + s.RUnlock() + return mtp +} + +// SetMedianTimePast sets the current median time past associated with the fake +// chain instance. +func (s *fakeChain) SetMedianTimePast(mtp time.Time) { + s.Lock() + s.medianTimePast = mtp + s.Unlock() +} + // spendableOutput is a convenience type that houses a particular utxo and the // amount associated with it. type spendableOutput struct { @@ -282,12 +301,12 @@ func newPoolHarness(chainParams *chaincfg.Params) (*poolHarness, []spendableOutp MaxSigOpsPerTx: blockchain.MaxSigOpsPerBlock / 5, MinRelayTxFee: 1000, // 1 Satoshi per byte }, - ChainParams: chainParams, - FetchUtxoView: chain.FetchUtxoView, - BestHeight: chain.BestHeight, - SigCache: nil, - TimeSource: blockchain.NewMedianTime(), - AddrIndex: nil, + ChainParams: chainParams, + FetchUtxoView: chain.FetchUtxoView, + BestHeight: chain.BestHeight, + MedianTimePast: chain.MedianTimePast, + SigCache: nil, + AddrIndex: nil, }), } diff --git a/server.go b/server.go index fc346c3b..7221c2fd 100644 --- a/server.go +++ b/server.go @@ -2582,12 +2582,12 @@ func newServer(listenAddrs []string, db database.DB, chainParams *chaincfg.Param MaxSigOpsPerTx: blockchain.MaxSigOpsPerBlock / 5, MinRelayTxFee: cfg.minRelayTxFee, }, - ChainParams: chainParams, - FetchUtxoView: s.blockManager.chain.FetchUtxoView, - BestHeight: func() int32 { return bm.chain.BestSnapshot().Height }, - SigCache: s.sigCache, - TimeSource: s.timeSource, - AddrIndex: s.addrIndex, + ChainParams: chainParams, + FetchUtxoView: s.blockManager.chain.FetchUtxoView, + BestHeight: func() int32 { return bm.chain.BestSnapshot().Height }, + MedianTimePast: func() time.Time { return bm.chain.BestSnapshot().MedianTime }, + SigCache: s.sigCache, + AddrIndex: s.addrIndex, } s.txMemPool = mempool.New(&txC) From e7caccc866b2caef61aefc9790a952c94cd04fd5 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 15 May 2016 13:22:21 +0300 Subject: [PATCH 2/3] mempool: transaction finality checks now use median-time-past This coincides with the mempool only, policy change which enforces transaction finality according to the median-time-past rather than blockheader timestamps. The behavior is pre-cursor to full blown BIP 113 consensus deployment, and subsequent activation. As a result, the TimeSource field in the mempoolConfig is no longer needed so it has been removed. Additionally, checkTransactionStandard has been modified to instead take a time.Time as the mempool is no longer explicitly dependant on a Chain instance. --- mempool/mempool.go | 8 +++----- mempool/policy.go | 8 +++++--- mempool/policy_test.go | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/mempool/mempool.go b/mempool/mempool.go index d8ce31e7..545335bc 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -67,9 +67,6 @@ type Config struct { // 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. @@ -551,8 +548,9 @@ func (mp *TxPool) maybeAcceptTransaction(tx *btcutil.Tx, isNew, rateLimit bool) // Don't allow non-standard transactions if the network parameters // forbid their relaying. if !mp.cfg.Policy.RelayNonStd { - err := checkTransactionStandard(tx, nextBlockHeight, - mp.cfg.TimeSource, mp.cfg.Policy.MinRelayTxFee) + medianTimePast := mp.cfg.MedianTimePast() + err = checkTransactionStandard(tx, nextBlockHeight, + medianTimePast, mp.cfg.Policy.MinRelayTxFee) if err != nil { // Attempt to extract a reject code from the error so // it can be retained. When not possible, fall back to diff --git a/mempool/policy.go b/mempool/policy.go index 69fc0ef2..229471d4 100644 --- a/mempool/policy.go +++ b/mempool/policy.go @@ -6,6 +6,7 @@ package mempool import ( "fmt" + "time" "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/txscript" @@ -326,7 +327,9 @@ func isDust(txOut *wire.TxOut, minRelayTxFee btcutil.Amount) bool { // finalized, conforming to more stringent size constraints, having scripts // 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 *btcutil.Tx, height int32, timeSource blockchain.MedianTimeSource, minRelayTxFee btcutil.Amount) error { +func checkTransactionStandard(tx *btcutil.Tx, height int32, + medianTimePast time.Time, minRelayTxFee btcutil.Amount) error { + // The transaction must be a currently supported version. msgTx := tx.MsgTx() if msgTx.Version > wire.TxVersion || msgTx.Version < 1 { @@ -338,8 +341,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int32, timeSource blockchai // 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, medianTimePast) { return txRuleError(wire.RejectNonstandard, "transaction is not finalized") } diff --git a/mempool/policy_test.go b/mempool/policy_test.go index b3c7faac..8637181e 100644 --- a/mempool/policy_test.go +++ b/mempool/policy_test.go @@ -7,8 +7,8 @@ package mempool import ( "bytes" "testing" + "time" - "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -466,11 +466,11 @@ func TestCheckTransactionStandard(t *testing.T) { }, } - timeSource := blockchain.NewMedianTime() + pastMedianTime := time.Now() for _, test := range tests { // Ensure standardness is as expected. err := checkTransactionStandard(btcutil.NewTx(&test.tx), - test.height, timeSource, DefaultMinRelayTxFee) + test.height, pastMedianTime, DefaultMinRelayTxFee) if err == nil && test.isStandard { // Test passes since function returned standard for a // transaction which is intended to be standard. From 403aaf5cf31eabb8408740d2fb3df0de9d1fa257 Mon Sep 17 00:00:00 2001 From: David Hill Date: Wed, 19 Oct 2016 13:15:21 -0400 Subject: [PATCH 3/3] rpcserver: avoid nested decodescript p2sh addrs --- btcjson/chainsvrresults.go | 2 +- rpcserver.go | 4 +++- rpcserverhelp.go | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/btcjson/chainsvrresults.go b/btcjson/chainsvrresults.go index 56728429..b6e3b103 100644 --- a/btcjson/chainsvrresults.go +++ b/btcjson/chainsvrresults.go @@ -56,7 +56,7 @@ type DecodeScriptResult struct { ReqSigs int32 `json:"reqSigs,omitempty"` Type string `json:"type"` Addresses []string `json:"addresses,omitempty"` - P2sh string `json:"p2sh"` + P2sh string `json:"p2sh,omitempty"` } // GetAddedNodeInfoResultAddr models the data of the addresses portion of the diff --git a/rpcserver.go b/rpcserver.go index 8eebe40b..5ec19776 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -833,7 +833,9 @@ func handleDecodeScript(s *rpcServer, cmd interface{}, closeChan <-chan struct{} ReqSigs: int32(reqSigs), Type: scriptClass.String(), Addresses: addresses, - P2sh: p2sh.EncodeAddress(), + } + if scriptClass != txscript.ScriptHashTy { + reply.P2sh = p2sh.EncodeAddress() } return reply, nil } diff --git a/rpcserverhelp.go b/rpcserverhelp.go index eee0053d..ad430eb7 100644 --- a/rpcserverhelp.go +++ b/rpcserverhelp.go @@ -106,7 +106,7 @@ var helpDescsEnUS = map[string]string{ "decodescriptresult-reqSigs": "The number of required signatures", "decodescriptresult-type": "The type of the script (e.g. 'pubkeyhash')", "decodescriptresult-addresses": "The bitcoin addresses associated with this script", - "decodescriptresult-p2sh": "The script hash for use in pay-to-script-hash transactions", + "decodescriptresult-p2sh": "The script hash for use in pay-to-script-hash transactions (only present if the provided redeem script is not already a pay-to-script-hash script)", // DecodeScriptCmd help. "decodescript--synopsis": "Returns a JSON object with information about the provided hex-encoded script.",