From 6086f988dbb36ed724133958269e9c972c6e4008 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Thu, 15 Feb 2018 16:13:08 -0600 Subject: [PATCH 1/2] blockchain: Validate allowed votes in block context. This moves the test for validating the votes in the block are eligible votes per the results of the ticket lottery into the checkBlockContext function where it more naturally belongs since it is only dependent on the block and its contextual position within the chain. While here, it also refactors the check into a separate function named checkAllowedVotes to improve readability. --- blockchain/validate.go | 115 ++++++++++++++++------------------------- 1 file changed, 45 insertions(+), 70 deletions(-) diff --git a/blockchain/validate.go b/blockchain/validate.go index d92816a79f..d808ed6a01 100644 --- a/blockchain/validate.go +++ b/blockchain/validate.go @@ -1006,6 +1006,38 @@ func (b *BlockChain) checkBlockHeaderContext(header *wire.BlockHeader, prevNode return nil } +// checkAllowedVotes performs validation of all votes in the block to ensure +// they spend tickets that are actually allowed to vote per the lottery. +// +// This function is safe for concurrent access. +func (b *BlockChain) checkAllowedVotes(parentStakeNode *stake.Node, block *wire.MsgBlock) error { + // Determine the winning ticket hashes and create a map for faster lookup. + ticketsPerBlock := int(b.chainParams.TicketsPerBlock) + winningHashes := make(map[chainhash.Hash]struct{}, ticketsPerBlock) + for _, ticketHash := range parentStakeNode.Winners() { + winningHashes[ticketHash] = struct{}{} + } + + for _, stx := range block.STransactions { + // Ignore non-vote stake transactions. + if !stake.IsSSGen(stx) { + continue + } + + // Ensure the ticket being spent is actually eligible to vote in + // this block. + ticketHash := stx.TxIn[1].PreviousOutPoint.Hash + if _, ok := winningHashes[ticketHash]; !ok { + errStr := fmt.Sprintf("block contains vote for "+ + "ineligible ticket %s (eligible tickets: %s)", + ticketHash, winningHashes) + return ruleError(ErrTicketUnavailable, errStr) + } + } + + return nil +} + // checkBlockContext peforms several validation checks on the block which depend // on its position within the block chain. // @@ -1090,6 +1122,19 @@ func (b *BlockChain) checkBlockContext(block *dcrutil.Block, prevNode *blockNode return err } } + + // Ensure that all votes are only for winning tickets once stake + // validation height has been reached. + if blockHeight >= b.chainParams.StakeValidationHeight { + parentStakeNode, err := b.fetchStakeNode(prevNode) + if err != nil { + return err + } + err = b.checkAllowedVotes(parentStakeNode, block.MsgBlock()) + if err != nil { + return err + } + } } return nil @@ -1148,7 +1193,6 @@ func (b *BlockChain) CheckBlockStakeSanity(stakeValidationHeight int64, node *bl // Setup variables. stakeTransactions := block.STransactions() blockHash := block.Hash() - ticketsPerBlock := int(b.chainParams.TicketsPerBlock) parentStakeNode, err := b.fetchStakeNode(node.parent) if err != nil { @@ -1160,75 +1204,6 @@ func (b *BlockChain) CheckBlockStakeSanity(stakeValidationHeight int64, node *bl return nil } - // ------------------------------------------------------------------- - // SSGen Tx Handling - // ------------------------------------------------------------------- - // PER SSGEN - // 1. Retrieve an emulated ticket database of SStxMemMaps from both the - // ticket database and the ticket store. - // 2. Check to ensure that the tickets included in the block are the - // ones that indeed should have been included according to the - // emulated ticket database. - // 3. Check to make sure that the SSGen votes on the correct - // block/height. - - // PER BLOCK - // 4. Check and make sure that we have the same number of SSGen tx as - // we do votes. - // 5. Check for voters overflows (highly unlikely, but check anyway). - // 6. Ensure that the block votes on tx tree regular of the previous - // block in the way of the majority of the voters. - - // 1. Retrieve an emulated ticket database of SStxMemMaps from both the - // ticket database and the ticket store. - ticketsWhichCouldBeUsed := make(map[chainhash.Hash]struct{}, - ticketsPerBlock) - ticketSlice := parentStakeNode.Winners() - - // 2. Obtain the tickets which could have been used on the block for - // votes and then check below to make sure that these were indeed - // the tickets used. - for _, ticketHash := range ticketSlice { - ticketsWhichCouldBeUsed[ticketHash] = struct{}{} - // Fetch utxo details for all of the transactions in this - // block. Typically, there will not be any utxos for any of - // the transactions. - } - - for _, staketx := range stakeTransactions { - msgTx := staketx.MsgTx() - if stake.IsSSGen(msgTx) { - // Grab the input SStx hash from the inputs of the - // transaction. - sstxIn := msgTx.TxIn[1] // sstx input - sstxHash := sstxIn.PreviousOutPoint.Hash - - // Check to make sure this was actually a ticket we - // were allowed to use. - _, ticketAvailable := ticketsWhichCouldBeUsed[sstxHash] - if !ticketAvailable { - errStr := fmt.Sprintf("Error in stake "+ - "consensus: Ticket %v was not found "+ - "to be available in the stake patch "+ - "or database, yet block %v spends it!", - sstxHash, blockHash) - return ruleError(ErrTicketUnavailable, errStr) - } - } - } - - // 3. Check to make sure that the SSGen votes on the correct - // block/height. Already checked in checkBlockSanity. - - // 4. Check and make sure that we have the same number of SSGen tx as - // we do votes. Already checked in checkBlockSanity. - - // 5. Check for too many voters. Already checked in checkBlockSanity. - - // 6. Determine if TxTreeRegular should be valid or not, and then check - // it against what is provided in the block header. Already checked - // in checkBlockSanity. - // ------------------------------------------------------------------- // SSRtx Tx Handling // ------------------------------------------------------------------- From f4d5baab465b84ef77cca8fd3ec65ab52e1443f6 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Thu, 15 Feb 2018 16:32:07 -0600 Subject: [PATCH 2/2] blockchain: Validate allowed revokes in blk contxt. This moves the test for validating the revocations in the block are actually eligible to be revoked per the results of the ticket lottery into the checkBlockContext function where it more naturally belongs since it is only dependent on the block and its contextual position within the chain. While here, it also refactors the check into a separate function named checkAllowedRevocations to improve readability. Also, since this was the final check in the CheckBlockStakeSanity function, that entire function has been removed along with its call site. --- blockchain/validate.go | 103 ++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 69 deletions(-) diff --git a/blockchain/validate.go b/blockchain/validate.go index d808ed6a01..84b9c7aed8 100644 --- a/blockchain/validate.go +++ b/blockchain/validate.go @@ -1038,6 +1038,32 @@ func (b *BlockChain) checkAllowedVotes(parentStakeNode *stake.Node, block *wire. return nil } +// checkAllowedRevocations performs validation of all revocations in the block +// to ensure they spend tickets that are actually allowed to be revoked per the +// lottery. Tickets are only eligible to be revoked if they were missed or have +// expired. +// +// This function is safe for concurrent access. +func (b *BlockChain) checkAllowedRevocations(parentStakeNode *stake.Node, block *wire.MsgBlock) error { + for _, stx := range block.STransactions { + // Ignore non-revocation stake transactions. + if !stake.IsSSRtx(stx) { + continue + } + + // Ensure the ticket being spent is actually eligible to be + // revoked in this block. + ticketHash := stx.TxIn[0].PreviousOutPoint.Hash + if !parentStakeNode.ExistsMissedTicket(ticketHash) { + errStr := fmt.Sprintf("block contains revocation of "+ + "ineligible ticket %s", ticketHash) + return ruleError(ErrInvalidSSRtx, errStr) + } + } + + return nil +} + // checkBlockContext peforms several validation checks on the block which depend // on its position within the block chain. // @@ -1123,7 +1149,8 @@ func (b *BlockChain) checkBlockContext(block *dcrutil.Block, prevNode *blockNode } } - // Ensure that all votes are only for winning tickets once stake + // Ensure that all votes are only for winning tickets and all + // revocations are actually eligible to be revoked once stake // validation height has been reached. if blockHeight >= b.chainParams.StakeValidationHeight { parentStakeNode, err := b.fetchStakeNode(prevNode) @@ -1134,6 +1161,12 @@ func (b *BlockChain) checkBlockContext(block *dcrutil.Block, prevNode *blockNode if err != nil { return err } + + err = b.checkAllowedRevocations(parentStakeNode, + block.MsgBlock()) + if err != nil { + return err + } } } @@ -1182,66 +1215,6 @@ func (b *BlockChain) checkDupTxs(txSet []*dcrutil.Tx, view *UtxoViewpoint) error return nil } -// CheckBlockStakeSanity performs a series of checks on a block to ensure that -// the information from the block's header about stake is sane. For instance, -// the number of SSGen tx must be equal to voters. -// TODO: We can consider breaking this into two functions and making some of -// these checks go through in processBlock, however if a block has demonstrable -// PoW it seems unlikely that it will have stake errors (because the miner is -// then just wasting hash power). -func (b *BlockChain) CheckBlockStakeSanity(stakeValidationHeight int64, node *blockNode, block *dcrutil.Block, parent *dcrutil.Block, chainParams *chaincfg.Params) error { - // Setup variables. - stakeTransactions := block.STransactions() - blockHash := block.Hash() - - parentStakeNode, err := b.fetchStakeNode(node.parent) - if err != nil { - return err - } - - // Break if the stake system is otherwise disabled. - if block.Height() < stakeValidationHeight { - return nil - } - - // ------------------------------------------------------------------- - // SSRtx Tx Handling - // ------------------------------------------------------------------- - // PER SSRTX - // 1. Ensure that the SSRtx has been marked missed in the ticket patch - // data and, if not, ensure it has been marked missed in the ticket - // database. - // 2. Ensure that at least ticketMaturity many blocks has passed since - // the SStx it references was included in the blockchain. - // PER BLOCK - for _, staketx := range stakeTransactions { - msgTx := staketx.MsgTx() - if stake.IsSSRtx(msgTx) { - // Grab the input SStx hash from the inputs of the - // transaction. - sstxIn := msgTx.TxIn[0] // sstx input - sstxHash := sstxIn.PreviousOutPoint.Hash - - ticketMissed := false - - if parentStakeNode.ExistsMissedTicket(sstxHash) { - ticketMissed = true - } - - if !ticketMissed { - errStr := fmt.Sprintf("Error in stake "+ - "consensus: Ticket %v was not found "+ - "to be missed in the stake patch or "+ - "database, yet block %v spends it!", - sstxHash, blockHash) - return ruleError(ErrInvalidSSRtx, errStr) - } - } - } - - return nil -} - // CheckTransactionInputs performs a series of checks on the inputs to a // transaction to ensure they are valid. An example of some of the checks // include verifying all inputs exist, ensuring the coinbase seasoning @@ -2387,14 +2360,6 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block, parent *dcrutil.B return err } - err = b.CheckBlockStakeSanity(b.chainParams.StakeValidationHeight, node, - block, parent, b.chainParams) - if err != nil { - log.Tracef("CheckBlockStakeSanity failed for incoming node "+ - "%v; error given: %v", node.hash, err) - return err - } - // Don't run scripts if this node is before the latest known good // checkpoint since the validity is verified via the checkpoints (all // transactions are included in the merkle root hash and any changes