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.

Sign up for GitHub

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

Jump to bottom

Fix allowing of selecting protocol versions after merge of 1.10.x #373

Merged
merged 1 commit into from
May 11, 2021
Merged

Fix allowing of selecting protocol versions after merge of 1.10.x #373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions cmd/utils/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ var (
}
EthProtocolsFlag = cli.StringFlag{
Name: "eth.protocols",
Usage: "Sets the Ethereum Protocol versions (65|64|63) (default = 65,64,63 first is primary)",
Usage: "Sets the Ethereum Protocol versions (66|65|64) (default = 66,65,64 first is primary)",
Value: "",
}
ClassicFlag = cli.BoolFlag{
Expand Down Expand Up @@ -1731,7 +1731,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
if ctx.GlobalIsSet(EthProtocolsFlag.Name) {
protocolVersions := SplitAndTrim(ctx.GlobalString(EthProtocolsFlag.Name))
if len(protocolVersions) == 0 {
Fatalf("--%s must be comma separated list of %s", EthProtocolsFlag.Name, strings.Join(strings.Fields(fmt.Sprint(ethconfig.Defaults.ProtocolVersions)), ","))
Fatalf("--%s must be comma separated list of %s", EthProtocolsFlag.Name, strings.Join(strings.Fields(fmt.Sprint(eth.DefaultProtocolVersions)), ","))
}

seenVersions := map[uint]interface{}{}
Expand All @@ -1746,7 +1746,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
}

isValid := false
for _, proto := range ethconfig.Defaults.ProtocolVersions {
for _, proto := range eth.DefaultProtocolVersions {
if proto == uint(version) {
isValid = true
seenVersions[uint(version)] = nil
Expand All @@ -1755,15 +1755,15 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
}

if !isValid {
Fatalf("--%s must be comma separated list of %s", EthProtocolsFlag.Name, strings.Join(strings.Fields(fmt.Sprint(ethconfig.Defaults.ProtocolVersions)), ","))
Fatalf("--%s must be comma separated list of %s", EthProtocolsFlag.Name, strings.Join(strings.Fields(fmt.Sprint(eth.DefaultProtocolVersions)), ","))
}
cfg.ProtocolVersions = append(cfg.ProtocolVersions, uint(version))
}
}

// set default protocol versions
if len(cfg.ProtocolVersions) == 0 {
cfg.ProtocolVersions = ethconfig.Defaults.ProtocolVersions
cfg.ProtocolVersions = eth.DefaultProtocolVersions
}

// Set DNS discovery defaults for hard coded networks with DNS defaults.
Expand Down
6 changes: 3 additions & 3 deletions eth/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
if bcVersion != nil {
dbVer = fmt.Sprintf("%d", *bcVersion)
}
log.Info("Initialising Ethereum protocol", "versions", config.ProtocolVersions, "network", config.NetworkId, "dbversion", dbVer)
log.Info("Initialising Ethereum protocol", "network", config.NetworkId, "dbversion", dbVer)

if !config.SkipBcVersionCheck {
if bcVersion != nil && *bcVersion > core.BlockChainVersion {
Expand Down Expand Up @@ -532,7 +532,7 @@ func (s *Ethereum) Engine() consensus.Engine { return s.engine }
func (s *Ethereum) ChainDb() ethdb.Database { return s.chainDb }
func (s *Ethereum) IsListening() bool { return true } // Always listening
func (s *Ethereum) EthVersion() int {
return int(eth.MakeProtocols((*ethHandler)(s.handler), s.networkID, s.ethDialCandidates)[0].Version)
return int(eth.MakeProtocols((*ethHandler)(s.handler), s.networkID, s.config.ProtocolVersions, s.ethDialCandidates)[0].Version)
}
func (s *Ethereum) NetVersion() uint64 { return s.networkID }
func (s *Ethereum) Downloader() *downloader.Downloader { return s.handler.downloader }
Expand All @@ -543,7 +543,7 @@ func (s *Ethereum) BloomIndexer() *core.ChainIndexer { return s.bloomIndexer }
// Protocols returns all the currently configured
// network protocols to start.
func (s *Ethereum) Protocols() []p2p.Protocol {
protos := eth.MakeProtocols((*ethHandler)(s.handler), s.networkID, s.ethDialCandidates)
protos := eth.MakeProtocols((*ethHandler)(s.handler), s.networkID, s.config.ProtocolVersions, s.ethDialCandidates)
if s.config.SnapshotCache > 0 {
protos = append(protos, snap.MakeProtocols((*snapHandler)(s.handler), s.snapDialCandidates)...)
Copy link
Member Author

Choose a reason for hiding this comment

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

At the moment we only have a flag for the eth protocols (66|65|64) and not for the snap protocols. I am not sure if we prefer handling the snap protocol version now or leave them out, as it's just one at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

Noted, thanks for noticing this. SGTM to leave as-is (no flag), then think about adding one when there's more than one to choose from.

}
Expand Down
4 changes: 4 additions & 0 deletions eth/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ var (
syncChallengeTimeout = 15 * time.Second // Time allowance for a node to reply to the sync progress challenge
)

// DefaultProtocolVersions are the supported versions of the `eth` protocol (first
// is primary).
var DefaultProtocolVersions = []uint{eth.ETH66, eth.ETH65, eth.ETH64}
Copy link
Member Author

Choose a reason for hiding this comment

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

This had to be set in the eth package as the protocol has nested packages logic for (eth|snap) protocols.


// txPool defines the methods needed from a transaction pool implementation to
// support all the operations needed by the Ethereum chain protocols.
type txPool interface {
Expand Down
6 changes: 3 additions & 3 deletions eth/protocols/eth/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ type TxPool interface {
}

// MakeProtocols constructs the P2P protocol definitions for `eth`.
func MakeProtocols(backend Backend, network uint64, dnsdisc enode.Iterator) []p2p.Protocol {
protocols := make([]p2p.Protocol, len(ProtocolVersions))
for i, version := range ProtocolVersions {
func MakeProtocols(backend Backend, network uint64, protocolVersions []uint, dnsdisc enode.Iterator) []p2p.Protocol {
protocols := make([]p2p.Protocol, len(protocolVersions))
for i, version := range protocolVersions {
version := version // Closure

protocols[i] = p2p.Protocol{
Expand Down