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

multi: add cpuminerConfig. #1423

Merged
merged 1 commit into from
Sep 13, 2018
Merged

multi: add cpuminerConfig. #1423

merged 1 commit into from
Sep 13, 2018

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Aug 25, 2018

This PR requires #1417, #1418, #1419, #1420, #1421 and #1422

Port of upstream commit 6719014.

@davecgh davecgh added this to the 1.4.0 milestone Sep 5, 2018
@dnldd dnldd force-pushed the cpu_miner_cfg branch 2 times, most recently from db548a3 to c3fd242 Compare September 7, 2018 13:24
@dnldd dnldd force-pushed the cpu_miner_cfg branch 2 times, most recently from 83a8b67 to 98dd3f3 Compare September 12, 2018 22:57
cpuminer.go Outdated
// g identifies the instance to use in order to generate
// block templates that the miner will attempt to solve.
g *BlkTmplGenerator
// ProcessBlock defines the function to call with any solved blocks.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs blank line before for consistency. Same with the others.

cpuminer.go Outdated
type cpuminerConfig struct {
// g identifies the instance to use in order to generate
// block templates that the miner will attempt to solve.
g *BlkTmplGenerator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please match the upstream commit. It will need to be exported to be able to be set from another package once it's moved to its own and it should be named BlockTemplateGenerator.

Also, this struct should have ChainParams as the first field, just like upstream, because, once again, they will need to be provided to the package in question when it's separated.

I see that it is missing MiningAddrs too.

cpuminer.go Outdated
@@ -151,7 +175,7 @@ func (m *CPUMiner) submitBlock(block *dcrutil.Block) bool {
// Occasionally errors are given out for timing errors with
// ReduceMinDifficulty and high block works that is above
// the target. Feed these to debug.
if m.server.chainParams.ReduceMinDifficulty &&
if m.cfg.g.chainParams.ReduceMinDifficulty &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m.cfg.ChainParams.

cpuminer.go Outdated
@@ -138,7 +162,7 @@ func (m *CPUMiner) submitBlock(block *dcrutil.Block) bool {

// Process this block using the same rules as blocks coming from other
// nodes. This will in turn relay it to the network like normal.
isOrphan, err := m.server.blockManager.ProcessBlock(block, blockchain.BFNone)
isOrphan, err := m.cfg.g.blockManager.ProcessBlock(block, blockchain.BFNone)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m.cfg.ProcessBlock

cpuminer.go Outdated
@@ -64,8 +89,7 @@ var (
// system which is typically sufficient.
type CPUMiner struct {
sync.Mutex
g *BlkTmplGenerator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be retained and just set to cfg.BlockTemplateGenerator in newCPUMiner just like upstream.

cpuminer.go Outdated
@@ -546,10 +570,10 @@ func (m *CPUMiner) GenerateNBlocks(n uint32) ([]*chainhash.Hash, error) {
m.Lock()

// Respond with an error if there's virtually 0 chance of CPU-mining a block.
if !m.server.chainParams.GenerateSupported {
if !m.cfg.g.chainParams.GenerateSupported {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m.cfg.ChainParams

cpuminer.go Outdated
@@ -600,7 +624,7 @@ func (m *CPUMiner) GenerateNBlocks(n uint32) ([]*chainhash.Hash, error) {
// Create a new block template using the available transactions
// in the memory pool as a source of transactions to potentially
// include in the block.
template, err := m.g.NewBlockTemplate(payToAddr)
template, err := m.cfg.g.NewBlockTemplate(payToAddr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to change this from m.g.NewBlockTempate once the other comments are addressed.

cpuminer.go Outdated
now.After(lastGenerated.Add(3*time.Second))) ||
now.After(lastGenerated.Add(60*time.Second)) {
return false
}

err = UpdateBlockTime(msgBlock, m.server.blockManager)
err = UpdateBlockTime(msgBlock, m.cfg.g.blockManager)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there needs to be another preliminary commit before this one to remove the rest of the dependencies on the block manager. Notice how the upstream commit by this point no longer needs a reference to it and that is intentional.

cpuminer.go Outdated
@@ -303,7 +327,7 @@ out:
// Hacks to make dcr work with Decred PoC (simnet only)
// TODO Remove before production.
if cfg.SimNet {
curHeight := m.server.blockManager.chain.BestSnapshot().Height
curHeight := m.cfg.g.chain.BestSnapshot().Height
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m.g....

cpuminer.go Outdated
m.Unlock()
return nil, errors.New("no support for `generate` on the current " +
"network, " + m.server.chainParams.Net.String() +
"network, " + m.cfg.g.chainParams.Net.String() +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m.cfg.ChainParams

cpuminer.go Outdated Show resolved Hide resolved
@dnldd dnldd force-pushed the cpu_miner_cfg branch 2 times, most recently from 4056595 to 19357a4 Compare September 13, 2018 15:59
@dnldd dnldd closed this Sep 13, 2018
@dnldd dnldd reopened this Sep 13, 2018
Port of upstream commit 6719014.
@davecgh davecgh merged commit 88ca6f5 into decred:master Sep 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants