Skip to content

Commit

Permalink
Populate accepted frontier when sending chits (ava-labs#2121)
Browse files Browse the repository at this point in the history
Co-authored-by: Stephen <[email protected]>
  • Loading branch information
gyuho and StephenButtolph authored Dec 30, 2022
1 parent 5c16945 commit af06d11
Show file tree
Hide file tree
Showing 16 changed files with 83 additions and 50 deletions.
4 changes: 4 additions & 0 deletions message/inbound_msg_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,18 +248,22 @@ func InboundChits(
chainID ids.ID,
requestID uint32,
preferredContainerIDs []ids.ID,
acceptedContainerIDs []ids.ID,
nodeID ids.NodeID,
engineType p2p.EngineType,
) InboundMessage {
preferredContainerIDBytes := make([][]byte, len(preferredContainerIDs))
encodeIDs(preferredContainerIDs, preferredContainerIDBytes)
acceptedContainerIDBytes := make([][]byte, len(acceptedContainerIDs))
encodeIDs(acceptedContainerIDs, acceptedContainerIDBytes)
return &inboundMessage{
nodeID: nodeID,
op: ChitsOp,
message: &p2p.Chits{
ChainId: chainID[:],
RequestId: requestID,
PreferredContainerIds: preferredContainerIDBytes,
AcceptedContainerIds: acceptedContainerIDBytes,
EngineType: engineType,
},
expiration: mockable.MaxTime,
Expand Down
30 changes: 19 additions & 11 deletions message/inbound_msg_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,18 @@ 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()}
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()}
acceptedContainerIDs = []ids.ID{ids.GenerateTestID(), 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 @@ -329,6 +330,7 @@ func TestInboundMsgBuilder(t *testing.T) {
chainID,
requestID,
containerIDs,
acceptedContainerIDs,
nodeID,
engineType,
)
Expand All @@ -346,6 +348,12 @@ func TestInboundMsgBuilder(t *testing.T) {
containerIDsBytes[i] = id[:]
}
require.Equal(containerIDsBytes, innerMsg.PreferredContainerIds)
acceptedContainerIDsBytes := make([][]byte, len(acceptedContainerIDs))
for i, id := range acceptedContainerIDs {
id := id
acceptedContainerIDsBytes[i] = id[:]
}
require.Equal(acceptedContainerIDsBytes, innerMsg.AcceptedContainerIds)
require.Equal(engineType, innerMsg.EngineType)
},
)
Expand Down
8 changes: 4 additions & 4 deletions message/mock_outbound_message_builder.go

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

7 changes: 6 additions & 1 deletion message/outbound_msg_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ type OutboundMsgBuilder interface {
Chits(
chainID ids.ID,
requestID uint32,
containerIDs []ids.ID,
preferredContainerIDs []ids.ID,
acceptedContainerIDs []ids.ID,
engineType p2p.EngineType,
) (OutboundMessage, error)

Expand Down Expand Up @@ -608,17 +609,21 @@ func (b *outMsgBuilder) Chits(
chainID ids.ID,
requestID uint32,
preferredContainerIDs []ids.ID,
acceptedContainerIDs []ids.ID,
engineType p2p.EngineType,
) (OutboundMessage, error) {
preferredContainerIDBytes := make([][]byte, len(preferredContainerIDs))
encodeIDs(preferredContainerIDs, preferredContainerIDBytes)
acceptedContainerIDBytes := make([][]byte, len(acceptedContainerIDs))
encodeIDs(acceptedContainerIDs, acceptedContainerIDBytes)
return b.builder.createOutbound(
&p2p.Message{
Message: &p2p.Message_Chits{
Chits: &p2p.Chits{
ChainId: chainID[:],
RequestId: requestID,
PreferredContainerIds: preferredContainerIDBytes,
AcceptedContainerIds: acceptedContainerIDBytes,
EngineType: engineType,
},
},
Expand Down
3 changes: 3 additions & 0 deletions snow/consensus/snowman/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ type Consensus interface {
// chain.
IsPreferred(Block) bool

// Returns the ID of the last accepted decision.
LastAccepted() ids.ID

// Returns the ID of the tail of the strongly preferred sequence of
// decisions.
Preference() ids.ID
Expand Down
4 changes: 4 additions & 0 deletions snow/consensus/snowman/topological.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ func (ts *Topological) IsPreferred(blk Block) bool {
return ts.preferredIDs.Contains(blk.ID())
}

func (ts *Topological) LastAccepted() ids.ID {
return ts.head
}

func (ts *Topological) Preference() ids.ID {
return ts.tail
}
Expand Down
4 changes: 2 additions & 2 deletions snow/engine/avalanche/transitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (t *Transitive) GetFailed(ctx context.Context, nodeID ids.NodeID, requestID

func (t *Transitive) PullQuery(ctx context.Context, nodeID ids.NodeID, requestID uint32, vtxID ids.ID) error {
// Immediately respond to the query with the current consensus preferences.
t.Sender.SendChits(ctx, nodeID, requestID, t.Consensus.Preferences().List())
t.Sender.SendChits(ctx, nodeID, requestID, t.Consensus.Preferences().List(), t.Manager.Edge(ctx))

// If we have [vtxID], attempt to put it into consensus, if we haven't
// already. If we don't not have [vtxID], fetch it from [nodeID].
Expand All @@ -184,7 +184,7 @@ func (t *Transitive) PullQuery(ctx context.Context, nodeID ids.NodeID, requestID

func (t *Transitive) PushQuery(ctx context.Context, nodeID ids.NodeID, requestID uint32, vtxBytes []byte) error {
// Immediately respond to the query with the current consensus preferences.
t.Sender.SendChits(ctx, nodeID, requestID, t.Consensus.Preferences().List())
t.Sender.SendChits(ctx, nodeID, requestID, t.Consensus.Preferences().List(), t.Manager.Edge(ctx))

vtx, err := t.Manager.ParseVtx(ctx, vtxBytes)
if err != nil {
Expand Down
8 changes: 5 additions & 3 deletions snow/engine/avalanche/transitive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func TestEngineQuery(t *testing.T) {
}

chitted := new(bool)
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, _ uint32, prefs []ids.ID) {
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, _ uint32, prefs []ids.ID, _ []ids.ID) {
if *chitted {
t.Fatalf("Sent multiple chits")
}
Expand Down Expand Up @@ -2582,6 +2582,7 @@ func TestEngineReissueAbortedVertex(t *testing.T) {

manager := vertex.NewTestManager(t)
manager.Default(true)
manager.TestStorage.CantEdge = false
engCfg.Manager = manager

gVtx := &avalanche.TestVertex{TestDecidable: choices.TestDecidable{
Expand Down Expand Up @@ -2721,6 +2722,7 @@ func TestEngineBootstrappingIntoConsensus(t *testing.T) {

manager := vertex.NewTestManager(t)
manager.Default(true)
manager.TestStorage.CantEdge = false
bootCfg.Manager = manager
engCfg.Manager = manager

Expand Down Expand Up @@ -2926,7 +2928,7 @@ func TestEngineBootstrappingIntoConsensus(t *testing.T) {
t.Fatalf("Unknown bytes provided")
panic("Unknown bytes provided")
}
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, _ uint32, chits []ids.ID) {
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, _ uint32, chits []ids.ID, _ []ids.ID) {
if inVdr != vdr {
t.Fatalf("Sent to the wrong validator")
}
Expand Down Expand Up @@ -3376,7 +3378,7 @@ func TestEngineReBootstrappingIntoConsensus(t *testing.T) {
t.Fatalf("Unknown bytes provided")
panic("Unknown bytes provided")
}
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, _ uint32, chits []ids.ID) {
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, _ uint32, chits []ids.ID, _ []ids.ID) {
if inVdr != vdr {
t.Fatalf("Sent to the wrong validator")
}
Expand Down
8 changes: 4 additions & 4 deletions snow/engine/common/mock_sender.go

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

2 changes: 1 addition & 1 deletion snow/engine/common/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ type QuerySender interface {
SendPullQuery(ctx context.Context, nodeIDs set.Set[ids.NodeID], requestID uint32, containerID ids.ID)

// Send chits to the specified node
SendChits(ctx context.Context, nodeID ids.NodeID, requestID uint32, votes []ids.ID)
SendChits(ctx context.Context, nodeID ids.NodeID, requestID uint32, votes []ids.ID, accepted []ids.ID)
}

// Gossiper defines how a consensus engine gossips a container on the accepted
Expand Down
6 changes: 3 additions & 3 deletions snow/engine/common/test_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type SenderTest struct {
SendAncestorsF func(context.Context, ids.NodeID, uint32, [][]byte)
SendPushQueryF func(context.Context, set.Set[ids.NodeID], uint32, []byte)
SendPullQueryF func(context.Context, set.Set[ids.NodeID], uint32, ids.ID)
SendChitsF func(context.Context, ids.NodeID, uint32, []ids.ID)
SendChitsF func(context.Context, ids.NodeID, uint32, []ids.ID, []ids.ID)
SendGossipF func(context.Context, []byte)
SendAppRequestF func(context.Context, set.Set[ids.NodeID], uint32, []byte) error
SendAppResponseF func(context.Context, ids.NodeID, uint32, []byte) error
Expand Down Expand Up @@ -263,9 +263,9 @@ func (s *SenderTest) SendPullQuery(ctx context.Context, vdrs set.Set[ids.NodeID]
// SendChits calls SendChitsF if it was initialized. If it wasn't initialized
// and this function shouldn't be called and testing was initialized, then
// testing will fail.
func (s *SenderTest) SendChits(ctx context.Context, vdr ids.NodeID, requestID uint32, votes []ids.ID) {
func (s *SenderTest) SendChits(ctx context.Context, vdr ids.NodeID, requestID uint32, votes []ids.ID, accepted []ids.ID) {
if s.SendChitsF != nil {
s.SendChitsF(ctx, vdr, requestID, votes)
s.SendChitsF(ctx, vdr, requestID, votes, accepted)
} else if s.CantSendChits && s.T != nil {
s.T.Fatalf("Unexpectedly called SendChits")
}
Expand Down
20 changes: 6 additions & 14 deletions snow/engine/snowman/transitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,7 @@ func (t *Transitive) GetFailed(ctx context.Context, nodeID ids.NodeID, requestID
}

func (t *Transitive) PullQuery(ctx context.Context, nodeID ids.NodeID, requestID uint32, blkID ids.ID) error {
if err := t.sendChits(ctx, nodeID, requestID); err != nil {
return err
}
t.sendChits(ctx, nodeID, requestID)

// Try to issue [blkID] to consensus.
// If we're missing an ancestor, request it from [vdr]
Expand All @@ -195,9 +193,7 @@ func (t *Transitive) PullQuery(ctx context.Context, nodeID ids.NodeID, requestID
}

func (t *Transitive) PushQuery(ctx context.Context, nodeID ids.NodeID, requestID uint32, blkBytes []byte) error {
if err := t.sendChits(ctx, nodeID, requestID); err != nil {
return err
}
t.sendChits(ctx, nodeID, requestID)

blk, err := t.VM.ParseBlock(ctx, blkBytes)
// If parsing fails, we just drop the request, as we didn't ask for it
Expand Down Expand Up @@ -471,17 +467,13 @@ func (t *Transitive) GetBlock(ctx context.Context, blkID ids.ID) (snowman.Block,
return t.VM.GetBlock(ctx, blkID)
}

func (t *Transitive) sendChits(ctx context.Context, nodeID ids.NodeID, requestID uint32) error {
func (t *Transitive) sendChits(ctx context.Context, nodeID ids.NodeID, requestID uint32) {
lastAccepted := t.Consensus.LastAccepted()
if t.Ctx.IsRunningStateSync() {
lastAcceptedID, err := t.VM.LastAccepted(ctx)
if err != nil {
return err
}
t.Sender.SendChits(ctx, nodeID, requestID, []ids.ID{lastAcceptedID})
t.Sender.SendChits(ctx, nodeID, requestID, []ids.ID{lastAccepted}, []ids.ID{lastAccepted})
} else {
t.Sender.SendChits(ctx, nodeID, requestID, []ids.ID{t.Consensus.Preference()})
t.Sender.SendChits(ctx, nodeID, requestID, []ids.ID{t.Consensus.Preference()}, []ids.ID{lastAccepted})
}
return nil
}

// Build blocks if they have been requested and the number of processing blocks
Expand Down
16 changes: 14 additions & 2 deletions snow/engine/snowman/transitive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func TestEngineQuery(t *testing.T) {
}

chitted := new(bool)
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, requestID uint32, prefSet []ids.ID) {
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, requestID uint32, prefSet []ids.ID, accepted []ids.ID) {
if *chitted {
t.Fatalf("Sent multiple chits")
}
Expand All @@ -228,6 +228,12 @@ func TestEngineQuery(t *testing.T) {
if gBlk.ID() != prefSet[0] {
t.Fatalf("Wrong chits block")
}
if len(accepted) != 1 {
t.Fatal("accepted should only have one element")
}
if gBlk.ID() != accepted[0] {
t.Fatalf("Wrong accepted frontier")
}
}

blocked := new(bool)
Expand Down Expand Up @@ -764,7 +770,7 @@ func TestEnginePushQuery(t *testing.T) {
}

chitted := new(bool)
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, requestID uint32, votes []ids.ID) {
sender.SendChitsF = func(_ context.Context, inVdr ids.NodeID, requestID uint32, votes []ids.ID, accepted []ids.ID) {
if *chitted {
t.Fatalf("Sent chit multiple times")
}
Expand All @@ -781,6 +787,12 @@ func TestEnginePushQuery(t *testing.T) {
if gBlk.ID() != votes[0] {
t.Fatalf("Asking for wrong block")
}
if len(accepted) != 1 {
t.Fatal("accepted should only have one element")
}
if gBlk.ID() != accepted[0] {
t.Fatalf("Wrong accepted frontier")
}
}

queried := new(bool)
Expand Down
3 changes: 2 additions & 1 deletion snow/networking/router/chain_router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,7 @@ func TestRouterClearTimeouts(t *testing.T) {
ctx.ChainID,
requestID,
nil,
nil,
nodeID,
engineType,
)
Expand Down Expand Up @@ -1037,7 +1038,7 @@ func TestValidatorOnlyMessageDrops(t *testing.T) {
err = vdrs.RemoveWeight(vID, 1)
require.NoError(t, err)

inMsg = message.InboundChits(ctx.ChainID, reqID, nil, nID, engineType)
inMsg = message.InboundChits(ctx.ChainID, reqID, nil, nil, nID, engineType)
chainRouter.HandleInbound(context.Background(), inMsg)

// shouldn't clear out timed request, as the request should be cleared when
Expand Down
5 changes: 3 additions & 2 deletions snow/networking/sender/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,7 @@ func (s *sender) SendPullQuery(ctx context.Context, nodeIDs set.Set[ids.NodeID],
}

// SendChits sends chits
func (s *sender) SendChits(ctx context.Context, nodeID ids.NodeID, requestID uint32, votes []ids.ID) {
func (s *sender) SendChits(ctx context.Context, nodeID ids.NodeID, requestID uint32, votes, accepted []ids.ID) {
ctx = utils.Detach(ctx)

// If [nodeID] is myself, send this message directly
Expand All @@ -1149,6 +1149,7 @@ func (s *sender) SendChits(ctx context.Context, nodeID ids.NodeID, requestID uin
s.ctx.ChainID,
requestID,
votes,
accepted,
nodeID,
s.engineType,
)
Expand All @@ -1157,7 +1158,7 @@ func (s *sender) SendChits(ctx context.Context, nodeID ids.NodeID, requestID uin
}

// Create the outbound message.
outMsg, err := s.msgCreator.Chits(s.ctx.ChainID, requestID, votes, s.engineType)
outMsg, err := s.msgCreator.Chits(s.ctx.ChainID, requestID, votes, accepted, s.engineType)
if err != nil {
s.ctx.Log.Error("failed to build message",
zap.Stringer("messageOp", message.ChitsOp),
Expand Down
Loading

0 comments on commit af06d11

Please sign in to comment.