[p2p] Improve p2p #3409
[p2p] Improve p2p #3409
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3409 +/- ##
==========================================
- Coverage 75.41% 75.32% -0.10%
==========================================
Files 245 245
Lines 22699 22715 +16
==========================================
- Hits 17119 17109 -10
- Misses 4655 4682 +27
+ Partials 925 924 -1
Continue to review full report at Codecov.
|
p2p/agent.go
Outdated
| } | ||
| p.host.FindPeers(context.Background()) |
There was a problem hiding this comment.
there is also a context.Background() at line 589, use one ctx
There was a problem hiding this comment.
this is not an issue. the func is directly returned in L590
There was a problem hiding this comment.
yes, the code can work fine. But programming-wise, feels better to define a common ctx = context.Background() ahead, and use it at 2 places, rather than declare a new context wherever needed.
p2p/agent_test.go
Outdated
| @@ -42,6 +44,7 @@ func TestDummyAgent(t *testing.T) { | |||
| } | |||
|
|
|||
| func TestBroadcast(t *testing.T) { | |||
| t.Skip() | |||
| @@ -494,93 +503,13 @@ func newTestConfig() (config.Config, error) { | |||
| return cfg, nil | |||
| } | |||
|
|
|||
| func TestBlockSyncerPeerBlockList(t *testing.T) { | |||
| r.NoError(bootnode.Start(ctx)) | ||
|
|
||
| addrs, err := bootnode.Self() | ||
| bootnode, err := p2p.NewHost(context.Background(), p2p.DHTProtocolID(2), p2p.Port(testutil.RandomPort()), p2p.SecureIO(), p2p.MasterKey("bootnode")) |
There was a problem hiding this comment.
why change to this? can we still use NewAgent()?
There was a problem hiding this comment.
bootnode shouldn't JoinOverlay()
p2p/agent_test.go
Outdated
| for i := 0; i < n; i++ { | ||
| port := bootnodePort + i + 1 | ||
| port := testutil.RandomPort() |
There was a problem hiding this comment.
+ i + 1 would make sure a diff port for each node, we don't need to change this, unless there is a strong reason?
Liuhaai
force-pushed
the
improveP2p
branch
2 times, most recently
from
9d686ab to
700d1a0
Compare
| @@ -71,7 +71,7 @@ func (b *blockBuffer) AddBlock(tipHeight uint64, blk *peerBlock) (bool, uint64) | |||
| defer b.mu.Unlock() | |||
| blkHeight := blk.block.Height() | |||
| if blkHeight <= tipHeight { | |||
| return false, 0 | |||
| return false, blkHeight | |||
There was a problem hiding this comment.
no impact to current logic
There was a problem hiding this comment.
yep. The change is helpful to log the change of target height by the block of higher height.
go.mod
Outdated
| @@ -15,7 +15,7 @@ require ( | |||
| github.com/grpc-ecosystem/go-grpc-middleware v1.2.0 | |||
| github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 | |||
| github.com/iotexproject/go-fsm v1.0.0 | |||
| github.com/iotexproject/go-p2p v0.3.4-0.20220624220702-2aa2dd39e7a6 | |||
There was a problem hiding this comment.
merge p2p PR31, make a release, use new version here
Description
Call adjustment due to the introduction of peerManager in go-pkg package
Neighborsinagentclass is replaced withConnectedPeerswhich is introduced inpeerManagerMaxPeersparameter is added into p2p configRequestBlockshandler inblocksyncis replaced with independent handlersNeighbors,UniCastOutbound, andBlockPeerfix #3234 #3412 #3448
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: