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

fix: p2p connect #145

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: p2p connect #145

wants to merge 2 commits into from

Conversation

lockp111
Copy link
Contributor

@lockp111 lockp111 commented Aug 7, 2020

fix p2p connect no return error.
handshake signature should have inner signature structure.

new handshake signature: inner signature structure must be present
WriteP2PMessage error should return
Copy link
Contributor

@maoueh maoueh left a comment

Choose a reason for hiding this comment

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

About the signature data change, the other one, I'll probably merge right away.

byte(ecc.CurveK1),
}
data = append(data, make([]byte, 65, 65)...)
signature, err := ecc.NewSignatureFromData(data)
Copy link
Contributor

Choose a reason for hiding this comment

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

The NewSignatureFromData(data) does this internally:

	signature := Signature{
		Curve:   CurveID(data[0]), // 1 byte
		Content: data[1:],         // 65 bytes for K1 and R1, variable length for WA
	}

Which is equivalent to the previous as far as I can tell. So what is being fixed here exactly? I'm not sure I follow.

Copy link

@angrypie angrypie Jun 16, 2021

Choose a reason for hiding this comment

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

Which is equivalent to the previous as far as I can tell. So what is being fixed here exactly? I'm not sure I follow.

@maoueh Curve is copied to Content, without this fix I'm getting an error

unable to encode message handshake ...... invalid signature: the inner signature structure must be present, was nil

The problem is either here or in the encoder I guess

@@ -303,7 +307,7 @@ func (p *Peer) SendHandshake(info *HandshakeInfo) error {

err = p.WriteP2PMessage(handshake)
if err != nil {
err = errors.Wrapf(err, "sending handshake to %s", p.Address)
return errors.Wrapf(err, "sending handshake to %s", p.Address)
Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely mergeable right away.

Copy link
Contributor

Choose a reason for hiding this comment

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

I merged this fix in.

@angrypie
Copy link

angrypie commented Dec 3, 2021

Hi, sadly It's still relevant. Any news?

@maoueh
Copy link
Contributor

maoueh commented Dec 6, 2021

@angrypie Sorry I missed your previous message in the commit/pr directly.

I still don't get the differences. What is the easier to reproduce for me, should a unit test exhibits the problem from your perspective?

@angrypie
Copy link

angrypie commented Dec 6, 2021

@maoueh Check an error on this line /cmd/eos-p2p-client/main.go:40
I have:
p2p message, invalid signature: the inner signature structure must be present, was nil

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