From b6c088dc0d14b399015bc6aa675fa2425066228e Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Fri, 14 Sep 2018 02:29:44 -0500 Subject: [PATCH] multi: No stake height checks in check tx inputs. This removes a redundant check for validating that votes and revocations are not in a block before they are allowed from CheckTransactionInputs since it has nothing to do with validation of transaction inputs and it's an impossible condition to hit in the block validation path because the same check is done much earlier in the validation process. It should also be noted that the checks this removes are also more permissive than they need to be because the first vote is not allowed until stake validation height, which is after stake enabled height. There is no effect on consensus because, as previously mentioned, the more restrictive checks earlier in the process prevent these checks from ever being exercised. However, the mempool previously relied on this duplicate check to reject early votes and revocations, so this adds the checks, with the corrected heights, directly to the mempool early in the process where they more naturally belong. It is more efficient to reject the transactions in that scenario earlier before doing more work as well. --- blockchain/validate.go | 20 -------------------- mempool/mempool.go | 22 ++++++++++++++++++++-- mempool/mempool_test.go | 4 ++-- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/blockchain/validate.go b/blockchain/validate.go index 61472ad103..b877e2d74e 100644 --- a/blockchain/validate.go +++ b/blockchain/validate.go @@ -1257,7 +1257,6 @@ func CheckTransactionInputs(subsidyCache *SubsidyCache, tx *dcrutil.Tx, txHeight msgTx := tx.MsgTx() ticketMaturity := int64(chainParams.TicketMaturity) - stakeEnabledHeight := chainParams.StakeEnabledHeight txHash := tx.Hash() var totalAtomIn int64 @@ -1348,15 +1347,6 @@ func CheckTransactionInputs(subsidyCache *SubsidyCache, tx *dcrutil.Tx, txHeight // OP_SSTX tagged output uses. isSSGen := stake.IsSSGen(msgTx) if isSSGen { - // Cursory check to see if we've even reached stake-enabled - // height. - if txHeight < stakeEnabledHeight { - errStr := fmt.Sprintf("SSGen tx appeared in block "+ - "height %v before stake enabled height %v", - txHeight, stakeEnabledHeight) - return 0, ruleError(ErrInvalidEarlyStakeTx, errStr) - } - // Grab the input SStx hash from the inputs of the transaction. nullIn := msgTx.TxIn[0] sstxIn := msgTx.TxIn[1] // sstx input @@ -1499,16 +1489,6 @@ func CheckTransactionInputs(subsidyCache *SubsidyCache, tx *dcrutil.Tx, txHeight // this later input check for OP_SSTX outs. isSSRtx := stake.IsSSRtx(msgTx) if isSSRtx { - // Cursory check to see if we've even reach stake-enabled - // height. Note for an SSRtx to be valid a vote must be - // missed, so for SSRtx the height of allowance is +1. - if txHeight < stakeEnabledHeight+1 { - errStr := fmt.Sprintf("SSRtx tx appeared in block "+ - "height %v before stake enabled height+1 %v", - txHeight, stakeEnabledHeight+1) - return 0, ruleError(ErrInvalidEarlyStakeTx, errStr) - } - // Grab the input SStx hash from the inputs of the transaction. sstxIn := msgTx.TxIn[0] // sstx input sstxHash := sstxIn.PreviousOutPoint.Hash diff --git a/mempool/mempool.go b/mempool/mempool.go index a5aac4a95e..e9ac3d9dab 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -853,6 +853,26 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, allow tx.SetTree(wire.TxTreeStake) } + // Reject votes before stake validation height. + isVote := txType == stake.TxTypeSSGen + stakeValidationHeight := mp.cfg.ChainParams.StakeValidationHeight + if isVote && nextBlockHeight < stakeValidationHeight { + str := fmt.Sprintf("votes are not valid until block height %d (next "+ + "block height %d)", stakeValidationHeight, nextBlockHeight) + return nil, txRuleError(wire.RejectInvalid, str) + } + + // Reject revocations before they can possibly be valid. A vote must be + // missed for a revocation to be valid and votes are not allowed until stake + // validation height, so, a revocations can't possibly be valid until one + // block later. + isRevocation := txType == stake.TxTypeSSRtx + if isRevocation && nextBlockHeight < stakeValidationHeight+1 { + str := fmt.Sprintf("revocations are not valid until block height %d "+ + "(next block height %d)", stakeValidationHeight+1, nextBlockHeight) + return nil, txRuleError(wire.RejectInvalid, str) + } + // Don't allow non-standard transactions if the mempool config forbids // their acceptance and relaying. medianTime := mp.cfg.PastMedianTime() @@ -876,8 +896,6 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, allow // If the transaction is a ticket, ensure that it meets the next // stake difficulty. - isVote := txType == stake.TxTypeSSGen - isRevocation := txType == stake.TxTypeSSRtx isTicket := txType == stake.TxTypeSStx if isTicket { sDiff, err := mp.cfg.NextStakeDifficulty() diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index 158705178c..bd09a59e8a 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -815,7 +815,7 @@ func TestVoteOrphan(t *testing.T) { t.Fatalf("unable to create ticket purchase transaction: %v", err) } - harness.chain.SetHeight(harness.chainParams.StakeEnabledHeight) + harness.chain.SetHeight(harness.chainParams.StakeValidationHeight) vote, err := harness.CreateVote(ticket) if err != nil { @@ -886,7 +886,7 @@ func TestRevocationOrphan(t *testing.T) { t.Fatalf("unable to create ticket purchase transaction: %v", err) } - harness.chain.SetHeight(harness.chainParams.StakeEnabledHeight + 1) + harness.chain.SetHeight(harness.chainParams.StakeValidationHeight + 1) revocation, err := harness.CreateRevocation(ticket) if err != nil {