-
Notifications
You must be signed in to change notification settings - Fork 663
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
Mismatched handshake #12690
Mismatched handshake #12690
Conversation
18692df
to
049021a
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12690 +/- ##
==========================================
+ Coverage 70.53% 70.65% +0.12%
==========================================
Files 847 848 +1
Lines 172839 173802 +963
Branches 172839 173802 +963
==========================================
+ Hits 121904 122806 +902
- Misses 45833 45871 +38
- Partials 5102 5125 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@saketh-are maybe you can help review. |
@mkamonMdt thank you for looking into this. I think you are correct that
However, there are other consumers of I think what should be done here is to add a new argument At the same time, we can rename the |
During routing of StateRequestPart the PeerInfo used was taken from connection (prev_hop) instead of actual message author (msg_author). That results in malformed PeerInfo that is coposed of prev_hop-party ID and the original msg_author's address.
049021a
to
06a2e78
Compare
@@ -1029,7 +1030,8 @@ impl PeerActor { | |||
Self::receive_routed_message( | |||
&clock, | |||
&network_state, | |||
peer_id, | |||
msg.msg.author, | |||
peer_id.clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is a bit confusing, why do we need a .clone()
now if we did not before?
The detailed description of the problem is available at #12364