diff --git a/.github/workflows/client.yml b/.github/workflows/client.yml index f719505eee..95d80c5958 100644 --- a/.github/workflows/client.yml +++ b/.github/workflows/client.yml @@ -134,7 +134,7 @@ jobs: docker run \ --workdir /go/src/github.com/keep-network/keep-core \ go-build-env \ - gotestsum -- -timeout 15m + gotestsum -- -timeout 15m ./... - name: Build Docker Runtime Image if: github.event_name != 'workflow_dispatch' diff --git a/cmd/flags.go b/cmd/flags.go index 6ce094c2e6..1de77bffff 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -202,11 +202,13 @@ func initBitcoinElectrumFlags(cmd *cobra.Command, cfg *config.Config) { // Initialize flags for Network configuration. func initNetworkFlags(cmd *cobra.Command, cfg *config.Config) { + // TODO: Remove in v3.0.0 along with isBootstrap() in start.go and + // the LibP2P.Bootstrap config field. cmd.Flags().BoolVar( &cfg.LibP2P.Bootstrap, "network.bootstrap", false, - "Run the client in bootstrap mode.", + "[DEPRECATED: remove in v3.0] Run the client in bootstrap mode. This flag is deprecated and will be removed in v3.0.", ) cmd.Flags().StringSliceVar( diff --git a/cmd/start.go b/cmd/start.go index cfaece274c..4a1726977b 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -188,6 +188,9 @@ func start(cmd *cobra.Command) error { } func isBootstrap() bool { + if clientConfig.LibP2P.Bootstrap { + logger.Warnf("--network.bootstrap is deprecated and will be removed in a future release") + } return clientConfig.LibP2P.Bootstrap } @@ -197,19 +200,9 @@ func initializeNetwork( operatorPrivateKey *operator.PrivateKey, blockCounter chain.BlockCounter, ) (net.Provider, error) { - bootstrapPeersPublicKeys, err := libp2p.ExtractPeersPublicKeys( - clientConfig.LibP2P.Peers, - ) - if err != nil { - return nil, fmt.Errorf( - "error extracting bootstrap peers public keys: [%v]", - err, - ) - } - firewall := firewall.AnyApplicationPolicy( applications, - firewall.NewAllowList(bootstrapPeersPublicKeys), + firewall.EmptyAllowList(), ) netProvider, err := libp2p.Connect( @@ -244,7 +237,7 @@ func initializeClientInfo( config.ClientInfo.NetworkMetricsTick, ) - registry.ObserveConnectedBootstrapCount( + registry.ObserveConnectedWellknownPeersCount( netProvider, config.LibP2P.Peers, config.ClientInfo.NetworkMetricsTick, diff --git a/cmd/start_test.go b/cmd/start_test.go new file mode 100644 index 0000000000..8c2d695c21 --- /dev/null +++ b/cmd/start_test.go @@ -0,0 +1,59 @@ +package cmd + +import ( + "strings" + "testing" + + "github.com/keep-network/keep-core/config" + "github.com/spf13/cobra" +) + +func TestNetworkBootstrapFlagDescription_ContainsDeprecationNotice(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cfg := &config.Config{} + + initNetworkFlags(cmd, cfg) + + flag := cmd.Flags().Lookup("network.bootstrap") + if flag == nil { + t.Fatal("expected network.bootstrap flag to be registered") + } + + usageLower := strings.ToLower(flag.Usage) + if !strings.Contains(usageLower, "deprecated") { + t.Errorf( + "expected flag description to contain deprecation notice, got: %q", + flag.Usage, + ) + } +} + +func TestIsBootstrap(t *testing.T) { + tests := map[string]struct { + bootstrapValue bool + expected bool + }{ + "returns true when bootstrap flag is set": { + bootstrapValue: true, + expected: true, + }, + "returns false when bootstrap flag is not set": { + bootstrapValue: false, + expected: false, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + originalBootstrap := clientConfig.LibP2P.Bootstrap + defer func() { clientConfig.LibP2P.Bootstrap = originalBootstrap }() + + clientConfig.LibP2P.Bootstrap = tc.bootstrapValue + + got := isBootstrap() + if got != tc.expected { + t.Errorf("expected isBootstrap() to return %v, got %v", tc.expected, got) + } + }) + } +} diff --git a/config/_peers/mainnet b/config/_peers/mainnet index c7b87358e4..e5020c4207 100644 --- a/config/_peers/mainnet +++ b/config/_peers/mainnet @@ -1,2 +1,4 @@ -/dns4/bst-a01.tbtc.boar.network/tcp/5001/ipfs/16Uiu2HAmAmCrLuUmnBgpavU8y8JBUN6jWAQ93JwydZy3ABRyY6wU -/dns4/bst-b01.tbtc.boar.network/tcp/5001/ipfs/16Uiu2HAm4w5HdJQxBnadGRepaiGfWVvtMzhdAGZVcrf9i71mv69V +# TODO: Add at least one additional mainnet peer across a different +# operator/ASN before production rollout. A single peer is a SPOF for +# initial peer discovery of fresh nodes. +/ip4/143.198.18.229/tcp/3919/ipfs/16Uiu2HAmDP4Z6LCogRMictJ6deGs4DRo99A5JTz5u3CLMg7URxC6 diff --git a/config/_peers/testnet b/config/_peers/testnet index d86e88643f..8e4f9e3775 100644 --- a/config/_peers/testnet +++ b/config/_peers/testnet @@ -1 +1,2 @@ -/dns4/bst-a01.test.keep.boar.network/tcp/6001/ipfs/16Uiu2HAmSLDSahiKyTbCNNu8wJmZAsiKF7wuYJ8mogY8ZuAG1jhu +/dns4/keep-operator-1.test.keep-nodes.io/tcp/3920/ipfs/16Uiu2HAmDrk2Bh4VNPUJfKRHTE2CvH9xfKzN4KFnmRJbGLkJFDqL +/dns4/keep-operator-2.test.keep-nodes.io/tcp/3920/ipfs/16Uiu2HAm3ex8rGzwFpWYbRreRUiX9JEYCKxp7KDMzB8RZ6fQWnMa diff --git a/config/peers_test.go b/config/peers_test.go index 56892b5028..c311d0bd7f 100644 --- a/config/peers_test.go +++ b/config/peers_test.go @@ -18,13 +18,13 @@ func TestResolvePeers(t *testing.T) { "mainnet network": { network: network.Mainnet, expectedPeers: []string{ - "/dns4/bst-a01.tbtc.boar.network/tcp/5001/ipfs/16Uiu2HAmAmCrLuUmnBgpavU8y8JBUN6jWAQ93JwydZy3ABRyY6wU", - "/dns4/bst-b01.tbtc.boar.network/tcp/5001/ipfs/16Uiu2HAm4w5HdJQxBnadGRepaiGfWVvtMzhdAGZVcrf9i71mv69V", + "/ip4/143.198.18.229/tcp/3919/ipfs/16Uiu2HAmDP4Z6LCogRMictJ6deGs4DRo99A5JTz5u3CLMg7URxC6", }}, "sepolia network": { network: network.Testnet, expectedPeers: []string{ - "/dns4/bst-a01.test.keep.boar.network/tcp/6001/ipfs/16Uiu2HAmSLDSahiKyTbCNNu8wJmZAsiKF7wuYJ8mogY8ZuAG1jhu", + "/dns4/keep-operator-1.test.keep-nodes.io/tcp/3920/ipfs/16Uiu2HAmDrk2Bh4VNPUJfKRHTE2CvH9xfKzN4KFnmRJbGLkJFDqL", + "/dns4/keep-operator-2.test.keep-nodes.io/tcp/3920/ipfs/16Uiu2HAm3ex8rGzwFpWYbRreRUiX9JEYCKxp7KDMzB8RZ6fQWnMa", }, }, "developer network": { diff --git a/go.mod b/go.mod index 802a5e4a2e..3b604d831e 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.24 toolchain go1.24.1 + replace ( github.com/bnb-chain/tss-lib => github.com/threshold-network/tss-lib v0.0.0-20230901144531-2e712689cfbe // btcd in version v.0.23 extracted `btcd/btcec` to a separate package `btcd/btcec/v2`. diff --git a/pkg/clientinfo/metrics.go b/pkg/clientinfo/metrics.go index c80755cab8..91af36bcd8 100644 --- a/pkg/clientinfo/metrics.go +++ b/pkg/clientinfo/metrics.go @@ -14,12 +14,16 @@ import ( type Source func() float64 // Names under which metrics are exposed. +// +// NOTE: ConnectedWellknownPeersCountMetricName was renamed from +// "connected_bootstrap_count" in v2.6.0. Update any Prometheus queries or +// Grafana dashboards that reference the old name. const ( - ConnectedPeersCountMetricName = "connected_peers_count" - ConnectedBootstrapCountMetricName = "connected_bootstrap_count" - EthConnectivityMetricName = "eth_connectivity" - BtcConnectivityMetricName = "btc_connectivity" - ClientInfoMetricName = "client_info" + ConnectedPeersCountMetricName = "connected_peers_count" + ConnectedWellknownPeersCountMetricName = "connected_wellknown_peers_count" + EthConnectivityMetricName = "eth_connectivity" + BtcConnectivityMetricName = "btc_connectivity" + ClientInfoMetricName = "client_info" ) const ( @@ -55,17 +59,17 @@ func (r *Registry) ObserveConnectedPeersCount( ) } -// ObserveConnectedBootstrapCount triggers an observation process of the -// connected_bootstrap_count metric. -func (r *Registry) ObserveConnectedBootstrapCount( +// ObserveConnectedWellknownPeersCount triggers an observation process of the +// connected_wellknown_peers_count metric. +func (r *Registry) ObserveConnectedWellknownPeersCount( netProvider net.Provider, - bootstraps []string, + wellknownPeers []string, tick time.Duration, ) { input := func() float64 { currentCount := 0 - for _, address := range bootstraps { + for _, address := range wellknownPeers { if netProvider.ConnectionManager().IsConnected(address) { currentCount++ } @@ -75,7 +79,7 @@ func (r *Registry) ObserveConnectedBootstrapCount( } r.observe( - ConnectedBootstrapCountMetricName, + ConnectedWellknownPeersCountMetricName, input, validateTick(tick, DefaultNetworkMetricsTick), ) diff --git a/pkg/clientinfo/metrics_test.go b/pkg/clientinfo/metrics_test.go new file mode 100644 index 0000000000..18769a8e0d --- /dev/null +++ b/pkg/clientinfo/metrics_test.go @@ -0,0 +1,91 @@ +package clientinfo + +import ( + "context" + "testing" + "time" + + keepclientinfo "github.com/keep-network/keep-common/pkg/clientinfo" + "github.com/keep-network/keep-core/pkg/net" + "github.com/keep-network/keep-core/pkg/operator" +) + +// mockTransportIdentifier implements net.TransportIdentifier for testing. +type mockTransportIdentifier struct{} + +func (m *mockTransportIdentifier) String() string { return "mock-id" } + +// mockConnectionManager implements net.ConnectionManager for testing. +type mockConnectionManager struct { + connectedAddresses map[string]bool +} + +func (m *mockConnectionManager) ConnectedPeers() []string { return nil } +func (m *mockConnectionManager) ConnectedPeersAddrInfo() map[string][]string { + return nil +} +func (m *mockConnectionManager) GetPeerPublicKey(string) (*operator.PublicKey, error) { + return nil, nil +} +func (m *mockConnectionManager) DisconnectPeer(string) {} +func (m *mockConnectionManager) AddrStrings() []string { return nil } +func (m *mockConnectionManager) IsConnected(address string) bool { + if m.connectedAddresses == nil { + return false + } + return m.connectedAddresses[address] +} + +// mockProvider implements net.Provider for testing. +type mockProvider struct { + connectionManager net.ConnectionManager +} + +func (m *mockProvider) ID() net.TransportIdentifier { return &mockTransportIdentifier{} } +func (m *mockProvider) Type() string { return "mock" } +func (m *mockProvider) BroadcastChannelFor(string) (net.BroadcastChannel, error) { + return nil, nil +} +func (m *mockProvider) ConnectionManager() net.ConnectionManager { + return m.connectionManager +} +func (m *mockProvider) CreateTransportIdentifier( + *operator.PublicKey, +) (net.TransportIdentifier, error) { + return nil, nil +} +func (m *mockProvider) BroadcastChannelForwarderFor(string) {} + +// TestObserveConnectedWellknownPeersCount_Callable verifies that the renamed +// function exists on the Registry type and can be called without panicking. +func TestObserveConnectedWellknownPeersCount_Callable(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + registry := &Registry{keepclientinfo.NewRegistry(), ctx} + + provider := &mockProvider{ + connectionManager: &mockConnectionManager{ + connectedAddresses: map[string]bool{ + "/ip4/127.0.0.1/tcp/3919": true, + }, + }, + } + + // The function should execute without panic. We use a recovered call + // to detect if the method does not exist or panics. + defer func() { + if r := recover(); r != nil { + t.Fatalf( + "ObserveConnectedWellknownPeersCount panicked: %v", + r, + ) + } + }() + + registry.ObserveConnectedWellknownPeersCount( + provider, + []string{"/ip4/127.0.0.1/tcp/3919"}, + 1*time.Minute, + ) +} diff --git a/pkg/firewall/firewall.go b/pkg/firewall/firewall.go index f657870659..d1f26489d8 100644 --- a/pkg/firewall/firewall.go +++ b/pkg/firewall/firewall.go @@ -48,8 +48,16 @@ func (al *AllowList) Contains(operatorPublicKey *operator.PublicKey) bool { return al.allowedPublicKeys[operatorPublicKey.String()] } -// EmptyAllowList represents an empty firewall allowlist. -var EmptyAllowList = NewAllowList([]*operator.PublicKey{}) +// emptyAllowList is the singleton empty allowlist used in production. +// All peers must pass IsRecognized checks; no bypass is available. +var emptyAllowList = NewAllowList([]*operator.PublicKey{}) + +// EmptyAllowList returns the empty firewall allowlist. In production, this +// ensures all peers are subject to on-chain staking verification with no +// AllowList bypass. +func EmptyAllowList() *AllowList { + return emptyAllowList +} const ( // PositiveIsRecognizedCachePeriod is the time period the cache maintains diff --git a/pkg/firewall/firewall_test.go b/pkg/firewall/firewall_test.go index 6354aec87b..8598411120 100644 --- a/pkg/firewall/firewall_test.go +++ b/pkg/firewall/firewall_test.go @@ -16,7 +16,7 @@ const cachingPeriod = time.Second func TestValidate_PeerNotRecognized_NoApplications(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -44,7 +44,7 @@ func TestValidate_PeerNotRecognized_MultipleApplications(t *testing.T) { applications: []Application{ newMockApplication(), newMockApplication()}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -71,7 +71,7 @@ func TestValidate_PeerRecognized_FirstApplicationRecognizes(t *testing.T) { applications: []Application{ application, newMockApplication()}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -100,7 +100,7 @@ func TestValidate_PeerRecognized_SecondApplicationRecognizes(t *testing.T) { applications: []Application{ newMockApplication(), application}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -139,7 +139,7 @@ func TestValidate_PeerNotRecognized_FirstApplicationReturnedError(t *testing.T) applications: []Application{ application1, application2}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -164,7 +164,7 @@ func TestValidate_PeerRecognized_Cached(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{application}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -203,7 +203,7 @@ func TestValidate_PeerNotRecognized_CacheEmptied(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{application}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -238,7 +238,7 @@ func TestValidate_PeerNotRecognized_Cached(t *testing.T) { application := newMockApplication() policy := &anyApplicationPolicy{ applications: []Application{application}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -273,7 +273,7 @@ func TestValidate_PeerRecognized_CacheEmptied(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{application}, - allowList: EmptyAllowList, + allowList: EmptyAllowList(), positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -305,8 +305,10 @@ func TestValidate_PeerIsAllowlistedNode(t *testing.T) { t.Fatal(err) } - // Mark the peer as an allowlisted node, so that it validated despite not - // being recognized by any application + // This test validates that the AllowList type mechanism still works + // correctly at the type level. In production, EmptyAllowList is used, + // so no peer receives this bypass. See the EmptyAllowList tests below + // for the production-relevant security behavior. allowList := NewAllowList([]*operator.PublicKey{peerOperatorPublicKey}) policy := &anyApplicationPolicy{ @@ -322,6 +324,79 @@ func TestValidate_PeerIsAllowlistedNode(t *testing.T) { } } +func TestValidate_EmptyAllowList_RecognizedPeerAccepted(t *testing.T) { + _, peerOperatorPublicKey, err := operator.GenerateKeyPair( + local_v1.DefaultCurve, + ) + if err != nil { + t.Fatal(err) + } + + application := newMockApplication() + application.setIsRecognized(peerOperatorPublicKey, result{ + isRecognized: true, + err: nil, + }) + + // With EmptyAllowList(), a recognized peer must pass validation through + // the IsRecognized path, not through an AllowList bypass. + policy := &anyApplicationPolicy{ + applications: []Application{application}, + allowList: EmptyAllowList(), + positiveResultCache: cache.NewTimeCache(cachingPeriod), + negativeResultCache: cache.NewTimeCache(cachingPeriod), + } + + err = policy.Validate(peerOperatorPublicKey) + if err != nil { + t.Fatal(err) + } +} + +func TestValidate_EmptyAllowList_UnrecognizedPeerRejected(t *testing.T) { + _, peerOperatorPublicKey, err := operator.GenerateKeyPair( + local_v1.DefaultCurve, + ) + if err != nil { + t.Fatal(err) + } + + // With EmptyAllowList(), a peer not recognized by any application must + // be rejected. No AllowList bypass is available. + policy := &anyApplicationPolicy{ + applications: []Application{newMockApplication()}, + allowList: EmptyAllowList(), + positiveResultCache: cache.NewTimeCache(cachingPeriod), + negativeResultCache: cache.NewTimeCache(cachingPeriod), + } + + err = policy.Validate(peerOperatorPublicKey) + testutils.AssertErrorsSame(t, errNotRecognized, err) +} + +func TestValidate_EmptyAllowList_PreviouslyAllowlistedPeerMustPassIsRecognized(t *testing.T) { + _, peerOperatorPublicKey, err := operator.GenerateKeyPair( + local_v1.DefaultCurve, + ) + if err != nil { + t.Fatal(err) + } + + // This is the core security assertion: a peer that would have been on + // a populated AllowList (e.g., a bootstrap node) is now subject to + // standard IsRecognized staking checks when EmptyAllowList is used. + // The peer is not recognized by the application and must be rejected. + policy := &anyApplicationPolicy{ + applications: []Application{newMockApplication()}, + allowList: EmptyAllowList(), + positiveResultCache: cache.NewTimeCache(cachingPeriod), + negativeResultCache: cache.NewTimeCache(cachingPeriod), + } + + err = policy.Validate(peerOperatorPublicKey) + testutils.AssertErrorsSame(t, errNotRecognized, err) +} + func newMockApplication() *mockApplication { return &mockApplication{ results: make(map[*operator.PublicKey]result), diff --git a/pkg/net/libp2p/libp2p.go b/pkg/net/libp2p/libp2p.go index 4d429a8f4f..0d8339df86 100644 --- a/pkg/net/libp2p/libp2p.go +++ b/pkg/net/libp2p/libp2p.go @@ -716,44 +716,3 @@ func multiaddressWithIdentity( ) string { return fmt.Sprintf("%s/ipfs/%s", multiaddress.String(), peerID.String()) } - -// ExtractPeersPublicKeys returns a list of operator public keys based on the -// provided list of peer addresses. Peer addresses must be in the format: -// /ipfs/ -func ExtractPeersPublicKeys(peerAddresses []string) ([]*operator.PublicKey, error) { - peerInfos, err := extractMultiAddrFromPeers(peerAddresses) - if err != nil { - return nil, fmt.Errorf( - "failed to extract multiaddress from peer addresses: [%v]", - err, - ) - } - - peersPublicKeys := make([]*operator.PublicKey, 0, len(peerInfos)) - - for _, peerInfo := range peerInfos { - peerNetworkPublicKey, err := peerInfo.ID.ExtractPublicKey() - if err != nil { - return nil, fmt.Errorf( - "failed to extract network public key for peer [%s]: [%v]", - peerInfo.ID.String(), - err, - ) - } - - peerOperatorPublicKey, err := networkPublicKeyToOperatorPublicKey( - peerNetworkPublicKey, - ) - if err != nil { - return nil, fmt.Errorf( - "failed to convert to operator public key for peer [%s]: [%v]", - peerInfo.ID.String(), - err, - ) - } - - peersPublicKeys = append(peersPublicKeys, peerOperatorPublicKey) - } - - return peersPublicKeys, nil -} diff --git a/pkg/net/libp2p/libp2p_test.go b/pkg/net/libp2p/libp2p_test.go index 2b9ec08e25..0564a65cfe 100644 --- a/pkg/net/libp2p/libp2p_test.go +++ b/pkg/net/libp2p/libp2p_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "reflect" "sort" "strings" "testing" @@ -204,103 +203,6 @@ func TestProviderSetAnnouncedAddresses(t *testing.T) { } } -func TestExtractPeersPublicKeys_EmptyList(t *testing.T) { - peerAddresses := []string{} - peerOperatorPublicKeys, err := ExtractPeersPublicKeys(peerAddresses) - if err != nil { - t.Fatal(err) - } - - if len(peerOperatorPublicKeys) != len(peerAddresses) { - t.Errorf( - "unexpected peer operator public keys length\nexpected: %v\n"+ - "actual: %v\n", - len(peerAddresses), - len(peerOperatorPublicKeys), - ) - } -} - -func TestExtractPeersPublicKeys_CorrectPeerAddresses(t *testing.T) { - peerAddresses := []string{ - "/ip4/127.0.0.1/tcp/3919/ipfs/" + - "16Uiu2HAmNpUbaz8UptSL1aWTNnR1GmcV6Pw1kSV5xkep3N44zi3m", - "/ip4/127.0.0.1/tcp/3920/ipfs/" + - "16Uiu2HAmQA19uJUtvMp7ZGCED7maXjQZCpdkLnEGCmxPRJRCvwJt", - "/ip4/127.0.0.1/tcp/3921/ipfs/" + - "16Uiu2HAm5N75v5gmMiSaR422q6RH2QfPxWVJkRjySonG3UbnmnnQ", - } - - peerOperatorPublicKeys, err := ExtractPeersPublicKeys(peerAddresses) - if err != nil { - t.Fatal(err) - } - - if len(peerOperatorPublicKeys) != len(peerAddresses) { - t.Errorf( - "unexpected peer operator public keys length\nexpected: %v\n"+ - "actual: %v\n", - len(peerAddresses), - len(peerOperatorPublicKeys), - ) - } - - // Convert to strings for easier testing - actualPeerOperatorPublicKeys := make([]string, len(peerOperatorPublicKeys)) - for i, key := range peerOperatorPublicKeys { - actualPeerOperatorPublicKeys[i] = key.String() - } - - expectedPeerOperatorPublicKeys := []string{ - "03970308f34ba0397e4a54713c126e63b8e42effcce9766d30776f24571796c39c", - "03aadf4ef0d4836404e5f06de50b05b9273e6b6b52b8c8726dae2735882d9354dd", - "0293aaeed76b0636b1c464f1c20a2f73936c175bae01a469f89c66f0e963fdc24d", - } - - if !reflect.DeepEqual( - expectedPeerOperatorPublicKeys, - actualPeerOperatorPublicKeys, - ) { - t.Errorf( - "unexpected peer operator public keys\nexpected: %v\nactual: %v\n", - expectedPeerOperatorPublicKeys, - actualPeerOperatorPublicKeys, - ) - } -} - -func TestExtractPeersPublicKeys_IncorrectPeerAddresses(t *testing.T) { - // Make the second address too short to cause an error - peerAddresses := []string{ - "/ip4/127.0.0.1/tcp/3919/ipfs/" + - "16Uiu2HAmNpUbaz8UptSL1aWTNnR1GmcV6Pw1kSV5xkep3N44zi3m", - "/ip4/127.0.0.1/tcp/3920/ipfs/" + - "16Uiu2HAmQA19uJUtvMp7ZGCED7maXjQZCpdkLnEGCmxPRJRCvwJ", - "/ip4/127.0.0.1/tcp/3921/ipfs/" + - "16Uiu2HAm5N75v5gmMiSaR422q6RH2QfPxWVJkRjySonG3UbnmnnQ", - } - - _, err := ExtractPeersPublicKeys(peerAddresses) - - expectedError := fmt.Errorf( - "failed to extract multiaddress from peer addresses: " + - "[failed to parse multiaddr \"/ip4/127.0.0.1/tcp/3920/ipfs/" + - "16Uiu2HAmQA19uJUtvMp7ZGCED7maXjQZCpdkLnEGCmxPRJRCvwJ\": " + - "invalid value " + - "\"16Uiu2HAmQA19uJUtvMp7ZGCED7maXjQZCpdkLnEGCmxPRJRCvwJ\" " + - "for protocol p2p: failed to parse p2p addr: " + - "16Uiu2HAmQA19uJUtvMp7ZGCED7maXjQZCpdkLnEGCmxPRJRCvwJ " + - "length greater than remaining number of bytes in buffer]", - ) - if !reflect.DeepEqual(expectedError, err) { - t.Errorf( - "unexpected error\nexpected: %v\nactual: %v\n", - expectedError, - err, - ) - } -} - type testMessage struct { Sender *identity Recipient *identity diff --git a/pkg/tbtc/node_test.go b/pkg/tbtc/node_test.go index bedfb30995..5a907b89b4 100644 --- a/pkg/tbtc/node_test.go +++ b/pkg/tbtc/node_test.go @@ -344,22 +344,27 @@ func TestNode_RunCoordinationLayer(t *testing.T) { if signer.wallet.publicKey.Equal(walletPublicKey) { result, ok := map[uint64]*coordinationResult{ 900: { + window: window, proposal: &mockCoordinationProposal{ActionDepositSweep}, }, // Omit window at block 1800 to make sure the layer doesn't // crash if no result is produced. 2700: { + window: window, proposal: &mockCoordinationProposal{ActionRedemption}, }, // Put some trash value to make sure coordination windows // are distributed correctly. 2705: { + window: window, proposal: &mockCoordinationProposal{ActionMovingFunds}, }, 3600: { + window: window, proposal: &mockCoordinationProposal{ActionNoop}, }, 4500: { + window: window, proposal: &mockCoordinationProposal{ActionMovedFundsSweep}, }, }[window.coordinationBlock] @@ -405,6 +410,10 @@ loop: for { select { case result := <-processedResultsChan: + if result == nil { + continue + } + processedResults = append(processedResults, result) // Once the second-last coordination window is processed, stop the @@ -425,24 +434,68 @@ loop: 3, len(processedResults), ) - testutils.AssertStringsEqual( + + resultActionsByWindow := make(map[uint64]WalletActionType, len(processedResults)) + for _, result := range processedResults { + resultActionsByWindow[result.window.coordinationBlock] = + result.proposal.ActionType() + } + + testutils.AssertIntsEqual( t, - "first result", - ActionDepositSweep.String(), - processedResults[0].proposal.ActionType().String(), + "processed coordination windows count", + 3, + len(resultActionsByWindow), ) + + firstAction, ok := resultActionsByWindow[900] + if !ok { + t.Fatal("expected coordination result for window at block 900") + } testutils.AssertStringsEqual( t, - "second result", - ActionRedemption.String(), - processedResults[1].proposal.ActionType().String(), + "result for block 900", + ActionDepositSweep.String(), + firstAction.String(), ) + + secondAction, ok := resultActionsByWindow[2700] + if !ok { + t.Fatal("expected coordination result for window at block 2700") + } testutils.AssertStringsEqual( t, - "third result", - ActionNoop.String(), - processedResults[2].proposal.ActionType().String(), + "result for block 2700", + ActionRedemption.String(), + secondAction.String(), ) + + if _, ok := resultActionsByWindow[2705]; ok { + t.Fatal("unexpected coordination result for non-window block 2705") + } + + // Result processing is asynchronous, so by the time the test cancels the + // coordination layer after the third processed result, either the 3600 + // window or the subsequent 4500 window may already be in flight. + if thirdAction, ok := resultActionsByWindow[3600]; ok { + testutils.AssertStringsEqual( + t, + "result for block 3600", + ActionNoop.String(), + thirdAction.String(), + ) + } else { + fourthAction, ok := resultActionsByWindow[4500] + if !ok { + t.Fatal("expected coordination result for block 3600 or 4500") + } + testutils.AssertStringsEqual( + t, + "result for block 4500", + ActionMovedFundsSweep.String(), + fourthAction.String(), + ) + } } type mockCoordinationProposal struct { diff --git a/pkg/tbtcpg/internal/test/marshaling.go b/pkg/tbtcpg/internal/test/marshaling.go index 2dd72dbaa0..5a1d172ee0 100644 --- a/pkg/tbtcpg/internal/test/marshaling.go +++ b/pkg/tbtcpg/internal/test/marshaling.go @@ -273,7 +273,7 @@ func (psts *ProposeSweepTestScenario) UnmarshalJSON(data []byte) error { // Unmarshal expected error if len(unmarshaled.ExpectedErr) > 0 { - psts.ExpectedErr = fmt.Errorf(unmarshaled.ExpectedErr) + psts.ExpectedErr = fmt.Errorf("%s", unmarshaled.ExpectedErr) } return nil