From bcbd6f997e5acd66055b2120f90a51f435be2286 Mon Sep 17 00:00:00 2001 From: Ryan Staudt Date: Fri, 22 Oct 2021 12:58:17 -0500 Subject: [PATCH] blockchain: Fix ticket db disconnect revocations. This addresses an issue in the ticket database when disconnecting a block that contains an automatic revocation. When the automatic ticket revocations agenda is enabled, a ticket can become missed and revoked in the same block. When connecting that block to the ticket database, this is recorded as two separate entries in the undo data (the first moving it to missed, and the second moving it to revoked). Therefore, when disconnecting the block, the undo data needs to be applied in reverse order. --- blockchain/stake/tickets.go | 56 ++++++++++++++++++++++++------------- blockchain/validate_test.go | 46 ++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 19 deletions(-) diff --git a/blockchain/stake/tickets.go b/blockchain/stake/tickets.go index 021fa5a555..e63666faa7 100644 --- a/blockchain/stake/tickets.go +++ b/blockchain/stake/tickets.go @@ -778,6 +778,7 @@ func disconnectNode(node *Node, parentLotteryIV chainhash.Hash, parentUtds UndoT } } + votesPerBlock := node.params.VotesPerBlock() restoredNode := &Node{ height: node.height - 1, liveTickets: node.liveTickets, @@ -785,16 +786,22 @@ func disconnectNode(node *Node, parentLotteryIV chainhash.Hash, parentUtds UndoT revokedTickets: node.revokedTickets, databaseUndoUpdate: parentUtds, databaseBlockTickets: parentTickets, - nextWinners: make([]chainhash.Hash, 0), + nextWinners: make([]chainhash.Hash, 0, votesPerBlock), params: node.params, } - // Iterate through the block undo data and write all database - // changes to the respective treap, reversing all the changes - // added when the child block was added to the chain. - stateBuffer := make([]byte, 0, - (node.params.VotesPerBlock()+1)*chainhash.HashSize) - for _, undo := range node.databaseUndoUpdate { + // Iterate through the block undo data and write all database changes to the + // respective treap, reversing all the changes added when the child block was + // added to the chain. + // + // Note that the undo data must be applied in reverse order since there may + // be multiple changes for the same ticket. For example, when the automatic + // revocations agenda is enabled a ticket can become missed and revoked in the + // same block. This is recorded as two separate entries in the undo data and + // must be processed in reverse order when disconnecting. + winners := make([]chainhash.Hash, 0, votesPerBlock) + for i := len(node.databaseUndoUpdate) - 1; i >= 0; i-- { + undo := node.databaseUndoUpdate[i] var err error k := tickettreap.Key(undo.TicketHash) v := &tickettreap.Value{ @@ -838,10 +845,7 @@ func disconnectNode(node *Node, parentLotteryIV chainhash.Hash, parentUtds UndoT // Expired tickets could never have been // winners. if !undo.Expired { - restoredNode.nextWinners = append(restoredNode.nextWinners, - undo.TicketHash) - stateBuffer = append(stateBuffer, - undo.TicketHash[:]...) + winners = append(winners, undo.TicketHash) } else { v.Expired = false } @@ -862,9 +866,7 @@ func disconnectNode(node *Node, parentLotteryIV chainhash.Hash, parentUtds UndoT // winners. case undo.Spent: v.Spent = false - restoredNode.nextWinners = append(restoredNode.nextWinners, - undo.TicketHash) - stateBuffer = append(stateBuffer, undo.TicketHash[:]...) + winners = append(winners, undo.TicketHash) restoredNode.liveTickets, err = safePut(restoredNode.liveTickets, k, v) if err != nil { @@ -877,6 +879,16 @@ func disconnectNode(node *Node, parentLotteryIV chainhash.Hash, parentUtds UndoT } } + // Set the next winners on the restored node. The winners are appended in + // reverse order since the undo data is applied in reverse above. + numWinners := len(winners) + stateBuffer := make([]byte, 0, (numWinners+1)*chainhash.HashSize) + for i := numWinners - 1; i >= 0; i-- { + winner := winners[i] + restoredNode.nextWinners = append(restoredNode.nextWinners, winner) + stateBuffer = append(stateBuffer, winner[:]...) + } + if node.height >= uint32(node.params.StakeValidationBeginHeight()) { prng := NewHash256PRNGFromIV(parentLotteryIV) _, err := findTicketIdxs(restoredNode.liveTickets.Len(), @@ -1029,11 +1041,17 @@ func WriteDisconnectedBestNode(dbTx database.Tx, node *Node, hash chainhash.Hash } } - // Iterate through the block undo data and write all database - // changes to the respective on-disk map, reversing all the - // changes added when the child block was added to the block - // chain. - for _, undo := range childUndoData { + // Iterate through the block undo data and write all database changes to the + // respective on-disk map, reversing all the changes added when the child + // block was added to the block chain. + // + // Note that the undo data must be applied in reverse order since there may + // be multiple changes for the same ticket. For example, when the automatic + // revocations agenda is enabled a ticket can become missed and revoked in the + // same block. This is recorded as two separate entries in the undo data and + // must be processed in reverse order when disconnecting. + for i := len(childUndoData) - 1; i >= 0; i-- { + undo := childUndoData[i] var err error switch { // All flags are unset; this is a newly added ticket. diff --git a/blockchain/validate_test.go b/blockchain/validate_test.go index b7677d4dbc..ab3fd6b193 100644 --- a/blockchain/validate_test.go +++ b/blockchain/validate_test.go @@ -1912,4 +1912,50 @@ func TestAutoRevocations(t *testing.T) { g.CreateRevocationsForMissedTickets(), replaceAutoRevocationsVersions) g.AssertTipNumRevocations(2) g.AcceptTipBlock() + + // Create a slice of the ticket hashes that revocations spent in the tip block + // that was just connected. + revocationTicketHashes := make([]chainhash.Hash, 0, params.TicketsPerBlock) + for _, stx := range g.Tip().STransactions { + // Append revocation ticket hashes. + if stake.IsSSRtx(stx, autoRevocationsEnabled) { + ticketHash := stx.TxIn[0].PreviousOutPoint.Hash + revocationTicketHashes = append(revocationTicketHashes, ticketHash) + + continue + } + } + + // Validate that the revocations are now in the revoked ticket treap in the + // ticket database. + tipHash = &g.chain.BestSnapshot().Hash + blockNode := g.chain.index.LookupNode(tipHash) + stakeNode, err := g.chain.fetchStakeNode(blockNode) + if err != nil { + t.Fatalf("error fetching stake node: %v", err) + } + for _, revocationTicketHash := range revocationTicketHashes { + if !stakeNode.ExistsRevokedTicket(revocationTicketHash) { + t.Fatalf("expected ticket %v to exist in the revoked ticket treap", + revocationTicketHash) + } + } + + // Invalidate the previously connected block so that it is disconnected. + g.InvalidateBlockAndExpectTip("b4", nil, startTip) + + // Validate that the revocations from the disconnected block are now back in + // the live ticket treap in the ticket database. + tipHash = &g.chain.BestSnapshot().Hash + blockNode = g.chain.index.LookupNode(tipHash) + stakeNode, err = g.chain.fetchStakeNode(blockNode) + if err != nil { + t.Fatalf("error fetching stake node: %v", err) + } + for _, revocationTicketHash := range revocationTicketHashes { + if !stakeNode.ExistsLiveTicket(revocationTicketHash) { + t.Fatalf("expected ticket %v to exist in the live ticket treap", + revocationTicketHash) + } + } }