From dee4231e01caed2b374aaa3a0147dd2d72a0e9a0 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Sat, 5 Aug 2017 14:45:24 -0500 Subject: [PATCH] mempool/wire: Don't make policy decisions in wire. This removes the function IsSupportedMsgTxVersion function from wire since that is a policy decision and does not belong in wire. It also updates the mempool standard transaction checks to be more inline with the upstream code such that the maximum supported transaction version is specified via a field in the mempool policy configuration struct. Finally, it adds a new test to the standard transaction tests to ensure transactions that are not serialized with the full seriaization type are considered nonstandard. --- mempool/mempool.go | 8 +++++++- mempool/mempool_test.go | 1 + mempool/policy.go | 19 ++++++++++++++----- mempool/policy_test.go | 20 ++++++++++++++++++-- server.go | 1 + wire/msgtx.go | 7 ------- 6 files changed, 41 insertions(+), 15 deletions(-) diff --git a/mempool/mempool.go b/mempool/mempool.go index 440b6175..58f2e8b6 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -131,6 +131,11 @@ type Config struct { // Policy houses the policy (configuration parameters) which is used to // control the mempool. type Policy struct { + // MaxTxVersion is the max transaction version that the mempool should + // accept. All transactions above this version are rejected as + // non-standard. + MaxTxVersion uint16 + // DisableRelayPriority defines whether to relay free or low-fee // transactions that do not have enough priority to be relayed. DisableRelayPriority bool @@ -818,7 +823,8 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, allow // forbid their relaying. if !mp.cfg.Policy.RelayNonStd { err := checkTransactionStandard(tx, txType, nextBlockHeight, - mp.cfg.TimeSource, mp.cfg.Policy.MinRelayTxFee) + mp.cfg.TimeSource, mp.cfg.Policy.MinRelayTxFee, + mp.cfg.Policy.MaxTxVersion) 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/mempool_test.go b/mempool/mempool_test.go index 2d87122d..08679252 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -340,6 +340,7 @@ func newPoolHarness(chainParams *chaincfg.Params) (*poolHarness, []spendableOutp chain: chain, txPool: New(&Config{ Policy: Policy{ + MaxTxVersion: wire.TxVersion, DisableRelayPriority: true, FreeTxRelayLimit: 15.0, MaxOrphanTxs: 5, diff --git a/mempool/policy.go b/mempool/policy.go index 1343d388..9ca06387 100644 --- a/mempool/policy.go +++ b/mempool/policy.go @@ -347,15 +347,24 @@ 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) error { + timeSource blockchain.MedianTimeSource, minRelayTxFee dcrutil.Amount, + maxTxVersion uint16) error { // The transaction must be a currently supported version. + // + // The version includes the real transaction version in the lower 16 + // bits and the transaction serialize type as the upper 16 bits. msgTx := tx.MsgTx() - if !wire.IsSupportedMsgTxVersion(msgTx) { + serType := wire.TxSerializeType(uint32(msgTx.Version) >> 16) + txVersion := uint16(uint32(msgTx.Version) & 0xffff) + if serType != wire.TxSerializeFull { + str := fmt.Sprintf("transaction is not serialized with all "+ + "required data -- type %v", serType) + return txRuleError(wire.RejectNonstandard, str) + } + if txVersion > maxTxVersion || txVersion < 1 { str := fmt.Sprintf("transaction version %d is not in the "+ - "valid range of %d-%d", msgTx.Version, 1, - wire.TxVersion) + "valid range of %d-%d", txVersion, 1, maxTxVersion) return txRuleError(wire.RejectNonstandard, str) } diff --git a/mempool/policy_test.go b/mempool/policy_test.go index b3e8f2aa..84143fd5 100644 --- a/mempool/policy_test.go +++ b/mempool/policy_test.go @@ -287,6 +287,10 @@ func TestDust(t *testing.T) { // TestCheckTransactionStandard tests the checkTransactionStandard API. func TestCheckTransactionStandard(t *testing.T) { + // maxTxVersion is used as the maximum support transaction version for + // the policy in these tests. + const maxTxVersion = 1 + // Create some dummy, but otherwise standard, data for transactions. prevOutHash, err := chainhash.NewHashFromStr("01") if err != nil { @@ -336,10 +340,22 @@ func TestCheckTransactionStandard(t *testing.T) { height: 300000, isStandard: true, }, + { + name: "Transaction serialize type not full", + tx: wire.MsgTx{ + Version: int32(uint32(wire.TxSerializeNoWitness)<<16 | 1), + TxIn: []*wire.TxIn{&dummyTxIn}, + TxOut: []*wire.TxOut{&dummyTxOut}, + LockTime: 0, + }, + height: 300000, + isStandard: false, + code: wire.RejectNonstandard, + }, { name: "Transaction version too high", tx: wire.MsgTx{ - Version: int32(wire.TxVersion + 1), + Version: maxTxVersion + 1, TxIn: []*wire.TxIn{&dummyTxIn}, TxOut: []*wire.TxOut{&dummyTxOut}, LockTime: 0, @@ -492,7 +508,7 @@ func TestCheckTransactionStandard(t *testing.T) { // Ensure standardness is as expected. tx := dcrutil.NewTx(&test.tx) err := checkTransactionStandard(tx, stake.DetermineTxType(&test.tx), - test.height, timeSource, DefaultMinRelayTxFee) + test.height, timeSource, DefaultMinRelayTxFee, maxTxVersion) if err == nil && test.isStandard { // Test passes since function returned standard for a // transaction which is intended to be standard. diff --git a/server.go b/server.go index 1995dd0b..55f7854e 100644 --- a/server.go +++ b/server.go @@ -2412,6 +2412,7 @@ func newServer(listenAddrs []string, db database.DB, chainParams *chaincfg.Param txC := mempool.Config{ Policy: mempool.Policy{ + MaxTxVersion: 1, DisableRelayPriority: cfg.NoRelayPriority, RelayNonStd: cfg.RelayNonStd, FreeTxRelayLimit: cfg.FreeTxRelayLimit, diff --git a/wire/msgtx.go b/wire/msgtx.go index e4ef9d09..a0153723 100644 --- a/wire/msgtx.go +++ b/wire/msgtx.go @@ -1581,10 +1581,3 @@ func writeTxOut(w io.Writer, pver uint32, version int32, to *TxOut) error { return WriteVarBytes(w, pver, to.PkScript) } - -// IsSupportedMsgTxVersion returns if a transaction version is supported or not. -// Currently, inclusion into the memory pool (and thus blocks) only supports -// the DefaultMsgTxVersion. -func IsSupportedMsgTxVersion(msgTx *MsgTx) bool { - return msgTx.Version == DefaultMsgTxVersion() -}