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

Remove uptimes from Pong messages #2936

Merged
merged 2 commits into from
Apr 11, 2024
Merged

Remove uptimes from Pong messages #2936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
134a439
Remove uptimes from Pong messages
StephenButtolph Apr 11, 2024
afa3585
mockgen
StephenButtolph Apr 11, 2024
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
32 changes: 4 additions & 28 deletions message/messages_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,11 @@ func TestMessage(t *testing.T) {
bytesSaved: false,
},
{
desc: "pong message with no compression no subnet uptimes",
desc: "pong message with no compression",
op: PongOp,
msg: &p2p.Message{
Message: &p2p.Message_Pong{
Pong: &p2p.Pong{
Uptime: 100,
},
Pong: &p2p.Pong{},
},
},
compressionType: compression.TypeNone,
Expand All @@ -99,26 +97,6 @@ func TestMessage(t *testing.T) {
bypassThrottling: true,
bytesSaved: false,
},
{
desc: "pong message with no compression and subnet uptimes",
op: PongOp,
msg: &p2p.Message{
Message: &p2p.Message_Pong{
Pong: &p2p.Pong{
Uptime: 100,
SubnetUptimes: []*p2p.SubnetUptime{
{
SubnetId: testID[:],
Uptime: 100,
},
},
},
},
},
compressionType: compression.TypeNone,
bypassThrottling: true,
bytesSaved: false,
},
{
desc: "Handshake message with no compression",
op: HandshakeOp,
Expand Down Expand Up @@ -699,9 +677,7 @@ func TestInboundMessageToString(t *testing.T) {
// msg that will become the tested InboundMessage
msg := &p2p.Message{
Message: &p2p.Message_Pong{
Pong: &p2p.Pong{
Uptime: 100,
},
Pong: &p2p.Pong{},
},
}
msgBytes, err := proto.Marshal(msg)
Expand All @@ -710,7 +686,7 @@ func TestInboundMessageToString(t *testing.T) {
inboundMsg, err := mb.parseInbound(msgBytes, ids.EmptyNodeID, func() {})
require.NoError(err)

require.Equal("NodeID-111111111111111111116DBWJs Op: pong Message: uptime:100", inboundMsg.String())
require.Equal("NodeID-111111111111111111116DBWJs Op: pong Message: ", inboundMsg.String())

internalMsg := InternalGetStateSummaryFrontierFailed(ids.EmptyNodeID, ids.Empty, 1)
require.Equal("NodeID-111111111111111111116DBWJs Op: get_state_summary_frontier_failed Message: ChainID: 11111111111111111111111111111111LpoYY RequestID: 1", internalMsg.String())
Expand Down
8 changes: 4 additions & 4 deletions message/mock_outbound_message_builder.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 3 additions & 12 deletions message/outbound_msg_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ type OutboundMsgBuilder interface {
subnetUptimes []*p2p.SubnetUptime,
) (OutboundMessage, error)

Pong(
primaryUptime uint32,
subnetUptimes []*p2p.SubnetUptime,
) (OutboundMessage, error)
Pong() (OutboundMessage, error)

GetStateSummaryFrontier(
chainID ids.ID,
Expand Down Expand Up @@ -216,17 +213,11 @@ func (b *outMsgBuilder) Ping(
)
}

func (b *outMsgBuilder) Pong(
primaryUptime uint32,
subnetUptimes []*p2p.SubnetUptime,
) (OutboundMessage, error) {
func (b *outMsgBuilder) Pong() (OutboundMessage, error) {
return b.builder.createOutbound(
&p2p.Message{
Message: &p2p.Message_Pong{
Pong: &p2p.Pong{
Uptime: primaryUptime,
SubnetUptimes: subnetUptimes,
},
Pong: &p2p.Pong{},
},
},
compression.TypeNone,
Expand Down
16 changes: 2 additions & 14 deletions network/peer/peer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,8 +776,7 @@ func (p *peer) handle(msg message.InboundMessage) {
func (p *peer) handlePing(msg *p2p.Ping) {
p.observeUptimes(msg.Uptime, msg.SubnetUptimes)

primaryUptime, subnetUptimes := p.getUptimes()
pongMessage, err := p.MessageCreator.Pong(primaryUptime, subnetUptimes)
pongMessage, err := p.MessageCreator.Pong()
if err != nil {
p.Log.Error("failed to create message",
zap.Stringer("messageOp", message.PongOp),
Expand Down Expand Up @@ -826,20 +825,9 @@ func (p *peer) getUptimes() (uint32, []*p2p.SubnetUptime) {
return primaryUptimePercent, subnetUptimes
}

func (p *peer) handlePong(msg *p2p.Pong) {
// TODO: Remove once everyone sends uptimes in Ping messages.
p.observeUptimes(msg.Uptime, msg.SubnetUptimes)
}
func (*peer) handlePong(*p2p.Pong) {}

func (p *peer) observeUptimes(primaryUptime uint32, subnetUptimes []*p2p.SubnetUptime) {
// TODO: Remove once everyone sends uptimes in Ping messages.
//
// If primaryUptime is 0, the message may not include any uptimes. This may
// happen with old Ping messages or new Pong messages.
if primaryUptime == 0 {
return
}

if primaryUptime > 100 {
p.Log.Debug("dropping message with invalid uptime",
zap.Stringer("nodeID", p.id),
Expand Down
10 changes: 2 additions & 8 deletions proto/p2p/p2p.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,9 @@ message SubnetUptime {
uint32 uptime = 2;
}

// Pong is sent in response to a Ping with the perceived uptime of the
// peer.
// Pong is sent in response to a Ping.
message Pong {
// Deprecated: uptime is now sent in Ping
// Uptime percentage on the primary network [0, 100]
uint32 uptime = 1;
// Deprecated: uptime is now sent in Ping
// Uptime percentage on subnets
repeated SubnetUptime subnet_uptimes = 2;
reserved 1, 2; // Until E upgrade is activated.
}

// Handshake is the first outbound message sent to a peer when a connection is
Expand Down
Loading
Loading