Skip to content

Commit

Permalink
multi: No stake height checks in check tx inputs.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davecgh committed Sep 14, 2018
1 parent 06d0021 commit b6c088d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 24 deletions.
20 changes: 0 additions & 20 deletions blockchain/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
22 changes: 20 additions & 2 deletions mempool/mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions mempool/mempool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit b6c088d

Please sign in to comment.