From 26ff8ddce4121c99ee9dc659625d800f3d6dc3fb Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 18 Oct 2016 19:21:55 -0700 Subject: [PATCH] mempool: modify mempool sanity checks to be segwit aware --- config.go | 2 +- mempool/mempool.go | 46 +++++++++++++++++++++++++++++--------- mempool/mempool_test.go | 2 +- mempool/policy.go | 49 +++++++++++++++++++++++++++++++---------- mempool/policy_test.go | 6 ++--- server.go | 6 +++-- 6 files changed, 82 insertions(+), 29 deletions(-) diff --git a/config.go b/config.go index d34fa5d4..df47405b 100644 --- a/config.go +++ b/config.go @@ -53,7 +53,7 @@ const ( blockMaxSizeMax = wire.MaxBlockPayload - 1000 defaultGenerate = false defaultMaxOrphanTransactions = 100 - defaultMaxOrphanTxSize = mempool.MaxStandardTxSize + defaultMaxOrphanTxSize = 100000 defaultSigCacheMaxSize = 100000 sampleConfigFilename = "sample-btcd.conf" defaultTxIndex = false diff --git a/mempool/mempool.go b/mempool/mempool.go index 99cb963d..bf08e260 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -73,9 +73,18 @@ type Config struct { // utxo view. CalcSequenceLock func(*btcutil.Tx, *blockchain.UtxoViewpoint) (*blockchain.SequenceLock, error) + // IsDeploymentActive returns true if the target deploymentID is + // active, and false otherwise. The mempool uses this function to gauge + // if transactions using new to be soft-forked rules should be allowed + // into the mempool or not. + IsDeploymentActive func(deploymentID uint32) (bool, error) + // SigCache defines a signature cache to use. SigCache *txscript.SigCache + // HashCache defines the transaction hash mid-state cache to use. + HashCache *txscript.HashCache + // 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. @@ -112,10 +121,10 @@ type Policy struct { // of big orphans. MaxOrphanTxSize int - // MaxSigOpsPerTx is the maximum number of signature operations - // in a single transaction we will relay or mine. It is a fraction - // of the max signature operations for a block. - MaxSigOpsPerTx int + // MaxSigOpCostPerTx is the cumulative maximum cost of all the signature + // operations in a single transaction we will relay or mine. It is a + // fraction of the max signature operations for a block. + MaxSigOpCostPerTx int // MinRelayTxFee defines the minimum transaction fee in BTC/kB to be // considered a non-zero fee. @@ -604,6 +613,22 @@ func (mp *TxPool) FetchTransaction(txHash *chainhash.Hash) (*btcutil.Tx, error) func (mp *TxPool) maybeAcceptTransaction(tx *btcutil.Tx, isNew, rateLimit, rejectDupOrphans bool) ([]*chainhash.Hash, *TxDesc, error) { txHash := tx.Hash() + // If a transaction has iwtness data, and segwit isn't active yet, If + // segwit isn't active yet, then we won't accept it into the mempool as + // it can't be mined yet. + if tx.MsgTx().HasWitness() { + segwitActive, err := mp.cfg.IsDeploymentActive(chaincfg.DeploymentSegwit) + if err != nil { + return nil, nil, err + } + + if !segwitActive { + str := fmt.Sprintf("transaction %v has witness data, "+ + "but segwit isn't active yet", txHash) + return nil, nil, txRuleError(wire.RejectNonstandard, str) + } + } + // Don't accept the transaction if it already exists in the pool. This // applies to orphan transactions as well when the reject duplicate // orphans flag is set. This check is intended to be a quick check to @@ -780,17 +805,17 @@ func (mp *TxPool) maybeAcceptTransaction(tx *btcutil.Tx, isNew, rateLimit, rejec // the coinbase address itself can contain signature operations, the // maximum allowed signature operations per transaction is less than // the maximum allowed signature operations per block. - numSigOps, err := blockchain.CountP2SHSigOps(tx, false, utxoView) + // TODO(roasbeef): last bool should be conditional on segwit activation + sigOpCost, err := blockchain.GetSigOpCost(tx, false, utxoView, true, true) if err != nil { if cerr, ok := err.(blockchain.RuleError); ok { return nil, nil, chainRuleError(cerr) } return nil, nil, err } - numSigOps += blockchain.CountSigOps(tx) - if numSigOps > mp.cfg.Policy.MaxSigOpsPerTx { - str := fmt.Sprintf("transaction %v has too many sigops: %d > %d", - txHash, numSigOps, mp.cfg.Policy.MaxSigOpsPerTx) + if sigOpCost > mp.cfg.Policy.MaxSigOpCostPerTx { + str := fmt.Sprintf("transaction %v sigop cost is too high: %d > %d", + txHash, sigOpCost, mp.cfg.Policy.MaxSigOpCostPerTx) return nil, nil, txRuleError(wire.RejectNonstandard, str) } @@ -857,7 +882,8 @@ func (mp *TxPool) maybeAcceptTransaction(tx *btcutil.Tx, isNew, rateLimit, rejec // Verify crypto signatures for each input and reject the transaction if // any don't verify. err = blockchain.ValidateTransactionScripts(tx, utxoView, - txscript.StandardVerifyFlags, mp.cfg.SigCache) + txscript.StandardVerifyFlags, mp.cfg.SigCache, + mp.cfg.HashCache) if err != nil { if cerr, ok := err.(blockchain.RuleError); ok { return nil, nil, chainRuleError(cerr) diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index dc3d7aca..c557d61f 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -310,7 +310,7 @@ func newPoolHarness(chainParams *chaincfg.Params) (*poolHarness, []spendableOutp FreeTxRelayLimit: 15.0, MaxOrphanTxs: 5, MaxOrphanTxSize: 1000, - MaxSigOpsPerTx: blockchain.MaxSigOpsPerBlock / 5, + MaxSigOpCostPerTx: blockchain.MaxBlockSigOpsCost / 4, MinRelayTxFee: 1000, // 1 Satoshi per byte MaxTxVersion: 1, }, diff --git a/mempool/policy.go b/mempool/policy.go index fc5c38ef..89cd7a50 100644 --- a/mempool/policy.go +++ b/mempool/policy.go @@ -19,10 +19,9 @@ const ( // that are considered standard in a pay-to-script-hash script. maxStandardP2SHSigOps = 15 - // MaxStandardTxSize is the maximum size allowed for transactions that - // are considered standard and will therefore be relayed and considered - // for mining. - MaxStandardTxSize = 100000 + // maxStandardTxCost is the max weight permitted by any transaction + // according to the current default policy. + maxStandardTxWeight = 400000 // maxStandardSigScriptSize is the maximum size allowed for a // transaction input signature script to be considered standard. This @@ -217,17 +216,43 @@ func isDust(txOut *wire.TxOut, minRelayTxFee btcutil.Amount) bool { // 36 prev outpoint, 1 script len, 73 script [1 OP_DATA_72, // 72 sig], 4 sequence // + // Pay-to-witness-pubkey-hash bytes breakdown: + // + // Output to witness key hash (31 bytes); + // 8 value, 1 script len, 22 script [1 OP_0, 1 OP_DATA_20, + // 20 bytes hash160] + // + // Input (67 bytes as the 107 witness stack is discounted): + // 36 prev outpoint, 1 script len, 0 script (not sigScript), 107 + // witness stack bytes [1 element length, 33 compressed pubkey, + // element length 72 sig], 4 sequence + // + // // Theoretically this could examine the script type of the output script // and use a different size for the typical input script size for // pay-to-pubkey vs pay-to-pubkey-hash inputs per the above breakdowns, - // but the only combinination which is less than the value chosen is + // but the only combination which is less than the value chosen is // a pay-to-pubkey script with a compressed pubkey, which is not very // common. // // The most common scripts are pay-to-pubkey-hash, and as per the above // breakdown, the minimum size of a p2pkh input script is 148 bytes. So - // that figure is used. - totalSize := txOut.SerializeSize() + 148 + // that figure is used. If the output being spent is a witness program, + // then we apply the witness discount to the size of the signature. + // + // The segwit analogue to p2pkh is a p2wkh output. This is the smallest + // output possible using the new segwit features. The 107 bytes of + // witness data is discounted by a factor of 4, leading to a computed + // value of 67 bytes of witness data. + // + // Both cases share a 41 byte preamble required to reference the input + // being spent and the sequence number of the input. + totalSize := txOut.SerializeSize() + 41 + if txscript.IsWitnessProgram(txOut.PkScript) { + totalSize += (107 / blockchain.WitnessScaleFactor) + } else { + totalSize += 107 + } // The output is considered dust if the cost to the network to spend the // coins is more than 1/3 of the minimum free transaction relay fee. @@ -275,10 +300,10 @@ func checkTransactionStandard(tx *btcutil.Tx, height int32, // almost as much to process as the sender fees, limit the maximum // size of a transaction. This also helps mitigate CPU exhaustion // attacks. - serializedLen := msgTx.SerializeSize() - if serializedLen > MaxStandardTxSize { - str := fmt.Sprintf("transaction size of %v is larger than max "+ - "allowed size of %v", serializedLen, MaxStandardTxSize) + txWeight := blockchain.GetTransactionWeight(tx) + if txWeight > maxStandardTxWeight { + str := fmt.Sprintf("weight of transaction %v is larger than max "+ + "allowed weight of %v", txWeight, maxStandardTxWeight) return txRuleError(wire.RejectNonstandard, str) } @@ -345,7 +370,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int32, } // GetTxVirtualSize computes the virtual size of a given transaction. A -// transaction's virtual size is based off it's weight, creating a discount for +// transaction's virtual size is based off its weight, creating a discount for // any witness data it contains, proportional to the current // blockchain.WitnessScaleFactor value. func GetTxVirtualSize(tx *btcutil.Tx) int64 { diff --git a/mempool/policy_test.go b/mempool/policy_test.go index 9b4e660c..9dd618ad 100644 --- a/mempool/policy_test.go +++ b/mempool/policy_test.go @@ -41,13 +41,13 @@ func TestCalcMinRequiredTxRelayFee(t *testing.T) { }, { "max standard tx size with default minimum relay fee", - MaxStandardTxSize, + maxStandardTxWeight / 4, DefaultMinRelayTxFee, 100000, }, { "max standard tx size with max satoshi relay fee", - MaxStandardTxSize, + maxStandardTxWeight / 4, btcutil.MaxSatoshi, btcutil.MaxSatoshi, }, @@ -360,7 +360,7 @@ func TestCheckTransactionStandard(t *testing.T) { TxOut: []*wire.TxOut{{ Value: 0, PkScript: bytes.Repeat([]byte{0x00}, - MaxStandardTxSize+1), + (maxStandardTxWeight/4)+1), }}, LockTime: 0, }, diff --git a/server.go b/server.go index 60dbeeb0..86ee5852 100644 --- a/server.go +++ b/server.go @@ -2470,8 +2470,10 @@ func newServer(listenAddrs []string, db database.DB, chainParams *chaincfg.Param CalcSequenceLock: func(tx *btcutil.Tx, view *blockchain.UtxoViewpoint) (*blockchain.SequenceLock, error) { return bm.chain.CalcSequenceLock(tx, view, true) }, - SigCache: s.sigCache, - AddrIndex: s.addrIndex, + IsDeploymentActive: bm.chain.IsDeploymentActive, + SigCache: s.sigCache, + HashCache: s.hashCache, + AddrIndex: s.addrIndex, } s.txMemPool = mempool.New(&txC)