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
Full archive heartbeat sender + new data pools #5332
Full archive heartbeat sender + new data pools #5332
Conversation
…well added todos for further implementation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## feat/multiple_p2p_messengers #5332 +/- ##
================================================================
- Coverage 80.29% 80.26% -0.04%
================================================================
Files 695 695
Lines 90189 90244 +55
================================================================
+ Hits 72416 72432 +16
- Misses 12595 12631 +36
- Partials 5178 5181 +3
☔ View full report in Codecov by Sentry. |
@@ -40,12 +40,12 @@ func (cpas *commonPeerAuthenticationSender) generateMessageBytes( | |||
msg.Payload = payloadBytes | |||
|
|||
if p2pSkBytes != nil { | |||
msg.PayloadSignature, err = cpas.messenger.SignUsingPrivateKey(p2pSkBytes, payloadBytes) | |||
msg.PayloadSignature, err = cpas.mainMessenger.SignUsingPrivateKey(p2pSkBytes, payloadBytes) |
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.
is this ok?
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.
yes, as this message won't be sent on the full archive network anyway.. mainMessenger = the one from main network
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.
ok
@@ -184,7 +185,8 @@ func (sender *multikeyHeartbeatSender) sendMessageForKey(pkBytes []byte) error { | |||
return err | |||
} | |||
|
|||
sender.messenger.BroadcastUsingPrivateKey(sender.topic, buff, pid, p2pSk) | |||
sender.mainMessenger.BroadcastUsingPrivateKey(sender.topic, buff, pid, p2pSk) | |||
sender.fullArchiveMessenger.BroadcastUsingPrivateKey(sender.topic, buff, pid, p2pSk) |
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.
debatable if we need to send the managed key also on the full archive network. Add a TODO to rethink about this?
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.
added
t.Parallel() | ||
|
||
argsBase := createMockBaseArgs() | ||
argsBase.fullArchiveMessenger = nil |
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.
the MultikeyPeerAuthenticationSender should not care about the full archive network.
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.
the fullArchiveMessenger is part of the baseSender, used in all heartbeat, multikeyHeartbeat, peerAuthentication, multikeyPeerAuthentication.. nil checks are done as part of the baseSender
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.
ok
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
feat
branch created?feat
branch merging, do all satellite projects have a proper tag insidego.mod
?