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: add support for --miner.notify.full flag #22558

Merged
merged 20 commits into from
Mar 26, 2021

Conversation

fjl
Copy link
Contributor

@fjl fjl commented Mar 23, 2021

This is a resubmit of #22323 with some additional fixes.
The PR implements the --miner.notify.full flag that enables full pending block notifications. Extends --miner.notify flag.

Notification without this flag:

POST / HTTP/1.1
Host: localhost
User-Agent: Go-http-client/1.1
Content-Length: 219
Content-Type: application/json
Accept-Encoding: gzip

["0x4ca621099304576a5d482bd5530d4fabfac22e6a5cc523b6b5b8209cf293bb89","0x1438f845a55515d47e39b80d72f530939c61d6b1e9cd28c2396c5bf25e4ac3a2","0x00000002fc4aad70f11023a1a899d5a5ae39540cf37b101aa84828e94e325419","0x93440c"]

Notification with this flag:

POST / HTTP/1.1
Host: localhost
User-Agent: Go-http-client/1.1
Content-Length: 1341
Content-Type: application/json
Accept-Encoding: gzip

{"difficulty":"0x4f8359f2","extraData":"0x","gasLimit":"0x7a1200","gasUsed":"0x6f328c","hash":"0xb218996ce8127b4dbacbc6058b298602338f88e5e7a61973ee2493803297706a","logsBloom":"0x0000000000000400000000a0001000000000040000000004000040000004000000000000000000000000410000000000000000000000002000000000002420000000101044008000000000080000040000002000001400044001000040000000000040000200000002000800004000000000000000020000000010180400000002a4004000000000000000000000000000001404021000000000000000000000020004000000000420000000000000000000804000000828001000000000000010000502000000000000000000000000000440005000008008400200010040600010000000400000004000000000000000000000010000014000011000800000","miner":"0000000000000000000000000000000000000000","mixHash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0000000000000000","number":"0x934054","parentHash":"0x5c38bf02ae71e2b27fddd96a36b4896c2f0716ee8f8a76839c7fd5b6fd1f129a","receiptsRoot":"0xe6413d8544987151d2c52000de5b950b818156447110360ab8963f7577817f45","sha3Uncles":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","size":"0x6df5","stateRoot":"0xa65f8d3d3833bbbed03162287db97ce382bd82073c1ccbec8b8dc9f2c6a906cf","timestamp":"0x6026bc67","transactionsRoot":"0x1e552804d6b796c6675c54b1a4b74faa820f1cf8cb5c52c8fd585e920b1111e5","uncles":[]}

@fjl fjl added this to the 1.10.2 milestone Mar 23, 2021
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

Some of these changes look wrong to me

case ethash.ModeTest:
log.Warn("Ethash used in test mode")
return ethash.NewTester(nil, noverify)
Copy link
Contributor

Choose a reason for hiding this comment

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

The NewTester did some more stuff that is now skipped:

func NewTester(notify []string, noverify bool) *Ethash {
	ethash := &Ethash{
		config:   Config{PowMode: ModeTest, Log: log.Root()},
		caches:   newlru("cache", 1, newCache),
		datasets: newlru("dataset", 1, newDataset),
		update:   make(chan struct{}),
		hashrate: metrics.NewMeterForced(),
	}
	ethash.remote = startRemoteSealer(ethash, notify, noverify)

Copy link
Contributor Author

@fjl fjl Mar 24, 2021

Choose a reason for hiding this comment

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

The code in NewTester was just a copy of the normal constructor. I've fixed it now to just call that instead.

case ethash.ModeShared:
log.Warn("Ethash used in shared mode")
return ethash.NewShared()
Copy link
Contributor

Choose a reason for hiding this comment

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

The idea here is to use a shared instance, but that's just ignored in these changes...?

func NewShared() *Ethash {
	return &Ethash{shared: sharedEthash}
}

Copy link
Contributor Author

@fjl fjl Mar 24, 2021

Choose a reason for hiding this comment

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

This is now fixed as well. The main constructor New, now sets shared when ModeShared is used.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

Looks ok to me

@fjl fjl merged commit cae6b55 into ethereum:master Mar 26, 2021
atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
…thereum#22558)

The PR implements the --miner.notify.full flag that enables full pending block
notifications. When this flag is used, the block notifications sent to mining
endpoints contain the complete block header JSON instead of a work package
array.

Co-authored-by: AlexSSD7 <alexandersadovskyi7@protonmail.com>
Co-authored-by: Martin Holst Swende <martin@swende.se>
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.

3 participants