diff --git a/mempool/mempool.go b/mempool/mempool.go index 92f406c7..abfe2eae 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -727,6 +727,46 @@ func (mp *TxPool) checkPoolDoubleSpend(tx *dcrutil.Tx, txType stake.TxType) erro return nil } +// checkVoteDoubleSpend checks whether or not the passed vote is for a block +// that already has a vote that spends the same ticket available. This is +// necessary because the same ticket might be selected for blocks on candidate +// side chains and thus a more generic check to merely reject double spends of +// tickets is not possible. +// +// This function MUST be called with the mempool lock held (for reads). +// This function MUST NOT be called with the votes mutex held. +func (mp *TxPool) checkVoteDoubleSpend(vote *dcrutil.Tx) error { + voteTx := vote.MsgTx() + ticketSpent := voteTx.TxIn[1].PreviousOutPoint.Hash + hashVotedOn, heightVotedOn := stake.SSGenBlockVotedOn(voteTx) + mp.votesMtx.RLock() + for _, existingVote := range mp.votes[hashVotedOn] { + if existingVote.TicketHash == ticketSpent { + // Ensure the vote is still actually in the mempool. This is needed + // because the votes map is not currently kept in sync with the + // contents of the pool. + // + // TODO(decred): Ideally the votes map and mempool would be kept in + // sync, which would remove the need for this check, however, there + // is currently code outside of mempool that relies on being able to + // look up seen votes by block hash, regardless of their current + // membership in the pool. + if _, exists := mp.pool[existingVote.VoteHash]; !exists { + continue + } + + mp.votesMtx.RUnlock() + str := fmt.Sprintf("vote %v spending ticket %v already votes on "+ + "block %s (height %d)", vote.Hash(), ticketSpent, hashVotedOn, + heightVotedOn) + return txRuleError(wire.RejectDuplicate, str) + } + } + mp.votesMtx.RUnlock() + + return nil +} + // IsRegTxTreeKnownDisapproved returns whether or not the regular tree of the // block represented by the provided hash is known to be disapproved according // to the votes currently in the memory pool. @@ -964,7 +1004,17 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, allow if err != nil { return nil, err } + } else if isVote { + // Reject votes on blocks that already have a vote that spends the same + // ticket available. This is necessary because the same ticket might be + // selected for blocks on candidate side chains and thus a more generic + // check to merely reject double spends of tickets is not possible. + err := mp.checkVoteDoubleSpend(tx) + if err != nil { + return nil, err + } + voteAlreadyFound := 0 for _, mpTx := range mp.pool { if mpTx.Type == stake.TxTypeSSGen { @@ -980,6 +1030,7 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, allow return nil, txRuleError(wire.RejectDuplicate, str) } } + } else if isRevocation { for _, mpTx := range mp.pool { if mpTx.Type == stake.TxTypeSSRtx { diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index a82962d9..fa127703 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -1920,3 +1920,89 @@ func TestMaxVoteDoubleSpendRejection(t *testing.T) { } testPoolMembership(tc, vote, false, false) } + +// TestDuplicateVoteRejection ensures that additional votes on the same block +// that spend the same ticket are rejected from the pool as expected. +func TestDuplicateVoteRejection(t *testing.T) { + t.Parallel() + + harness, spendableOuts, err := newPoolHarness(&chaincfg.MainNetParams) + if err != nil { + t.Fatalf("unable to create test pool: %v", err) + } + tc := &testContext{t, harness} + + // Create a regular transaction from the first spendable output provided by + // the harness. + tx, err := harness.CreateTx(spendableOuts[0]) + if err != nil { + t.Fatalf("unable to create transaction: %v", err) + } + + // Create a ticket purchase transaction spending the outputs of the prior + // regular transaction. + ticket, err := harness.CreateTicketPurchase(tx, 40000) + if err != nil { + t.Fatalf("unable to create ticket purchase transaction: %v", err) + } + + // Add the ticket outputs as utxos to fake their existence. Use one after + // the stake enabled height for the height of the fake utxos to ensure they + // are matured for the votes cast a stake validation height below. + harness.chain.SetHeight(harness.chainParams.StakeEnabledHeight + 1) + harness.chain.utxos.AddTxOuts(ticket, harness.chain.BestHeight(), 0) + + // Create a vote that votes on a block at stake validation height. + harness.chain.SetBestHash(&chainhash.Hash{0x5c, 0xa1, 0xab, 0x1e}) + harness.chain.SetHeight(harness.chainParams.StakeValidationHeight) + vote, err := harness.CreateVote(ticket) + if err != nil { + t.Fatalf("unable to create vote: %v", err) + } + + // Add the vote and ensure it is not in the orphan pool, is in the + // transaction pool, and is reported as available. + _, err = harness.txPool.ProcessTransaction(vote, false, false, true) + if err != nil { + t.Fatalf("ProcessTransaction: failed to accept valid vote %v", err) + } + testPoolMembership(tc, vote, false, true) + + // Create another vote with a different hash that votes on the same block + // using the same ticket. + dupVote, err := harness.CreateVote(ticket, func(tx *wire.MsgTx) { + voteBits := stake.VoteBits{Bits: uint16(0x03), ExtendedBits: nil} + voteScript, err := newVoteScript(voteBits) + if err != nil { + t.Fatalf("failed to create vote script: %v", err) + } + tx.TxOut[1].PkScript = voteScript + }) + if err != nil { + t.Fatalf("unable to create vote: %v", err) + } + + // Attempt to add the duplicate vote and ensure it is rejected. Also, + // ensure it is not in the orphan pool, not in the transaction pool, and not + // reported as available. + _, err = harness.txPool.ProcessTransaction(dupVote, false, false, true) + if err == nil { + t.Fatalf("ProcessTransaction: accepted duplicate vote with different " + + "hash") + } + testPoolMembership(tc, dupVote, false, false) + + // Remove the original vote from the pool and ensure it is not in the orphan + // pool, not in the transaction pool, and not reported as available. + harness.txPool.RemoveTransaction(vote, true) + testPoolMembership(tc, vote, false, false) + + // Add the duplicate vote which should now be accepted. Also, ensure it is + // not in the orphan pool, is in the transaction pool, and is reported as + // available. + _, err = harness.txPool.ProcessTransaction(dupVote, false, false, true) + if err != nil { + t.Fatalf("ProcessTransaction: failed to accept valid vote %v", err) + } + testPoolMembership(tc, dupVote, false, true) +}