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

miner: move agent logic to worker #17351

Merged
merged 3 commits into from
Aug 14, 2018

Conversation

rjl493456442
Copy link
Member

@rjl493456442 rjl493456442 commented Aug 8, 2018

Refactor miner package a bit and fix some issues.

This PR includes:

  • Remove cpu agent stuff and move all logic to worker
  • Fix a issue that we should fire ChainEvent only when we insert a canonical block
  • Push full sealing work only there are some available pending transactions in the txpool
  • Add some unit tests

@rjl493456442 rjl493456442 force-pushed the remove_mining_agent branch 6 times, most recently from 5c01fa1 to 8ddee3f Compare August 10, 2018 02:44
miner/worker.go Outdated
// worker is the main object which takes care of applying messages to the new state
type worker struct {
// Task contains all information for consensus engine sealing and result submitting.
type Task struct {
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be public?

miner/worker.go Outdated

// Worker is the main object which takes care of submitting new work to consensus engine
// and gathering the sealing result.
type Worker struct {
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, does the worker need to be public?

miner/worker.go Outdated

block := result.block
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick, please move this back above the empty line. It's easier on the eye that the first core-block is "initialization" and then an empty line, then comment and continuation.

miner/worker.go Outdated
// makeCurrent creates a new environment for the current cycle.
func (self *worker) makeCurrent(parent *types.Block, header *types.Header) error {
state, err := self.chain.StateAt(parent.Root())
// makeCurrent creates an new environment for the current cycle.
Copy link
Member

Choose a reason for hiding this comment

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

"an" -> "a"

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

LGTM

@karalabe karalabe added this to the 1.8.14 milestone Aug 14, 2018
@karalabe karalabe merged commit a1783d1 into ethereum:master Aug 14, 2018
firmianavan pushed a commit to firmianavan/go-ethereum that referenced this pull request Aug 28, 2018
* miner: move agent logic to worker

* miner: polish

* core: persist block before reorg
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