Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

blockchain: Simplify blocknode construction. #1056

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions blockchain/accept.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags)
return false, ruleError(ErrMissingParent, str)
}

blockHeight := block.Height()

// The block must pass all of the validation rules which depend on the
// position of the block within the block chain.
err = b.checkBlockContext(block, prevNode, flags)
Expand Down Expand Up @@ -160,11 +158,8 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags)
// Create a new block node for the block and add it to the in-memory
// block chain (could be either a side chain or the main chain).
blockHeader := &block.MsgBlock().Header
newNode := newBlockNode(blockHeader,
stake.FindSpentTicketsInBlock(block.MsgBlock()))
newNode.parent = prevNode
newNode.height = blockHeight
newNode.workSum.Add(prevNode.workSum, newNode.workSum)
newNode := newBlockNode(blockHeader, prevNode)
newNode.populateTicketInfo(stake.FindSpentTicketsInBlock(block.MsgBlock()))

// Fetching a stake node could enable a new DoS vector, so restrict
// this only to blocks that are recent in history.
Expand Down
80 changes: 42 additions & 38 deletions blockchain/blockindex.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,10 @@ type blockNode struct {
votes []stake.VoteVersionTuple
}

// newBlockNode returns a new block node for the given block header. It is
// completely disconnected from the chain and the workSum value is just the work
// for the passed block. The work sum is updated accordingly when the node is
// inserted into a chain.
func newBlockNode(blockHeader *wire.BlockHeader, spentTickets *stake.SpentTicketsInBlock) *blockNode {
// newBlockNode returns a new block node for the given block header and parent
// node. The workSum is calculated based on the parent, or, in the case no
// parent is provided, it will just be the work for the passed block.
func newBlockNode(blockHeader *wire.BlockHeader, parent *blockNode) *blockNode {
// Serialize the block header for use in calculating the initialization
// vector for the ticket lottery. The only way this can fail is if the
// process is out of memory in which case it would panic anyways, so
Expand All @@ -106,42 +105,35 @@ func newBlockNode(blockHeader *wire.BlockHeader, spentTickets *stake.SpentTicket
panic(err)
}

var ticketsVoted, ticketsRevoked []chainhash.Hash
var votes []stake.VoteVersionTuple
if spentTickets != nil {
ticketsVoted = spentTickets.VotedTickets
ticketsRevoked = spentTickets.RevokedTickets
votes = spentTickets.Votes
}

// Make a copy of the hash so the node doesn't keep a reference to part
// of the full block/block header preventing it from being garbage
// collected.
node := blockNode{
hash: blockHeader.BlockHash(),
parentHash: blockHeader.PrevBlock,
workSum: CalcWork(blockHeader.Bits),
height: int64(blockHeader.Height),
blockVersion: blockHeader.Version,
voteBits: blockHeader.VoteBits,
finalState: blockHeader.FinalState,
voters: blockHeader.Voters,
freshStake: blockHeader.FreshStake,
poolSize: blockHeader.PoolSize,
bits: blockHeader.Bits,
sbits: blockHeader.SBits,
timestamp: blockHeader.Timestamp.Unix(),
merkleRoot: blockHeader.MerkleRoot,
stakeRoot: blockHeader.StakeRoot,
revocations: blockHeader.Revocations,
blockSize: blockHeader.Size,
nonce: blockHeader.Nonce,
extraData: blockHeader.ExtraData,
stakeVersion: blockHeader.StakeVersion,
lotteryIV: stake.CalcHash256PRNGIV(hB),
ticketsVoted: ticketsVoted,
ticketsRevoked: ticketsRevoked,
votes: votes,
hash: blockHeader.BlockHash(),
parentHash: blockHeader.PrevBlock,
workSum: CalcWork(blockHeader.Bits),
height: int64(blockHeader.Height),
blockVersion: blockHeader.Version,
voteBits: blockHeader.VoteBits,
finalState: blockHeader.FinalState,
voters: blockHeader.Voters,
freshStake: blockHeader.FreshStake,
poolSize: blockHeader.PoolSize,
bits: blockHeader.Bits,
sbits: blockHeader.SBits,
timestamp: blockHeader.Timestamp.Unix(),
merkleRoot: blockHeader.MerkleRoot,
stakeRoot: blockHeader.StakeRoot,
revocations: blockHeader.Revocations,
blockSize: blockHeader.Size,
nonce: blockHeader.Nonce,
extraData: blockHeader.ExtraData,
stakeVersion: blockHeader.StakeVersion,
lotteryIV: stake.CalcHash256PRNGIV(hB),
}
if parent != nil {
node.parent = parent
node.workSum = node.workSum.Add(parent.workSum, node.workSum)
}

return &node
Expand Down Expand Up @@ -174,6 +166,17 @@ func (node *blockNode) Header() wire.BlockHeader {
}
}

// populateTicketInfo sets prunable ticket information in the provided block
// node.
//
// This function is NOT safe for concurrent access. It must only be called when
// initially creating a node.
func (node *blockNode) populateTicketInfo(spentTickets *stake.SpentTicketsInBlock) {
node.ticketsVoted = spentTickets.VotedTickets
node.ticketsRevoked = spentTickets.RevokedTickets
node.votes = spentTickets.Votes
}

// removeChildNode deletes node from the provided slice of child block
// nodes. It ensures the final pointer reference is set to nil to prevent
// potential memory leaks. The original slice is returned unmodified if node
Expand Down Expand Up @@ -253,7 +256,8 @@ func (bi *blockIndex) loadBlockNode(dbTx database.Tx, hash *chainhash.Hash) (*bl

// Create the new block node for the block.
blockHeader := block.MsgBlock().Header
node := newBlockNode(&blockHeader, stake.FindSpentTicketsInBlock(block.MsgBlock()))
node := newBlockNode(&blockHeader, nil)
node.populateTicketInfo(stake.FindSpentTicketsInBlock(block.MsgBlock()))
node.inMainChain = true

// Add the node to the chain.
Expand Down
3 changes: 1 addition & 2 deletions blockchain/blockindex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ func TestBlockNodeHeader(t *testing.T) {
ExtraData: [32]byte{0xbb},
StakeVersion: 5,
}
node := newBlockNode(&testHeader, nil)
node := newBlockNode(&testHeader, bc.bestNode)
bc.index.AddNode(node)
node.parent = bc.bestNode

// Ensure reconstructing the header for the node produces the same header
// used to create the node.
Expand Down
3 changes: 2 additions & 1 deletion blockchain/chainio.go
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,8 @@ func (b *BlockChain) initChainState(interrupt <-chan struct{}) error {
// Create a new node and set it as the best node. The preceding
// nodes will be loaded on demand as needed.
header := &block.Header
node := newBlockNode(header, stake.FindSpentTicketsInBlock(&block))
node := newBlockNode(header, nil)
node.populateTicketInfo(stake.FindSpentTicketsInBlock(&block))
node.inMainChain = true
node.workSum = state.workSum

Expand Down
3 changes: 1 addition & 2 deletions blockchain/difficulty.go
Original file line number Diff line number Diff line change
Expand Up @@ -1087,8 +1087,7 @@ func (b *BlockChain) estimateNextStakeDifficultyV1(curNode *blockNode, ticketsIn
// Connect the header.
emptyHeader.PrevBlock = topNode.hash

thisNode := newBlockNode(&emptyHeader, nil)
thisNode.parent = topNode
thisNode := newBlockNode(&emptyHeader, topNode)
topNode = thisNode
}
}
Expand Down
6 changes: 2 additions & 4 deletions blockchain/difficulty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,7 @@ nextTest:
FreshStake: ticketInfo.newTickets,
PoolSize: poolSize,
}
node := newBlockNode(header, nil)
node.parent = bc.bestNode
node := newBlockNode(header, bc.bestNode)

// Update the pool size for the next header.
// Notice how tickets that mature for this block
Expand Down Expand Up @@ -729,8 +728,7 @@ nextTest:
FreshStake: ticketInfo.newTickets,
PoolSize: poolSize,
}
node := newBlockNode(header, nil)
node.parent = bc.bestNode
node := newBlockNode(header, bc.bestNode)

// Update the pool size for the next header.
// Notice how tickets that mature for this block
Expand Down
5 changes: 2 additions & 3 deletions blockchain/internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package blockchain
import (
"sort"

"github.com/decred/dcrd/blockchain/stake"
"github.com/decred/dcrd/wire"
)

Expand All @@ -40,6 +39,6 @@ func (b *BlockChain) TstCheckBlockHeaderContext(header *wire.BlockHeader, prevNo

// TstNewBlockNode makes the internal newBlockNode function available to the
// test package.
func TstNewBlockNode(blockHeader *wire.BlockHeader, spentTickets *stake.SpentTicketsInBlock) *blockNode {
return newBlockNode(blockHeader, spentTickets)
func TstNewBlockNode(blockHeader *wire.BlockHeader, parent *blockNode) *blockNode {
return newBlockNode(blockHeader, parent)
}
5 changes: 1 addition & 4 deletions blockchain/stakeversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,7 @@ func newFakeNode(parent *blockNode, blockVersion int32, stakeVersion uint32, bit
Timestamp: timestamp,
StakeVersion: stakeVersion,
}
node := newBlockNode(header, nil)
node.parent = parent
node.workSum.Add(parent.workSum, node.workSum)
return node
return newBlockNode(header, parent)
}

// appendFakeVotes appends the passed number of votes to the node with the
Expand Down
6 changes: 2 additions & 4 deletions blockchain/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2601,10 +2601,8 @@ func (b *BlockChain) CheckConnectBlock(block *dcrutil.Block, flags BehaviorFlags
return err
}

newNode := newBlockNode(&block.MsgBlock().Header,
stake.FindSpentTicketsInBlock(block.MsgBlock()))
newNode.parent = prevNode
newNode.workSum.Add(prevNode.workSum, newNode.workSum)
newNode := newBlockNode(&block.MsgBlock().Header, prevNode)
newNode.populateTicketInfo(stake.FindSpentTicketsInBlock(block.MsgBlock()))

// If we are extending the main (best) chain with a new block, just use
// the ticket database we already have.
Expand Down
5 changes: 2 additions & 3 deletions blockchain/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2110,7 +2110,7 @@ func TestCheckWorklessBlockSanity(t *testing.T) {
}

// TestCheckBlockHeaderContext tests that genesis block passes context headers
// because it's previousNode is nil.
// because its parent is nil.
func TestCheckBlockHeaderContext(t *testing.T) {
// Create a new database for the blocks.
params := &chaincfg.SimNetParams
Expand Down Expand Up @@ -2145,8 +2145,7 @@ func TestCheckBlockHeaderContext(t *testing.T) {
// Test failing checkBlockHeaderContext when calcNextRequiredDifficulty
// fails.
block := dcrutil.NewBlock(&badBlock)
newNode := blockchain.TstNewBlockNode(&block.MsgBlock().Header,
&stake.SpentTicketsInBlock{})
newNode := blockchain.TstNewBlockNode(&block.MsgBlock().Header, nil)
err = chain.TstCheckBlockHeaderContext(&block.MsgBlock().Header, newNode, blockchain.BFNone)
if err == nil {
t.Fatalf("Should fail due to bad diff in newNode\n")
Expand Down