multi: add BlkTmplGenerator. #1422
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dnldd
force-pushed
the
add_blk_template_gen
branch
from
742aaa8 to
c9e3928
Compare
davecgh
requested changes
Sep 12, 2018
mining.go
Outdated
| // BlkTmplGenerator generates block templates based on a given mining policy | ||
| // and a transactions source. It also houses additional state required in | ||
| // order to ensure the templates are built on top of the current best | ||
| // chain and adhere to the consensus rules. |
There was a problem hiding this comment.
This is missing some of the verbiage from the commit being ported. Also, it needs to be updated to reflect reality for Decred. Specifically that templates can either be on the best chain tip or its parent in the case the best chain tip is unable to get enough votes.
mining.go
Outdated
| @@ -1365,7 +1380,7 @@ mempoolLoop: | |||
|
|
|||
| // Skip if we already have too many SStx. | |||
| if isSStx && (numSStx >= | |||
| int(server.chainParams.MaxFreshStakePerBlock)) { | |||
| int(g.blockManager.server.chainParams.MaxFreshStakePerBlock)) { | |||
There was a problem hiding this comment.
Please make the creation of the block template generator takes the chain params and use it directly from g. A large reason for having the block template generator is to reduce the dependencies on the other types so that it can be more easily split into a separate package.
mining.go
Outdated
| @@ -1499,15 +1514,16 @@ mempoolLoop: | |||
| // The fraud proof is not checked because it will be filled in | |||
| // by the miner. | |||
| _, err = blockchain.CheckTransactionInputs(subsidyCache, tx, | |||
| nextBlockHeight, blockUtxos, false, server.chainParams) | |||
| nextBlockHeight, blockUtxos, false, | |||
| g.blockManager.server.chainParams) | |||
There was a problem hiding this comment.
Same here regarding chain params.
mining.go
Outdated
| if err != nil { | ||
| minrLog.Tracef("Skipping tx %s due to error in "+ | ||
| "CheckTransactionInputs: %v", tx.Hash(), err) | ||
| logSkippedDeps(tx, deps) | ||
| continue | ||
| } | ||
| err = blockchain.ValidateTransactionScripts(tx, blockUtxos, | ||
| scriptFlags, server.sigCache) | ||
| scriptFlags, g.blockManager.server.sigCache) |
There was a problem hiding this comment.
The same applies to the signature cache. It should be specified as part of BlkTmplGenerator and directly accessible in order to break the dependency on server, just like the upstream commit does.
e.g.
type BlkTmplGenerator struct {
policy *mining.Policy
txSource mining.TxSource
sigCache *txscript.SigCache
blockManager *blockManager
timeSource blockchain.MedianTimeSource
}
mining.go
Outdated
| @@ -1751,7 +1768,7 @@ mempoolLoop: | |||
| nextBlockHeight, | |||
| payToAddress, | |||
| uint16(voters), | |||
| server.chainParams) | |||
| g.blockManager.server.chainParams) | |||
mining.go
Outdated
| @@ -1633,7 +1649,7 @@ mempoolLoop: | |||
|
|
|||
| // Retrieve the current top block, whose TxTreeRegular was voted | |||
| // out. | |||
| topBlock, err := blockManager.chain.BlockByHash(&prevHash) | |||
| topBlock, err := g.blockManager.chain.BlockByHash(&prevHash) | |||
There was a problem hiding this comment.
Same thing here regarding the chain. Basically anything that is a component of block manager should be provided in the BlkTmplGenerator struct instead to decouple them.
mining.go
Outdated
| @@ -1853,13 +1870,13 @@ mempoolLoop: | |||
| // bit for the mempool to sync with the votes map and we end up down | |||
| // here despite having the relevant votes available in the votes map. | |||
| minimumVotesRequired := | |||
| int((server.chainParams.TicketsPerBlock / 2) + 1) | |||
| int((g.blockManager.server.chainParams.TicketsPerBlock / 2) + 1) | |||
mining.go
Outdated
| @@ -1938,13 +1955,13 @@ mempoolLoop: | |||
|
|
|||
| // Choose the block version to generate based on the network. | |||
| blockVersion := int32(generatedBlockVersion) | |||
| if server.chainParams.Net != wire.MainNet { | |||
| if g.blockManager.server.chainParams.Net != wire.MainNet { | |||
mining.go
Outdated
| generatedStakeVersion, err := blockManager.chain. | ||
| CalcStakeVersionByHash(&prevHash) | ||
| generatedStakeVersion, err := | ||
| g.blockManager.chain.CalcStakeVersionByHash(&prevHash) |
mining.go
Outdated
| @@ -1991,7 +2008,7 @@ mempoolLoop: | |||
| // consensus rules to ensure it properly connects to the current best | |||
| // chain with no issues. | |||
| block := dcrutil.NewBlockDeepCopyCoinbase(&msgBlock) | |||
| err = blockManager.chain.CheckConnectBlockTemplate(block) | |||
| err = g.blockManager.chain.CheckConnectBlockTemplate(block) | |||
dnldd
force-pushed
the
add_blk_template_gen
branch
6 times, most recently
from
af75e2c to
c906c86
Compare
dnldd
force-pushed
the
add_blk_template_gen
branch
from
c906c86 to
3b33c0c
Compare
Port of upstream commit 74fe2a4. In preparation of moving all mining related code into the mining package.
davecgh
approved these changes
Sep 12, 2018
This was referenced Sep 12, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR requires #1417, #1418, #1419, #1420 and #1421
Port of upstream commit 74fe2a4. In preparation of moving all
mining related code into the mining package.