Skip to content

Conversation

@renaynay
Copy link
Contributor

No description provided.

Hash common.Hash // Hash of one particular block being announced
Number uint64 // Number of one particular block being announced
type NewBlockHashes struct {
*eth.NewBlockHashesPacket
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be better not to embed it here. If you just use

type NewBlockHashes eth.NewBlockHashesPacket

you can still use the fields of the upstream definition, but don't need to wrap all the constructor call sites

@renaynay renaynay force-pushed the replace_message_types_ethtestsuite branch from 270c44e to e3505e2 Compare February 15, 2021 17:21
@renaynay renaynay force-pushed the replace_message_types_ethtestsuite branch from e3505e2 to 5b0e3cb Compare February 15, 2021 17:24
@renaynay renaynay requested a review from fjl February 15, 2021 17:57
@renaynay renaynay marked this pull request as ready for review February 15, 2021 17:58
@renaynay renaynay changed the title [DRAFT]: replace message types ethtestsuite cmd/devp2p: replace message types ethtestsuite Feb 15, 2021
Copy link
Contributor

@fjl fjl left a comment

Choose a reason for hiding this comment

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

Looks very good now, just need to remove this extra type.

@fjl fjl changed the title cmd/devp2p: replace message types ethtestsuite cmd/devp2p/internal/ethtest: replace message types ethtestsuite Feb 16, 2021
@fjl fjl changed the title cmd/devp2p/internal/ethtest: replace message types ethtestsuite cmd/devp2p/internal/ethtest: use shared message types Feb 16, 2021
@fjl fjl merged commit f9445e9 into ethereum:master Feb 16, 2021
@renaynay renaynay deleted the replace_message_types_ethtestsuite branch February 16, 2021 14:56
filipescuc pushed a commit to EthereumGenesys/go-ethereum that referenced this pull request Mar 2, 2021
This updates the eth protocol test suite to use the message type
definitions of the 'production' protocol implementation in eth/protocols/eth.
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