Skip to content

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

return ethash.NewFaker()
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.

return ethash.NewTester(nil, noverify)
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 <[email protected]>
Co-authored-by: Martin Holst Swende <[email protected]>
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