diff --git a/dot/parachain/collator-protocol/message_test.go b/dot/parachain/collator-protocol/message_test.go index 67565a0c73..3d87930954 100644 --- a/dot/parachain/collator-protocol/message_test.go +++ b/dot/parachain/collator-protocol/message_test.go @@ -240,7 +240,7 @@ func TestHandleCollationMessageDeclare(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ @@ -268,7 +268,7 @@ func TestHandleCollationMessageDeclare(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Connected, }, @@ -285,7 +285,7 @@ func TestHandleCollationMessageDeclare(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Connected, }, @@ -312,7 +312,7 @@ func TestHandleCollationMessageDeclare(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Connected, }, @@ -342,7 +342,7 @@ func TestHandleCollationMessageDeclare(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Connected, }, @@ -449,7 +449,7 @@ func TestHandleCollationMessageAdvertiseCollation(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Connected, }, @@ -475,7 +475,7 @@ func TestHandleCollationMessageAdvertiseCollation(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ @@ -510,7 +510,7 @@ func TestHandleCollationMessageAdvertiseCollation(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ @@ -536,7 +536,7 @@ func TestHandleCollationMessageAdvertiseCollation(t *testing.T) { }, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ @@ -616,7 +616,7 @@ func TestInsertAdvertisement(t *testing.T) { { description: "fail with undeclared collator", peerData: PeerData{ - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Connected, }, @@ -626,7 +626,7 @@ func TestInsertAdvertisement(t *testing.T) { { description: "fail with error out of view", peerData: PeerData{ - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, }, @@ -639,7 +639,7 @@ func TestInsertAdvertisement(t *testing.T) { { description: "fail with error duplicate advertisement", peerData: PeerData{ - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ @@ -662,7 +662,7 @@ func TestInsertAdvertisement(t *testing.T) { { description: "success case", peerData: PeerData{ - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ diff --git a/dot/parachain/collator-protocol/validator_side.go b/dot/parachain/collator-protocol/validator_side.go index 8b23bacdb7..ddd5b576e6 100644 --- a/dot/parachain/collator-protocol/validator_side.go +++ b/dot/parachain/collator-protocol/validator_side.go @@ -13,7 +13,6 @@ import ( "github.com/ChainSafe/gossamer/dot/network" collatorprotocolmessages "github.com/ChainSafe/gossamer/dot/parachain/collator-protocol/messages" - "github.com/ChainSafe/gossamer/dot/parachain/network-bridge/events" networkbridgeevents "github.com/ChainSafe/gossamer/dot/parachain/network-bridge/events" networkbridgemessages "github.com/ChainSafe/gossamer/dot/parachain/network-bridge/messages" parachaintypes "github.com/ChainSafe/gossamer/dot/parachain/types" @@ -123,7 +122,7 @@ func (cpvs *CollatorProtocolValidatorSide) ProcessActiveLeavesUpdateSignal( return nil } -func (cpvs *CollatorProtocolValidatorSide) handleOurViewChange(view events.View) error { +func (cpvs *CollatorProtocolValidatorSide) handleOurViewChange(view parachaintypes.View) error { // 1. Find out removed leaves (hashes) and newly added leaves // 2. Go over each new leaves, // - check if perspective parachain mode is enabled @@ -400,7 +399,7 @@ type PendingCollation struct { } type PeerData struct { - view View + view parachaintypes.View state PeerStateInfo } @@ -495,6 +494,31 @@ func (peerData *PeerData) InsertAdvertisement( return true, nil } +// UpdateView updates the view clearing all advertisements that are no longer in the current view. +func (peerData *PeerData) UpdateView(implicitView ImplicitView, + activeLeaves map[common.Hash]parachaintypes.ProspectiveParachainsMode, perRelayParent map[common.Hash]PerRelayParent, + newView parachaintypes.View) { + + oldView := peerData.view + if peerData.state.PeerState == Collating { + // remove relay parent advertisement if it went out of implicit view + diff := oldView.Difference(newView) + for _, relayParent := range diff { + _, ok := perRelayParent[relayParent] + if !ok { + delete(peerData.state.CollatingPeerState.advertisements, relayParent) + } + + keep := IsRelayParentInImplicitView(relayParent, perRelayParent[relayParent].prospectiveParachainMode, + implicitView, activeLeaves, peerData.state.CollatingPeerState.ParaID) + + if !keep { + delete(peerData.state.CollatingPeerState.advertisements, relayParent) + } + } + } +} + type PeerStateInfo struct { PeerState PeerState // instant at which peer got connected @@ -521,47 +545,6 @@ const ( // We use the same limit to compute the view sent to peers locally. const MaxViewHeads uint8 = 5 -// A succinct representation of a peer's view. This consists of a bounded amount of chain heads -// and the highest known finalized block number. -// -// Up to `N` (5?) chain heads. -type View struct { - // a bounded amount of chain heads - heads []common.Hash - // the highest known finalized number - finalizedNumber uint32 -} - -type SortableHeads []common.Hash - -func (s SortableHeads) Len() int { - return len(s) -} - -func (s SortableHeads) Less(i, j int) bool { - return s[i].String() > s[j].String() -} - -func (s SortableHeads) Swap(i, j int) { - s[i], s[j] = s[j], s[i] -} - -func ConstructView(liveHeads map[common.Hash]struct{}, finalizedNumber uint32) View { - heads := make([]common.Hash, 0, len(liveHeads)) - for head := range liveHeads { - heads = append(heads, head) - } - - if len(heads) >= 5 { - heads = heads[:5] - } - - return View{ - heads: heads, - finalizedNumber: finalizedNumber, - } -} - // Network is the interface required by parachain service for the network type Network interface { GossipMessage(msg network.NotificationsMessage) @@ -743,7 +726,13 @@ func (cpvs CollatorProtocolValidatorSide) handleNetworkBridgeEvents(msg any) err case networkbridgeevents.NewGossipTopology: // NOTE: This won't happen case networkbridgeevents.PeerViewChange: - // TODO #4155 + // - advertisements by this peer that are no longer relevant have to be removed + peerData, ok := cpvs.peerData[msg.PeerID] + if ok { + peerData.UpdateView(cpvs.implicitView, cpvs.activeLeaves, cpvs.perRelayParent, msg.View) + cpvs.peerData[msg.PeerID] = peerData + } + case networkbridgeevents.OurViewChange: return cpvs.handleOurViewChange(msg.View) case networkbridgeevents.UpdatedAuthorityIDs: diff --git a/dot/parachain/collator-protocol/validator_side_test.go b/dot/parachain/collator-protocol/validator_side_test.go index 18631a5d98..aa8e37c793 100644 --- a/dot/parachain/collator-protocol/validator_side_test.go +++ b/dot/parachain/collator-protocol/validator_side_test.go @@ -10,6 +10,7 @@ import ( "github.com/ChainSafe/gossamer/dot/network" "github.com/ChainSafe/gossamer/dot/parachain/backing" collatorprotocolmessages "github.com/ChainSafe/gossamer/dot/parachain/collator-protocol/messages" + networkbridgeevents "github.com/ChainSafe/gossamer/dot/parachain/network-bridge/events" networkbridgemessages "github.com/ChainSafe/gossamer/dot/parachain/network-bridge/messages" "github.com/ChainSafe/gossamer/dot/parachain/overseer" parachaintypes "github.com/ChainSafe/gossamer/dot/parachain/types" @@ -104,7 +105,7 @@ func TestProcessOverseerMessage(t *testing.T) { }}, peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ @@ -168,7 +169,7 @@ func TestProcessOverseerMessage(t *testing.T) { }(), peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ @@ -250,7 +251,7 @@ func TestProcessOverseerMessage(t *testing.T) { }(), peerData: map[peer.ID]PeerData{ peerID: { - view: View{}, + view: parachaintypes.View{}, state: PeerStateInfo{ PeerState: Collating, CollatingPeerState: CollatingPeerState{ @@ -460,3 +461,52 @@ func TestProcessBackedOverseerMessage(t *testing.T) { }) } } + +func TestPeerViewChange(t *testing.T) { + t.Parallel() + + // test that relay parent advertisement gets removed if it went out of implicit view + + cpvs := CollatorProtocolValidatorSide{ + activeLeaves: map[common.Hash]parachaintypes.ProspectiveParachainsMode{}, + perRelayParent: map[common.Hash]PerRelayParent{ + {0x01}: { + prospectiveParachainMode: parachaintypes.ProspectiveParachainsMode{ + IsEnabled: false, + }, + }, + }, + peerData: map[peer.ID]PeerData{ + peer.ID("peer1"): { + // this shows our current view of peer1 + view: parachaintypes.View{ + Heads: []common.Hash{{0x01}}, + }, + state: PeerStateInfo{ + PeerState: Collating, + CollatingPeerState: CollatingPeerState{ + advertisements: map[common.Hash][]parachaintypes.CandidateHash{ + {0x01}: {}, + }, + }, + }, + }, + }, + } + + msg := networkbridgeevents.PeerViewChange{ + PeerID: peer.ID("peer1"), + // this shows the new view of peer1, since the new view does not contain relay parent {0x01}, + // we will remove the advertisement for that relay parent + View: parachaintypes.View{ + Heads: []common.Hash{{0x02}}, + }, + } + + err := cpvs.handleNetworkBridgeEvents(msg) + require.NoError(t, err) + + // advertisement for relay parent {0x01} should be removed + _, ok := cpvs.peerData[peer.ID("peer1")].state.CollatingPeerState.advertisements[common.Hash{0x01}] + require.False(t, ok) +} diff --git a/dot/parachain/network-bridge/events/events.go b/dot/parachain/network-bridge/events/events.go index 124bd2fbca..735aa2f06e 100644 --- a/dot/parachain/network-bridge/events/events.go +++ b/dot/parachain/network-bridge/events/events.go @@ -46,22 +46,11 @@ type NewGossipTopology struct { type PeerViewChange struct { PeerID peer.ID - View View -} - -// View is a succinct representation of a peer's view. This consists of a bounded amount of chain heads -// and the highest known finalized block number. -// -// Up to `N` (5?) chain heads. -type View struct { - // a bounded amount of chain heads - Heads []common.Hash - // the highest known finalized number - FinalizedNumber uint32 + View parachaintypes.View } type OurViewChange struct { - View View + View parachaintypes.View } type PeerMessage[Message collationprotocol.CollationProtocol | validationprotocol.ValidationProtocol] struct { diff --git a/dot/parachain/network-bridge/receiver.go b/dot/parachain/network-bridge/receiver.go index e056479e58..aa6baf983a 100644 --- a/dot/parachain/network-bridge/receiver.go +++ b/dot/parachain/network-bridge/receiver.go @@ -46,7 +46,7 @@ type NetworkBridgeReceiver struct { Keystore keystore.Keystore sync Sync - localView *View + localView *parachaintypes.View // heads are sorted in descending order by block number liveHeads []parachaintypes.ActivatedLeaf @@ -60,7 +60,7 @@ type NetworkBridgeReceiver struct { authorityDiscoveryService AuthorityDiscoveryService peerData map[peer.ID]struct { - view View + view parachaintypes.View protocolVersion uint32 } } @@ -204,7 +204,7 @@ func (nbr *NetworkBridgeReceiver) updateOurView() error { //nolint for _, head := range nbr.liveHeads { headHashes = append(headHashes, head.Hash) } - newView := View{ + newView := parachaintypes.View{ Heads: headHashes, FinalizedNumber: nbr.finalizedNumber, } @@ -218,7 +218,7 @@ func (nbr *NetworkBridgeReceiver) updateOurView() error { //nolint // we only want to send a view update if the heads have changed. // A change in finalized block is not enough to trigger a view update. - if nbr.localView.checkHeadsEqual(newView) { + if nbr.localView.CheckHeadsEqual(newView) { // nothing to update return nil } @@ -236,8 +236,6 @@ func (nbr *NetworkBridgeReceiver) updateOurView() error { //nolint func (nbr *NetworkBridgeReceiver) handleCollationMessage( sender peer.ID, msg network.NotificationsMessage) (bool, error) { - // TODO: Handle ViewUpdate message. ViewUpdate happens on both protocols. #4156 #4155 - // we don't propagate collation messages, so it will always be false propagate := false @@ -326,23 +324,23 @@ func (nbr *NetworkBridgeReceiver) handleViewUpdate(peer peer.ID, view ViewUpdate Value: peerset.CostMinor, Reason: "peer sent us empty view", }, peer) - } else if View(view).checkHeadsEqual(peerData.view) { + } else if parachaintypes.View(view).CheckHeadsEqual(peerData.view) { // nothing } else { - peerData.view = View(view) + peerData.view = parachaintypes.View(view) nbr.peerData[peer] = peerData nbr.SubsystemsToOverseer <- events.Event[collatorprotocolmessages.CollationProtocol]{ Inner: events.PeerViewChange{ PeerID: peer, - View: events.View(view), + View: parachaintypes.View(view), }, } nbr.SubsystemsToOverseer <- events.Event[validationprotocol.ValidationProtocol]{ Inner: events.PeerViewChange{ PeerID: peer, - View: events.View(view), + View: parachaintypes.View(view), }, } } diff --git a/dot/parachain/network-bridge/wire_message.go b/dot/parachain/network-bridge/wire_message.go index 78a2762ea8..a7cf66fd80 100644 --- a/dot/parachain/network-bridge/wire_message.go +++ b/dot/parachain/network-bridge/wire_message.go @@ -5,15 +5,13 @@ package networkbridge import ( "fmt" - "reflect" - "sort" "github.com/ChainSafe/gossamer/lib/common" "github.com/ChainSafe/gossamer/pkg/scale" "github.com/ChainSafe/gossamer/dot/network" collatorprotocolmessages "github.com/ChainSafe/gossamer/dot/parachain/collator-protocol/messages" - events "github.com/ChainSafe/gossamer/dot/parachain/network-bridge/events" + parachaintypes "github.com/ChainSafe/gossamer/dot/parachain/types" validationprotocol "github.com/ChainSafe/gossamer/dot/parachain/validation-protocol" ) @@ -97,37 +95,7 @@ func (w WireMessage) Encode() ([]byte, error) { return enc, nil } -type ViewUpdate events.View - -type View events.View - -type SortableHeads []common.Hash - -func (s SortableHeads) Len() int { - return len(s) -} - -func (s SortableHeads) Less(i, j int) bool { - return s[i].String() > s[j].String() -} - -func (s SortableHeads) Swap(i, j int) { - s[i], s[j] = s[j], s[i] -} - -// checkHeadsEqual checks if the heads of the view are equal to the heads of the other view. -func (v View) checkHeadsEqual(other View) bool { - if len(v.Heads) != len(other.Heads) { - return false - } - - localHeads := v.Heads - sort.Sort(SortableHeads(localHeads)) - otherHeads := other.Heads - sort.Sort(SortableHeads(otherHeads)) - - return reflect.DeepEqual(localHeads, otherHeads) -} +type ViewUpdate parachaintypes.View type ProtocolMessage struct { inner any diff --git a/dot/parachain/types/view.go b/dot/parachain/types/view.go new file mode 100644 index 0000000000..7e08224af0 --- /dev/null +++ b/dot/parachain/types/view.go @@ -0,0 +1,94 @@ +package parachaintypes + +import ( + "reflect" + "sort" + + "github.com/ChainSafe/gossamer/lib/common" +) + +// A succinct representation of a peer's view. This consists of a bounded amount of chain heads +// and the highest known finalized block number. +// +// Up to `N` (5?) chain heads. +type View struct { + // a bounded amount of chain Heads + Heads []common.Hash + // the highest known finalized number + FinalizedNumber uint32 +} + +// Difference returns hashes present in v View but not in v2 View. +func (v View) Difference(v2 View) []common.Hash { + + vHeads := SortableHeads(v.Heads) + v2Heads := SortableHeads(v2.Heads) + + sort.Sort(vHeads) + sort.Sort(v2Heads) + + var diff []common.Hash + i, j := 0, 0 + for i < len(vHeads) && j < len(v2Heads) { + if vHeads[i] == v2Heads[j] { + i++ + j++ + } else if vHeads[i].String() < v2Heads[j].String() { + diff = append(diff, vHeads[i]) + i++ + } else { + j++ + } + } + + for i < len(vHeads) { + diff = append(diff, vHeads[i]) + i++ + } + + return diff +} + +// CheckHeadsEqual checks if the heads of the view are equal to the heads of the other view. +func (v View) CheckHeadsEqual(other View) bool { + if len(v.Heads) != len(other.Heads) { + return false + } + + localHeads := v.Heads + sort.Sort(SortableHeads(localHeads)) + otherHeads := other.Heads + sort.Sort(SortableHeads(otherHeads)) + + return reflect.DeepEqual(localHeads, otherHeads) +} + +type SortableHeads []common.Hash + +func (s SortableHeads) Len() int { + return len(s) +} + +func (s SortableHeads) Less(i, j int) bool { + return s[i].String() > s[j].String() +} + +func (s SortableHeads) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + +func ConstructView(liveHeads map[common.Hash]struct{}, finalizedNumber uint32) View { + heads := make([]common.Hash, 0, len(liveHeads)) + for head := range liveHeads { + heads = append(heads, head) + } + + if len(heads) >= 5 { + heads = heads[:5] + } + + return View{ + Heads: heads, + FinalizedNumber: finalizedNumber, + } +}