blockchain: update BestState. #1416

dnldd · 2018-08-22T04:25:49Z

This adds PrevHash, NextPoolSize, NextStakeDiff, NextWinningTickets, MissedTickets and NextFinalState fields to BestState.

Port of upstream commit c57c18c. In preparation of the removal of chainState.

davecgh · 2018-08-22T04:39:35Z

blockchain/chain.go

+	TotalTxns      uint64           // The total number of txns in the chain.
+	MedianTime     time.Time        // Median time as per CalcPastMedianTime.
+	TotalSubsidy   int64            // The total subsidy for the chain.
+	WinningTickets []chainhash.Hash // The eligigle tickets to vote on the next block.


davecgh · 2018-08-22T04:47:30Z

blockchain/chain.go

+	TotalSubsidy   int64            // The total subsidy for the chain.
+	WinningTickets []chainhash.Hash // The eligigle tickets to vote on the next block.
+	MissedTickets  []chainhash.Hash // The missed tickets set to be revoked.
+	FinalState     [6]byte          // The calculated state of the lottery.


This is never set. Please don't add any fields that are invalid.

To follow up, this can simply be set in newBestState from node.finalState.

davecgh · 2018-08-22T04:50:33Z

blockchain/chain.go

@@ -934,9 +951,15 @@ func (b *BlockChain) disconnectBlock(node *blockNode, block, parent *dcrutil.Blo
 	subsidy := CalculateAddedSubsidy(block, parent)
 	newTotalSubsidy := curTotalSubsidy - subsidy

+	// Calcultate the next stake difficulty.
+	nextStakeDiff, err := b.CalcNextRequiredStakeDifficulty()


This is wrong. It would calculate the stake diff for the block after the one being disconnected. You can just use the data that is already in node.sbits though since it's about to be disconnected and hence is the next stake difficulty as compared to the previous block for which the new state represents.

davecgh · 2018-08-22T04:52:59Z

blockchain/chainio.go

@@ -1463,12 +1463,19 @@ func (b *BlockChain) createChainState() error {
 	// genesis block, use its timestamp for the median time.
 	numTxns := uint64(len(genesisBlock.MsgBlock().Transactions))
 	blockSize := uint64(genesisBlock.MsgBlock().SerializeSize())
+
+	// Calcultate the next stake difficulty.
+	nextStakeDiff, err := b.CalcNextRequiredStakeDifficulty()


No need to calculate this when creating a chain state for the genesis block. The next stake difficulty is already well known for the block after the genesis block. In particular, it's b.chainParams.MinimumStakeDiff.

davecgh · 2018-08-22T04:54:47Z

blockchain/chain.go

@@ -761,9 +772,15 @@ func (b *BlockChain) connectBlock(node *blockNode, block, parent *dcrutil.Block,
 	// Calculate the exact subsidy produced by adding the block.
 	subsidy := CalculateAddedSubsidy(block, parent)

+	// Calcultate the next stake difficulty.
+	nextStakeDiff, err := b.CalcNextRequiredStakeDifficulty()


You should use b.calcNextRequiredStakeDifficulty(node) instead to be sure you're calculating it for the correct node. Otherwise, the tip isn't updated yet, so you'll be off by one.

davecgh · 2018-08-22T04:57:58Z

blockchain/chainio.go

@@ -1770,9 +1777,16 @@ func (b *BlockChain) initChainState(interrupt <-chan struct{}) error {
 		block := utilBlock.MsgBlock()
 		blockSize := uint64(block.SerializeSize())
 		numTxns := uint64(len(block.Transactions))
+
+		// Calcultate the next stake difficulty.
+		nextStakeDiff, err := b.CalcNextRequiredStakeDifficulty()


You should use b.calcNextRequiredStakeDifficulty(tip) instead to be sure you're calculating it for the correct node.

davecgh · 2018-08-22T05:43:35Z

blockchain/chain.go

-		MedianTime:   medianTime,
-		TotalSubsidy: totalSubsidy,
+		Hash:           node.hash,
+		PrevHash:       node.parent.hash,


This will crash if called on a new chain with the genesis block, because it has not parent.

Needs to be:

prevHash := zeroHash if node.parent != nil { prevHash = &node.parent.hash } return &BestState{ ... PrevHash: *prevHash, ... }

davecgh · 2018-08-22T05:46:26Z

blockchain/chain.go

+		TotalTxns:      totalTxns,
+		MedianTime:     medianTime,
+		TotalSubsidy:   totalSubsidy,
+		WinningTickets: node.stakeNode.Winners(),


This could easily crash with node.stakeNode not being loaded since the field is dynamically loaded. You need to make sure that you call fetchStakeNode prior to invoking newBestState with these changes.

In the case of connectBlock and disconnectBlock, there are already calls to load the stake nodes, but they are after the calls to newBestState, so the loading code should be moved above the blocks of code which deal with setting the best state.

For example:

diff --git a/blockchain/chain.go b/blockchain/chain.go index 4856d241..3bc0c86a 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -747,6 +747,15 @@ func (b *BlockChain) connectBlock(node *blockNode, block, parent *dcrutil.Block, return err } + // Get the stake node for this node, filling in any data that + // may have yet to have been filled in. In all cases this + // should simply give a pointer to data already prepared, but + // run this anyway to be safe. + stakeNode, err := b.fetchStakeNode(node) + if err != nil { + return err + } + // Generate a new best state snapshot that will be used to update the // database and later memory if all database updates are successful. b.stateLock.RLock() @@ -765,15 +774,6 @@ func (b *BlockChain) connectBlock(node *blockNode, block, parent *dcrutil.Block, state := newBestState(node, blockSize, numTxns, curTotalTxns+numTxns, node.CalcPastMedianTime(), curTotalSubsidy+subsidy) - // Get the stake node for this node, filling in any data that - // may have yet to have been filled in. In all cases this - // should simply give a pointer to data already prepared, but - // run this anyway to be safe. - stakeNode, err := b.fetchStakeNode(node) - if err != nil { - return err - } - // Atomically insert info into the database. err = b.db.Update(func(dbTx database.Tx) error { // Update best block state.

In the case of the init path, you'll need to load the stake nodes.

davecgh · 2018-08-22T06:13:59Z

blockchain/chain.go

-	blockSize := uint64(block.MsgBlock().Header.Size)
-	state := newBestState(node, blockSize, numTxns, curTotalTxns+numTxns,
-		node.CalcPastMedianTime(), curTotalSubsidy+subsidy)
+	// Calcultate the next stake difficulty.


davecgh · 2018-08-22T06:14:21Z

blockchain/chain.go

+		prevHash = node.parent.hash
+	}
+
+	winners := make([]chainhash.Hash, 0)


This is incorrect. Please see my previous comments about the need to call fetchStakeNode and from where.

davecgh · 2018-08-22T09:05:27Z

Commit message and PR desc needs to be updated to reflect reality.

This adds PrevHash, NextPoolSize, NextStakeDiff, NextWinningTickets, MissedTickets and NextFinalState fields to BestState. Port of upstream commit c57c18c. In preparation of the removal of chainState.

…c-peer netsync/manager: prioritize higher height peers for sync peer

davecgh requested changes Aug 22, 2018

View reviewed changes

dnldd force-pushed the port_update_best_state branch 2 times, most recently from 992e591 to 2b5483c Compare August 22, 2018 06:11

davecgh requested changes Aug 22, 2018

View reviewed changes

dnldd force-pushed the port_update_best_state branch 4 times, most recently from ee65f1b to d79bf00 Compare August 22, 2018 07:52

dnldd force-pushed the port_update_best_state branch 3 times, most recently from d08358d to e608cf0 Compare August 22, 2018 22:00

blockchain: update BestState.

c540c2c

This adds PrevHash, NextPoolSize, NextStakeDiff, NextWinningTickets, MissedTickets and NextFinalState fields to BestState. Port of upstream commit c57c18c. In preparation of the removal of chainState.

dnldd force-pushed the port_update_best_state branch from e608cf0 to c540c2c Compare August 22, 2018 22:03

davecgh added this to the 1.4.0 milestone Aug 22, 2018

davecgh approved these changes Aug 23, 2018

View reviewed changes

davecgh merged commit c540c2c into decred:master Aug 23, 2018

davecgh mentioned this pull request Aug 28, 2018

Multi-peer Checklist #1145

Open

33 tasks

JFixby pushed a commit to JFixby/dcrd that referenced this pull request Sep 20, 2019

Merge pull request decred#1416 from cfromknecht/prioritize-higher-syn…

8bb9c0b

…c-peer netsync/manager: prioritize higher height peers for sync peer

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

blockchain: update BestState. #1416

blockchain: update BestState. #1416

dnldd commented Aug 22, 2018 •

edited

Loading

davecgh Aug 22, 2018

davecgh Aug 22, 2018

davecgh Aug 22, 2018

davecgh Aug 22, 2018

davecgh Aug 22, 2018

davecgh Aug 22, 2018

davecgh Aug 22, 2018

davecgh Aug 22, 2018

davecgh Aug 22, 2018 •

edited

Loading

davecgh Aug 22, 2018

davecgh Aug 22, 2018

davecgh commented Aug 22, 2018

blockchain: update BestState. #1416

blockchain: update BestState. #1416

Conversation

dnldd commented Aug 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davecgh Aug 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davecgh commented Aug 22, 2018

dnldd commented Aug 22, 2018 •

edited

Loading

davecgh Aug 22, 2018 •

edited

Loading