Skip to content

Commit

Permalink
p2p: fix wantTXGossip modifications (#5982)
Browse files Browse the repository at this point in the history
  • Loading branch information
algorandskiy authored Apr 19, 2024
1 parent 46b5e99 commit d38f35c
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 7 deletions.
11 changes: 10 additions & 1 deletion network/p2pNetwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,15 @@ type gossipSubPeer struct {

func (p gossipSubPeer) GetNetwork() GossipNode { return p.net }

Check warning on line 198 in network/p2pNetwork.go

View check run for this annotation

Codecov / codecov/patch

network/p2pNetwork.go#L198

Added line #L198 was not covered by tests

func (p gossipSubPeer) OnClose(f func()) {
net := p.GetNetwork().(*P2PNetwork)
net.wsPeersLock.Lock()
defer net.wsPeersLock.Unlock()
if wsp, ok := net.wsPeers[p.peerID]; ok {
wsp.OnClose(f)

Check warning on line 205 in network/p2pNetwork.go

View check run for this annotation

Codecov / codecov/patch

network/p2pNetwork.go#L200-L205

Added lines #L200 - L205 were not covered by tests
}
}

// NewP2PNetwork returns an instance of GossipNode that uses the p2p.Service
func NewP2PNetwork(log logging.Logger, cfg config.Local, datadir string, phonebookAddresses []string, genesisID string, networkID protocol.NetworkID, node NodeInfo) (*P2PNetwork, error) {
const readBufferLen = 2048
Expand Down Expand Up @@ -668,7 +677,7 @@ func (n *P2PNetwork) GetHTTPClient(address string) (*http.Client, error) {
func (n *P2PNetwork) OnNetworkAdvance() {
if n.nodeInfo != nil {
old := n.wantTXGossip.Load()
new := n.nodeInfo.IsParticipating()
new := n.relayMessages || n.config.ForceFetchTransactions || n.nodeInfo.IsParticipating()
if old != new {
n.wantTXGossip.Store(new)
if new {
Expand Down
42 changes: 40 additions & 2 deletions network/p2pNetwork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -907,9 +907,9 @@ func TestP2PWantTXGossip(t *testing.T) {
ctx: ctx,
nodeInfo: &nopeNodeInfo{},
}
net.wantTXGossip.Store(false)

// ensure wantTXGossip from false to false is noop
net.wantTXGossip.Store(false)
net.OnNetworkAdvance()
require.Eventually(t, func() bool { net.wg.Wait(); return true }, 1*time.Second, 50*time.Millisecond)
require.Equal(t, int64(0), mockService.count.Load())
Expand All @@ -923,14 +923,52 @@ func TestP2PWantTXGossip(t *testing.T) {
require.False(t, net.wantTXGossip.Load())

// check false to true change triggers subscription
net.wantTXGossip.Store(false)
net.nodeInfo = &participatingNodeInfo{}
net.OnNetworkAdvance()
require.Eventually(t, func() bool { return mockService.count.Load() == 1 }, 1*time.Second, 50*time.Millisecond)
require.True(t, net.wantTXGossip.Load())

// check IsParticipating changes wantTXGossip
net.wantTXGossip.Store(true)
net.nodeInfo = &nopeNodeInfo{}
net.config.ForceFetchTransactions = false
net.relayMessages = false
net.OnNetworkAdvance()
require.Eventually(t, func() bool { net.wg.Wait(); return true }, 1*time.Second, 50*time.Millisecond)
require.False(t, net.wantTXGossip.Load())

// check ForceFetchTransactions and relayMessages also take effect
net.wantTXGossip.Store(false)
net.nodeInfo = &nopeNodeInfo{}
net.config.ForceFetchTransactions = true
net.relayMessages = false
net.OnNetworkAdvance()
require.Eventually(t, func() bool { return mockService.count.Load() == 2 }, 1*time.Second, 50*time.Millisecond)
require.True(t, net.wantTXGossip.Load())

net.wantTXGossip.Store(false)
net.nodeInfo = &nopeNodeInfo{}
net.config.ForceFetchTransactions = false
net.relayMessages = true
net.OnNetworkAdvance()
require.Eventually(t, func() bool { return mockService.count.Load() == 3 }, 1*time.Second, 50*time.Millisecond)
require.True(t, net.wantTXGossip.Load())

// ensure empty nodeInfo prevents changing the value
net.wantTXGossip.Store(false)
net.nodeInfo = nil
net.config.ForceFetchTransactions = true
net.relayMessages = true
net.OnNetworkAdvance()
require.Eventually(t, func() bool { net.wg.Wait(); return true }, 1*time.Second, 50*time.Millisecond)
require.False(t, net.wantTXGossip.Load())

// check true to true change is noop
net.wantTXGossip.Store(true)
net.nodeInfo = &participatingNodeInfo{}
net.OnNetworkAdvance()
require.Eventually(t, func() bool { return mockService.count.Load() == 1 }, 1*time.Second, 50*time.Millisecond)
require.Eventually(t, func() bool { return mockService.count.Load() == 3 }, 1*time.Second, 50*time.Millisecond)
require.True(t, net.wantTXGossip.Load())
}

Expand Down
5 changes: 1 addition & 4 deletions network/wsNetwork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import (
"time"

"github.com/algorand/go-algorand/internal/rapidgen"
"github.com/algorand/go-algorand/network/p2p"
"github.com/algorand/go-algorand/network/phonebook"
"pgregory.net/rapid"

Expand Down Expand Up @@ -3293,14 +3292,12 @@ func TestWebsocketNetworkTXMessageOfInterestNPN(t *testing.T) {
}

type participatingNodeInfo struct {
nopeNodeInfo
}

func (nnni *participatingNodeInfo) IsParticipating() bool {
return true
}
func (nnni *participatingNodeInfo) Capabilities() []p2p.Capability {
return nil
}

func TestWebsocketNetworkTXMessageOfInterestPN(t *testing.T) {
// Tests that A->B follows MOI
Expand Down

0 comments on commit d38f35c

Please sign in to comment.