Skip to content

Fixed ineffectual assignments #19869

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

Merged
merged 1 commit into from
Jul 22, 2019
Merged

Fixed ineffectual assignments #19869

merged 1 commit into from
Jul 22, 2019

Conversation

muesli
Copy link
Contributor

@muesli muesli commented Jul 22, 2019

Fixed assigning values to variables we don't end up using.

Fixed assigning values to variables we don't end up using.
@@ -416,7 +416,7 @@ func startExecNodeStack() (*node.Node, error) {
}

// create enode record
nodeTcpConn, err := net.ResolveTCPAddr("tcp", conf.Stack.P2P.ListenAddr)
nodeTcpConn, _ := net.ResolveTCPAddr("tcp", conf.Stack.P2P.ListenAddr)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we rather handle this, than ignore? Ping @nonsense @fjl

@@ -130,7 +130,7 @@ func (peer *Peer) handshake() error {
}
}

isRemotePeerLightNode, err := s.Bool()
isRemotePeerLightNode, _ := s.Bool()
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we rather handle this, than ignore? Ping @gballet

Copy link
Member

Choose a reason for hiding this comment

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

this is a feature required by status, I don't think Parity has it. So if that field is not present, then isRemotePeerLightNode == false should be assumed.

@karalabe karalabe added this to the 1.9.1 milestone Jul 22, 2019
@karalabe karalabe merged commit a32a2b9 into ethereum:master Jul 22, 2019
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