Skip to content

Commit ebac4e2

Browse files
martintomazicmartintomazic
committed
fixup
1 parent 4109161 commit ebac4e2

File tree

9 files changed

+32
-23
lines changed

9 files changed

+32
-23
lines changed

‎go/storage/mkvs/checkpoint/checkpoint.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,12 @@ type ChunkProvider interface {
4848

4949
// GetCheckpointsRequest is a GetCheckpoints request.
5050
type GetCheckpointsRequest struct {
51-
Version uint16 `json:"version"`
51+
// Version is version of the checkpoint request.
52+
Version uint16 `json:"version"`
53+
// Namespace is the namespace this checkpoint is for.
54+
//
55+
// Existing server implementation may ignore validation of this field.
56+
// Best practice is to still pass it, but not rely on the actual validation.
5257
Namespace common.Namespace `json:"namespace"`
5358

5459
// RootVersion specifies an optional root version to limit the request to. If specified, only

‎go/worker/storage/committee/node.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,7 @@ func NewNode(
276276
})
277277

278278
// Advertise and serve legacy storage sync protocol.
279-
// TODO #6270 will move advertisement part out of the protocol registration.
280279
commonNode.P2P.RegisterProtocolServer(synclegacy.NewServer(commonNode.ChainContext, commonNode.Runtime.ID(), localStorage))
281-
commonNode.P2P.RegisterProtocol(synclegacy.GetStorageSyncProtocolID(commonNode.ChainContext, commonNode.Runtime.ID()), 5, 10)
282280

283281
// Register diff sync protocol server.
284282
commonNode.P2P.RegisterProtocolServer(diffsync.NewServer(commonNode.ChainContext, commonNode.Runtime.ID(), localStorage))
@@ -288,8 +286,6 @@ func NewNode(
288286
}
289287

290288
// Create diff and checkpoint sync p2p protocol clients that have a fallback to the old legacy storage sync protocol.
291-
// TODO: This automatically starts advertising the given protocol even if the server is not registered.
292-
// #6270 wil fix that by moving advertisement out of the client creation.
293289
n.diffSync = diffsync.NewClient(commonNode.P2P, commonNode.ChainContext, commonNode.Runtime.ID())
294290
n.checkpointSync = checkpointsync.NewClient(commonNode.P2P, commonNode.ChainContext, commonNode.Runtime.ID())
295291

‎go/worker/storage/p2p/checkpointsync/client.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,8 @@ func (c *client) getBestPeers(opts ...rpc.BestPeersOption) []core.PeerID {
120120
// upgrades of the network, this client has a fallback to the old legacy protocol.
121121
// The new protocol is prioritized.
122122
//
123-
// Warning: This client only registers the checkpoint sync protocol with the P2P
124-
// service. To enable advertisement of the legacy protocol, it must be registered
125-
// separately.
123+
// Finally, it ensures underlying p2p service starts tracking protocol peers
124+
// for both new and legacy protocol.
126125
func NewClient(p2p rpc.P2P, chainContext string, runtimeID common.Namespace) Client {
127126
pid := protocol.NewRuntimeProtocolID(chainContext, runtimeID, CheckpointSyncProtocolID, CheckpointSyncProtocolVersion)
128127
fallbackPid := synclegacy.GetStorageSyncProtocolID(chainContext, runtimeID)
@@ -135,6 +134,7 @@ func NewClient(p2p rpc.P2P, chainContext string, runtimeID common.Namespace) Cli
135134
rc.RegisterListener(fallbackMgr)
136135

137136
p2p.RegisterProtocol(pid, minProtocolPeers, totalProtocolPeers)
137+
p2p.RegisterProtocol(fallbackPid, minProtocolPeers, totalProtocolPeers)
138138

139139
return &client{
140140
rc: rc,

‎go/worker/storage/p2p/checkpointsync/protocol.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/libp2p/go-libp2p/core"
99

10+
"github.com/oasisprotocol/oasis-core/go/common"
1011
"github.com/oasisprotocol/oasis-core/go/common/crypto/hash"
1112
"github.com/oasisprotocol/oasis-core/go/common/node"
1213
"github.com/oasisprotocol/oasis-core/go/common/version"
@@ -22,6 +23,11 @@ const CheckpointSyncProtocolID = "checkpointsync"
2223
// CheckpointSyncProtocolVersion is the supported version of the checkpoint sync protocol.
2324
var CheckpointSyncProtocolVersion = version.Version{Major: 1, Minor: 0, Patch: 0}
2425

26+
// ProtocolID returns the runtime checkpoint sync protocol ID.
27+
func ProtocolID(chainContext string, runtimeID common.Namespace) core.ProtocolID {
28+
return protocol.NewRuntimeProtocolID(chainContext, runtimeID, CheckpointSyncProtocolID, CheckpointSyncProtocolVersion)
29+
}
30+
2531
// Constants related to the GetCheckpoints method.
2632
const (
2733
MethodGetCheckpoints = "GetCheckpoints"
@@ -40,7 +46,7 @@ type GetCheckpointsResponse struct {
4046
// Constants related to the GetCheckpointChunk method.
4147
const (
4248
MethodGetCheckpointChunk = "GetCheckpointChunk"
43-
MaxGetCheckpointChunkResponseTime = 60 * time.Second
49+
MaxGetCheckpointChunkResponseTime = 1 * time.Minute
4450
)
4551

4652
// GetCheckpointChunkRequest is a GetCheckpointChunk request.

‎go/worker/storage/p2p/checkpointsync/server.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66

77
"github.com/oasisprotocol/oasis-core/go/common"
88
"github.com/oasisprotocol/oasis-core/go/common/cbor"
9-
"github.com/oasisprotocol/oasis-core/go/p2p/protocol"
109
"github.com/oasisprotocol/oasis-core/go/p2p/rpc"
1110
"github.com/oasisprotocol/oasis-core/go/storage/api"
1211
"github.com/oasisprotocol/oasis-core/go/storage/mkvs/checkpoint"
@@ -70,5 +69,5 @@ func (s *service) handleGetCheckpointChunk(ctx context.Context, request *GetChec
7069

7170
// NewServer creates a new checkpoints protocol server.
7271
func NewServer(chainContext string, runtimeID common.Namespace, backend api.Backend) rpc.Server {
73-
return rpc.NewServer(protocol.NewRuntimeProtocolID(chainContext, runtimeID, CheckpointSyncProtocolID, CheckpointSyncProtocolVersion), &service{backend})
72+
return rpc.NewServer(ProtocolID(chainContext, runtimeID), &service{backend})
7473
}

‎go/worker/storage/p2p/diffsync/client.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,8 @@ func (c *client) GetDiff(ctx context.Context, request *GetDiffRequest) (*GetDiff
4949
// upgrades of the network, this client has a fallback to the old legacy protocol.
5050
// The new protocol is prioritized.
5151
//
52-
// Warning: This client only registers the diff sync protocol with the P2P
53-
// service. To enable advertisement of the legacy protocol, it must be registered
54-
// separately.
52+
// Finally, it ensures underlying p2p service starts tracking protocol peers
53+
// for both new and legacy protocol.
5554
func NewClient(p2p rpc.P2P, chainContext string, runtimeID common.Namespace) Client {
5655
pid := protocol.NewRuntimeProtocolID(chainContext, runtimeID, DiffSyncProtocolID, DiffSyncProtocolVersion)
5756
fallbackPid := synclegacy.GetStorageSyncProtocolID(chainContext, runtimeID)
@@ -64,6 +63,7 @@ func NewClient(p2p rpc.P2P, chainContext string, runtimeID common.Namespace) Cli
6463
rc.RegisterListener(fallbackMgr)
6564

6665
p2p.RegisterProtocol(pid, minProtocolPeers, totalProtocolPeers)
66+
p2p.RegisterProtocol(fallbackPid, minProtocolPeers, totalProtocolPeers)
6767

6868
return &client{
6969
rc: rc,

‎go/worker/storage/p2p/diffsync/protocol.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/libp2p/go-libp2p/core"
99

10+
"github.com/oasisprotocol/oasis-core/go/common"
1011
"github.com/oasisprotocol/oasis-core/go/common/node"
1112
"github.com/oasisprotocol/oasis-core/go/common/version"
1213
"github.com/oasisprotocol/oasis-core/go/p2p/peermgmt"
@@ -20,6 +21,11 @@ const DiffSyncProtocolID = "diffsync"
2021
// DiffSyncProtocolVersion is the supported version of the diff sync protocol.
2122
var DiffSyncProtocolVersion = version.Version{Major: 1, Minor: 0, Patch: 0}
2223

24+
// ProtocolID returns the runtime diff sync protocol ID.
25+
func ProtocolID(chainContext string, runtimeID common.Namespace) core.ProtocolID {
26+
return protocol.NewRuntimeProtocolID(chainContext, runtimeID, DiffSyncProtocolID, DiffSyncProtocolVersion)
27+
}
28+
2329
// Constants related to the GetDiff method.
2430
const (
2531
MethodGetDiff = "GetDiff"

‎go/worker/storage/p2p/diffsync/server.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55

66
"github.com/oasisprotocol/oasis-core/go/common"
77
"github.com/oasisprotocol/oasis-core/go/common/cbor"
8-
"github.com/oasisprotocol/oasis-core/go/p2p/protocol"
98
"github.com/oasisprotocol/oasis-core/go/p2p/rpc"
109
"github.com/oasisprotocol/oasis-core/go/storage/api"
1110
)
@@ -58,5 +57,5 @@ func (s *service) handleGetDiff(ctx context.Context, request *GetDiffRequest) (*
5857

5958
// NewServer creates a new storage diff protocol server.
6059
func NewServer(chainContext string, runtimeID common.Namespace, backend api.Backend) rpc.Server {
61-
return rpc.NewServer(protocol.NewRuntimeProtocolID(chainContext, runtimeID, DiffSyncProtocolID, DiffSyncProtocolVersion), &service{backend})
60+
return rpc.NewServer(ProtocolID(chainContext, runtimeID), &service{backend})
6261
}

‎go/worker/storage/p2p/sync_test/interoperable_test.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func assertEqualCheckpointMeta(this, other *checkpoint.Metadata) error {
213213
if this.Version != other.Version {
214214
return fmt.Errorf("version not equal")
215215
}
216-
if this.Root != other.Root {
216+
if this.Root.Equal(&other.Root) {
217217
return fmt.Errorf("root not equal")
218218
}
219219
if len(this.Chunks) != len(other.Chunks) {
@@ -337,12 +337,10 @@ var (
337337
func (bm *backendMock) GetCheckpoints(_ context.Context, request *checkpoint.GetCheckpointsRequest) ([]*checkpoint.Metadata, error) {
338338
cpVersion = request.Version
339339
root = node.Root{
340-
// Existing bug: storagesync p2p server does not set namespace, nor does checkpoint.ChunkProvider validates it.
341-
// As a result empty namespace is always passed around, thus commenting Namespace field below.
342-
// Namespace: request.Namespace
343-
Version: 1,
344-
Type: api.RootTypeState,
345-
Hash: hashFromBytes([]byte("root has")),
340+
Namespace: request.Namespace,
341+
Version: 1,
342+
Type: api.RootTypeState,
343+
Hash: hashFromBytes([]byte("root has")),
346344
}
347345
cp := &checkpoint.Metadata{
348346
Version: cpVersion,

0 commit comments

Comments
 (0)