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

Add height voting for chits #2102

Merged
merged 22 commits into from
Oct 5, 2023
Merged

Add height voting for chits #2102

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9d70593
Move vote bubble before poll termination
StephenButtolph Sep 26, 2023
53fffe1
merged
StephenButtolph Sep 26, 2023
d9227fc
Update snow/engine/snowman/voter.go
StephenButtolph Sep 26, 2023
400255c
Merge branch 'early-bubble-votes' into bubble-vote-messaging
StephenButtolph Sep 27, 2023
cc6905a
Add height voting for chits
StephenButtolph Sep 27, 2023
80fef0a
Merge branch 'dev' into early-bubble-votes
StephenButtolph Sep 27, 2023
95cf59f
Merge branch 'early-bubble-votes' into bubble-vote-messaging
StephenButtolph Sep 27, 2023
c5cdc20
merged
StephenButtolph Oct 3, 2023
a9fc1ff
merged
StephenButtolph Oct 3, 2023
54e0e11
nit
StephenButtolph Oct 3, 2023
178882b
logging additions
StephenButtolph Oct 3, 2023
f12b555
nit
StephenButtolph Oct 3, 2023
fbb3817
add comment
StephenButtolph Oct 3, 2023
36b3879
add log
StephenButtolph Oct 3, 2023
155eec9
Merge branch 'dev' into bubble-vote-messaging
StephenButtolph Oct 3, 2023
5f1005e
Address PR review comments
StephenButtolph Oct 4, 2023
157a4e8
Add invariant comment
StephenButtolph Oct 4, 2023
061d231
Merge branch 'dev' into bubble-vote-messaging
StephenButtolph Oct 4, 2023
755e374
merged
StephenButtolph Oct 4, 2023
bdf293c
nit
StephenButtolph Oct 4, 2023
83fd55b
add metric
StephenButtolph Oct 5, 2023
a97e487
Merge branch 'dev' into bubble-vote-messaging
StephenButtolph Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 20 additions & 14 deletions message/inbound_msg_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,18 +199,20 @@ func InboundPushQuery(
requestID uint32,
deadline time.Duration,
container []byte,
requestedHeight uint64,
nodeID ids.NodeID,
engineType p2p.EngineType,
) InboundMessage {
return &inboundMessage{
nodeID: nodeID,
op: PushQueryOp,
message: &p2p.PushQuery{
ChainId: chainID[:],
RequestId: requestID,
Deadline: uint64(deadline),
Container: container,
EngineType: engineType,
ChainId: chainID[:],
RequestId: requestID,
Deadline: uint64(deadline),
Container: container,
RequestedHeight: requestedHeight,
EngineType: engineType,
},
expiration: time.Now().Add(deadline),
}
Expand All @@ -221,18 +223,20 @@ func InboundPullQuery(
requestID uint32,
deadline time.Duration,
containerID ids.ID,
requestedHeight uint64,
nodeID ids.NodeID,
engineType p2p.EngineType,
) InboundMessage {
return &inboundMessage{
nodeID: nodeID,
op: PullQueryOp,
message: &p2p.PullQuery{
ChainId: chainID[:],
RequestId: requestID,
Deadline: uint64(deadline),
ContainerId: containerID[:],
EngineType: engineType,
ChainId: chainID[:],
RequestId: requestID,
Deadline: uint64(deadline),
ContainerId: containerID[:],
RequestedHeight: requestedHeight,
EngineType: engineType,
},
expiration: time.Now().Add(deadline),
}
Expand All @@ -242,17 +246,19 @@ func InboundChits(
chainID ids.ID,
requestID uint32,
preferredID ids.ID,
preferredIDAtHeight ids.ID,
acceptedID ids.ID,
nodeID ids.NodeID,
) InboundMessage {
return &inboundMessage{
nodeID: nodeID,
op: ChitsOp,
message: &p2p.Chits{
ChainId: chainID[:],
RequestId: requestID,
PreferredId: preferredID[:],
AcceptedId: acceptedID[:],
ChainId: chainID[:],
RequestId: requestID,
PreferredId: preferredID[:],
PreferredIdAtHeight: preferredIDAtHeight[:],
Copy link
Contributor

Choose a reason for hiding this comment

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

This also allows us to apply votes on a failed chit, right? Or am I too optimistic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use the AcceptedID for failed chits. I don't think it would be safe to re-use the PreferredIDAtHeight in the same way unless we included an additional boolean (to say if PreferredIDAtHeight was accepted).

AcceptedId: acceptedID[:],
},
expiration: mockable.MaxTime,
}
Expand Down
35 changes: 21 additions & 14 deletions message/inbound_msg_builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,19 @@ func Test_newMsgBuilder(t *testing.T) {

func TestInboundMsgBuilder(t *testing.T) {
var (
chainID = ids.GenerateTestID()
requestID uint32 = 12345
deadline = time.Hour
nodeID = ids.GenerateTestNodeID()
summary = []byte{9, 8, 7}
appBytes = []byte{1, 3, 3, 7}
container = []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}
containerIDs = []ids.ID{ids.GenerateTestID(), ids.GenerateTestID()}
acceptedContainerIDs = []ids.ID{ids.GenerateTestID(), ids.GenerateTestID()}
summaryIDs = []ids.ID{ids.GenerateTestID(), ids.GenerateTestID()}
heights = []uint64{1000, 2000}
engineType = p2p.EngineType_ENGINE_TYPE_SNOWMAN
chainID = ids.GenerateTestID()
requestID uint32 = 12345
deadline = time.Hour
nodeID = ids.GenerateTestNodeID()
summary = []byte{9, 8, 7}
appBytes = []byte{1, 3, 3, 7}
container = []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}
containerIDs = []ids.ID{ids.GenerateTestID(), ids.GenerateTestID()}
requestedHeight uint64 = 999
acceptedContainerID = ids.GenerateTestID()
summaryIDs = []ids.ID{ids.GenerateTestID(), ids.GenerateTestID()}
heights = []uint64{1000, 2000}
engineType = p2p.EngineType_ENGINE_TYPE_SNOWMAN
)

t.Run(
Expand Down Expand Up @@ -267,6 +268,7 @@ func TestInboundMsgBuilder(t *testing.T) {
requestID,
deadline,
container,
requestedHeight,
nodeID,
engineType,
)
Expand All @@ -281,6 +283,7 @@ func TestInboundMsgBuilder(t *testing.T) {
require.Equal(chainID[:], innerMsg.ChainId)
require.Equal(requestID, innerMsg.RequestId)
require.Equal(container, innerMsg.Container)
require.Equal(requestedHeight, innerMsg.RequestedHeight)
require.Equal(engineType, innerMsg.EngineType)
},
)
Expand All @@ -296,6 +299,7 @@ func TestInboundMsgBuilder(t *testing.T) {
requestID,
deadline,
containerIDs[0],
requestedHeight,
nodeID,
engineType,
)
Expand All @@ -310,6 +314,7 @@ func TestInboundMsgBuilder(t *testing.T) {
require.Equal(chainID[:], innerMsg.ChainId)
require.Equal(requestID, innerMsg.RequestId)
require.Equal(containerIDs[0][:], innerMsg.ContainerId)
require.Equal(requestedHeight, innerMsg.RequestedHeight)
require.Equal(engineType, innerMsg.EngineType)
},
)
Expand All @@ -323,7 +328,8 @@ func TestInboundMsgBuilder(t *testing.T) {
chainID,
requestID,
containerIDs[0],
acceptedContainerIDs[0],
containerIDs[1],
acceptedContainerID,
nodeID,
)

Expand All @@ -335,7 +341,8 @@ func TestInboundMsgBuilder(t *testing.T) {
require.Equal(chainID[:], innerMsg.ChainId)
require.Equal(requestID, innerMsg.RequestId)
require.Equal(containerIDs[0][:], innerMsg.PreferredId)
require.Equal(acceptedContainerIDs[0][:], innerMsg.AcceptedId)
require.Equal(containerIDs[1][:], innerMsg.PreferredIdAtHeight)
require.Equal(acceptedContainerID[:], innerMsg.AcceptedId)
},
)

Expand Down
24 changes: 12 additions & 12 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.

37 changes: 23 additions & 14 deletions message/outbound_msg_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ type OutboundMsgBuilder interface {
requestID uint32,
deadline time.Duration,
container []byte,
requestedHeight uint64,
engineType p2p.EngineType,
) (OutboundMessage, error)

Expand All @@ -141,13 +142,15 @@ type OutboundMsgBuilder interface {
requestID uint32,
deadline time.Duration,
containerID ids.ID,
requestedHeight uint64,
engineType p2p.EngineType,
) (OutboundMessage, error)

Chits(
chainID ids.ID,
requestID uint32,
preferredID ids.ID,
preferredIDAtHeight ids.ID,
acceptedID ids.ID,
) (OutboundMessage, error)

Expand Down Expand Up @@ -560,17 +563,19 @@ func (b *outMsgBuilder) PushQuery(
requestID uint32,
deadline time.Duration,
container []byte,
requestedHeight uint64,
engineType p2p.EngineType,
) (OutboundMessage, error) {
return b.builder.createOutbound(
&p2p.Message{
Message: &p2p.Message_PushQuery{
PushQuery: &p2p.PushQuery{
ChainId: chainID[:],
RequestId: requestID,
Deadline: uint64(deadline),
Container: container,
EngineType: engineType,
ChainId: chainID[:],
RequestId: requestID,
Deadline: uint64(deadline),
Container: container,
RequestedHeight: requestedHeight,
EngineType: engineType,
},
},
},
Expand All @@ -584,17 +589,19 @@ func (b *outMsgBuilder) PullQuery(
requestID uint32,
deadline time.Duration,
containerID ids.ID,
requestedHeight uint64,
engineType p2p.EngineType,
) (OutboundMessage, error) {
return b.builder.createOutbound(
&p2p.Message{
Message: &p2p.Message_PullQuery{
PullQuery: &p2p.PullQuery{
ChainId: chainID[:],
RequestId: requestID,
Deadline: uint64(deadline),
ContainerId: containerID[:],
EngineType: engineType,
ChainId: chainID[:],
RequestId: requestID,
Deadline: uint64(deadline),
ContainerId: containerID[:],
RequestedHeight: requestedHeight,
EngineType: engineType,
},
},
},
Expand All @@ -607,16 +614,18 @@ func (b *outMsgBuilder) Chits(
chainID ids.ID,
requestID uint32,
preferredID ids.ID,
preferredIDAtHeight ids.ID,
acceptedID ids.ID,
) (OutboundMessage, error) {
return b.builder.createOutbound(
&p2p.Message{
Message: &p2p.Message_Chits{
Chits: &p2p.Chits{
ChainId: chainID[:],
RequestId: requestID,
PreferredId: preferredID[:],
AcceptedId: acceptedID[:],
ChainId: chainID[:],
RequestId: requestID,
PreferredId: preferredID[:],
PreferredIdAtHeight: preferredIDAtHeight[:],
AcceptedId: acceptedID[:],
},
},
},
Expand Down
6 changes: 4 additions & 2 deletions proto/p2p/p2p.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ message PushQuery {
uint64 deadline = 3;
bytes container = 4;
EngineType engine_type = 5;
uint64 requested_height = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we populate requestedHeight last in the messages above then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The order doesn't actually matter... I felt like the code flowed more naturally with the order I used... The only reason the proto uses this order is because it is required for compatibility

}

// Message that contains a preferred container ID to query other peers
Expand All @@ -316,6 +317,7 @@ message PullQuery {
uint64 deadline = 3;
bytes container_id = 4;
EngineType engine_type = 5;
uint64 requested_height = 6;
}

// Message that contains the votes/preferences of the node. It is sent in
Expand All @@ -326,14 +328,14 @@ message PullQuery {
// will respond with a "get" message to fetch the missing block from the remote
// peer.
message Chits {
reserved 5; // Until Cortina upgrade is activated

bytes chain_id = 1;
uint32 request_id = 2;
// Represents the current preferred block.
bytes preferred_id = 3;
// Represents the last accepted block.
bytes accepted_id = 4;
// Represents the current preferred block at the requested height.
bytes preferred_id_at_height = 5;
}

message AppRequest {
Expand Down