Skip to content

Commit b272ea6

Browse files
tac0turtletac0turtlecoderabbitai[bot]
authored
test: use mocknet in unit test (#2371)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. NOTE: PR titles should follow semantic commits: https://www.conventionalcommits.org/en/v1.0.0/ --> ## Overview This pr uses a mocknet in the syncservicer_test in order to avoid spinning up a network for a unit test. It also adds a check to prevent dialling ourself closes #2359 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for creating a client with a custom or mock host, enabling greater flexibility in peer-to-peer networking scenarios. - **Bug Fixes** - Improved resource cleanup to avoid errors when closing components that may not be initialized. - Prevented clients from attempting to connect to themselves, reducing unnecessary connection attempts. - **Tests** - Updated tests to utilize mock networks for more robust and isolated peer-to-peer testing. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: tac0turtle <you@example.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 7808d2d commit b272ea6

3 files changed

Lines changed: 119 additions & 10 deletions

File tree

pkg/p2p/client.go

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,33 @@ func NewClient(
9292
}, nil
9393
}
9494

95+
func NewClientWithHost(
96+
conf config.Config,
97+
nodeKey *key.NodeKey,
98+
ds datastore.Datastore,
99+
logger log.Logger,
100+
metrics *Metrics,
101+
h host.Host, // injected host (mocknet or custom)
102+
) (*Client, error) {
103+
c, err := NewClient(conf, nodeKey, ds, logger, metrics)
104+
if err != nil {
105+
return nil, err
106+
}
107+
108+
// Reject hosts whose identity does not match the supplied node key
109+
expectedID, _ := peer.IDFromPrivateKey(nodeKey.PrivKey)
110+
if h.ID() != expectedID {
111+
return nil, fmt.Errorf(
112+
"injected host ID %s does not match node key ID %s",
113+
h.ID(),
114+
expectedID,
115+
)
116+
}
117+
118+
c.host = h
119+
return c, nil
120+
}
121+
95122
// Start establish Client's P2P connectivity.
96123
//
97124
// Following steps are taken:
@@ -101,11 +128,16 @@ func NewClient(
101128
// 4. Use active peer discovery to look for peers from same ORU network.
102129
func (c *Client) Start(ctx context.Context) error {
103130
c.logger.Debug("starting P2P client")
104-
host, err := c.listen()
131+
132+
if c.host != nil {
133+
return c.startWithHost(ctx, c.host)
134+
}
135+
136+
h, err := c.listen()
105137
if err != nil {
106138
return err
107139
}
108-
return c.startWithHost(ctx, host)
140+
return c.startWithHost(ctx, h)
109141
}
110142

111143
func (c *Client) startWithHost(ctx context.Context, h host.Host) error {
@@ -144,10 +176,14 @@ func (c *Client) startWithHost(ctx context.Context, h host.Host) error {
144176

145177
// Close gently stops Client.
146178
func (c *Client) Close() error {
147-
return errors.Join(
148-
c.dht.Close(),
149-
c.host.Close(),
150-
)
179+
var dhtErr, hostErr error
180+
if c.dht != nil {
181+
dhtErr = c.dht.Close()
182+
}
183+
if c.host != nil {
184+
hostErr = c.host.Close()
185+
}
186+
return errors.Join(dhtErr, hostErr)
151187
}
152188

153189
// Addrs returns listen addresses of Client.
@@ -310,6 +346,10 @@ func (c *Client) findPeers(ctx context.Context) error {
310346

311347
// tryConnect attempts to connect to a peer and logs error if necessary
312348
func (c *Client) tryConnect(ctx context.Context, peer peer.AddrInfo) {
349+
if peer.ID == c.host.ID() {
350+
return
351+
}
352+
313353
err := c.host.Connect(ctx, peer)
314354
if err != nil && ctx.Err() == nil {
315355
c.logger.Error("failed to connect to peer", "peer", peer, "error", err)

pkg/p2p/client_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,77 @@ import (
1010
"cosmossdk.io/log"
1111
"github.com/ipfs/go-datastore"
1212
dssync "github.com/ipfs/go-datastore/sync"
13+
libp2p "github.com/libp2p/go-libp2p"
1314
"github.com/libp2p/go-libp2p/core/peer"
1415
"github.com/multiformats/go-multiaddr"
1516
"github.com/stretchr/testify/assert"
1617
"github.com/stretchr/testify/require"
1718

19+
mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
20+
1821
"github.com/rollkit/rollkit/pkg/config"
1922
"github.com/rollkit/rollkit/pkg/p2p/key"
2023
)
2124

25+
func TestNewClientWithHost(t *testing.T) {
26+
require := require.New(t)
27+
assert := assert.New(t)
28+
29+
// Common setup
30+
conf := config.DefaultConfig
31+
conf.RootDir = t.TempDir()
32+
nodeKey, err := key.LoadOrGenNodeKey(filepath.Join(conf.RootDir, "config", "node_key.json"))
33+
require.NoError(err)
34+
ds := dssync.MutexWrap(datastore.NewMapDatastore())
35+
logger := log.NewTestLogger(t)
36+
metrics := NopMetrics()
37+
38+
// Ensure config directory exists for nodeKey loading
39+
ClientInitFiles(t, conf.RootDir)
40+
41+
t.Run("successful client creation with injected host", func(t *testing.T) {
42+
// First, create a client to get its expected gater
43+
baseClient, err := NewClient(conf, nodeKey, ds, logger, metrics)
44+
require.NoError(err)
45+
require.NotNil(baseClient)
46+
47+
mn := mocknet.New()
48+
defer mn.Close()
49+
50+
h, err := libp2p.New(
51+
libp2p.Identity(nodeKey.PrivKey),
52+
libp2p.ListenAddrs(multiaddr.StringCast("/ip4/127.0.0.1/tcp/0")),
53+
libp2p.ConnectionGater(baseClient.ConnectionGater()), // Use the gater from baseClient
54+
)
55+
require.NoError(err)
56+
defer h.Close()
57+
58+
client, err := NewClientWithHost(conf, nodeKey, ds, logger, metrics, h)
59+
assert.NoError(err)
60+
assert.NotNil(client)
61+
assert.Equal(h, client.Host())
62+
})
63+
64+
t.Run("error when injected host ID does not match node key ID", func(t *testing.T) {
65+
mn := mocknet.New()
66+
defer mn.Close()
67+
68+
// Generate a different node key for the host
69+
otherNodeKey, err := key.GenerateNodeKey()
70+
require.NoError(err)
71+
72+
h, err := mn.AddPeer(otherNodeKey.PrivKey, multiaddr.StringCast("/ip4/127.0.0.1/tcp/0"))
73+
require.NoError(err)
74+
75+
client, err := NewClientWithHost(conf, nodeKey, ds, logger, metrics, h)
76+
assert.Error(err)
77+
assert.Nil(client)
78+
assert.Contains(err.Error(), "injected host ID")
79+
assert.Contains(err.Error(), "does not match node key ID")
80+
})
81+
82+
}
83+
2284
func TestClientStartup(t *testing.T) {
2385
assert := assert.New(t)
2486
// create temp config dir

pkg/sync/sync_service_test.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/ipfs/go-datastore"
1313
"github.com/ipfs/go-datastore/sync"
1414
"github.com/libp2p/go-libp2p/core/crypto"
15+
mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
1516
"github.com/rollkit/rollkit/pkg/config"
1617
genesispkg "github.com/rollkit/rollkit/pkg/genesis"
1718
"github.com/rollkit/rollkit/pkg/p2p"
@@ -29,6 +30,7 @@ func TestHeaderSyncServiceRestart(t *testing.T) {
2930
noopSigner, err := noop.NewNoopSigner(pk)
3031
require.NoError(t, err)
3132
rnd := rand.New(rand.NewSource(1)) // nolint:gosec // test code only
33+
mn := mocknet.New()
3234

3335
proposerAddr := []byte("test")
3436
genesisDoc := genesispkg.Genesis{
@@ -42,14 +44,17 @@ func TestHeaderSyncServiceRestart(t *testing.T) {
4244
nodeKey, err := key.LoadOrGenNodeKey(filepath.Dir(conf.ConfigPath()))
4345
require.NoError(t, err)
4446
logger := sdklog.NewTestLogger(t)
45-
p2pClient, err := p2p.NewClient(conf, nodeKey, mainKV, logger, p2p.NopMetrics())
47+
priv := nodeKey.PrivKey
48+
h, err := mn.AddPeer(priv, nil)
49+
require.NoError(t, err)
50+
51+
p2pClient, err := p2p.NewClientWithHost(conf, nodeKey, mainKV, logger, p2p.NopMetrics(), h)
4652
require.NoError(t, err)
4753

4854
// Start p2p client before creating sync service
4955
ctx, cancel := context.WithCancel(t.Context())
5056
defer cancel()
51-
err = p2pClient.Start(ctx)
52-
require.NoError(t, err)
57+
require.NoError(t, p2pClient.Start(ctx))
5358

5459
svc, err := NewHeaderSyncService(mainKV, conf, genesisDoc, p2pClient, logger)
5560
require.NoError(t, err)
@@ -79,7 +84,9 @@ func TestHeaderSyncServiceRestart(t *testing.T) {
7984
_ = svc.Stop(ctx)
8085
cancel()
8186

82-
p2pClient, err = p2p.NewClient(conf, nodeKey, mainKV, logger, p2p.NopMetrics())
87+
h2, err := mn.AddPeer(priv, nil)
88+
require.NoError(t, err)
89+
p2pClient, err = p2p.NewClientWithHost(conf, nodeKey, mainKV, logger, p2p.NopMetrics(), h2)
8390
require.NoError(t, err)
8491

8592
// Start p2p client again

0 commit comments

Comments
 (0)