replace ping liveness probe (#1799)

Signed-off-by: Rado M <radkomih@gmail.com>

Author: radkomih

sdk/account_info_query_unit_test.go

@@ -146,3 +146,65 @@ func TestUnitAccountInfoQueryMock(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, HbarFromTinybar(2), cost)
 }
+
+// A paid query on a client without an operator must fail cleanly instead of panicking while
+// building the payment.
+func TestUnitAccountInfoQueryExecuteWithoutOperator(t *testing.T) {
+	t.Parallel()
+
+	client, server := NewMockClientAndServer([][]interface{}{{}})
+	defer server.Close()
+
+	client.operator = nil
+
+	// An explicit payment skips the cost lookup and goes straight to payment signing.
+	_, err := NewAccountInfoQuery().
+		SetAccountID(AccountID{Account: 1800}).
+		SetNodeAccountIDs([]AccountID{{Account: 3}}).
+		SetQueryPayment(NewHbar(1)).
+		Execute(client)
+	require.ErrorIs(t, err, errNoClientProvided)
+}
+
+// A COST_ANSWER query is unpaid, so it must go out without a payment even on a query object a
+// previous Execute already attached the client to - reaching for the absent operator there would
+// panic while building the payment.
+func TestUnitAccountInfoQueryGetCostAfterExecuteWithoutOperator(t *testing.T) {
+	t.Parallel()
+
+	var captured []*services.Query
+	client, server := NewMockClientAndServer([][]interface{}{{_CostAnswer(services.ResponseCodeEnum_OK, &captured)}})
+	defer server.Close()
+
+	client.operator = nil
+
+	query := NewAccountInfoQuery().
+		SetAccountID(AccountID{Account: 1800}).
+		SetNodeAccountIDs([]AccountID{{Account: 3}})
+
+	// Fails for want of an operator, but leaves the client on the query.
+	_, err := query.Execute(client)
+	require.ErrorIs(t, err, errNoClientProvided)
+	require.NotNil(t, query.client)
+
+	cost, err := query.GetCost(client)
+	require.NoError(t, err)
+	require.Equal(t, HbarFromTinybar(25), cost)
+
+	// The failed Execute must not have reached the network, so this is the cost lookup.
+	require.Len(t, captured, 1)
+	info, ok := captured[0].Query.(*services.Query_CryptoGetInfo)
+	require.True(t, ok)
+	require.Equal(t, services.ResponseType_COST_ANSWER, info.CryptoGetInfo.Header.ResponseType)
+	require.Nil(t, info.CryptoGetInfo.Header.Payment, "a COST_ANSWER query must not attach a payment transaction")
+}
+
+// GetCost must fail cleanly without a client.
+func TestUnitAccountInfoQueryGetCostNilClient(t *testing.T) {
+	t.Parallel()
+
+	_, err := NewAccountInfoQuery().
+		SetAccountID(AccountID{Account: 1800}).
+		GetCost(nil)
+	require.ErrorIs(t, err, errNoClientProvided)
+}

sdk/client.go

@@ -749,15 +749,22 @@ func (client *Client) GetOperatorPublicKey() PublicKey {
 	return PublicKey{}
 }
 
-// Ping sends an AccountBalanceQuery to the specified _Node returning nil if no
-// problems occur. Otherwise, an error representing the status of the _Node will
-// be returned.
+// treasuryAccountNum is the treasury system account, present on every network from genesis.
+const treasuryAccountNum = 2
+
+// Ping checks the liveness of the given consensus node, returning nil if the node is reachable.
+//
+// It sends a COST_ANSWER getAccountInfo query for the treasury account, which a healthy node answers
+// with the query fee without executing it: nothing is charged and no operator is required.
+//
+// The probe follows the client's retry settings. A node already in backoff is never contacted, so
+// Ping fails without reaching the network; readmission happens when the backoff elapses, not on a
+// successful Ping.
 func (client *Client) Ping(nodeID AccountID) error {
-	_, err := NewAccountBalanceQuery().
+	_, err := NewAccountInfoQuery().
+		SetAccountID(AccountID{Account: treasuryAccountNum}).
 		SetNodeAccountIDs([]AccountID{nodeID}).
-		SetAccountID(client.GetOperatorAccountID()).
-		Execute(client)
-
+		GetCost(client)
 	return err
 }
 

sdk/client_e2e_test.go

@@ -162,3 +162,52 @@ func TestClientIntegrationForMirrorNetworkWithShardAndRealm(t *testing.T) {
 	require.Nil(t, client)
 	assert.Contains(t, err.Error(), "failed to query address book: no healthy nodes")
 }
+
+// Every node of a live network answers the probe, and PingAll probes every node.
+func TestIntegrationClientPingAllNetworkNodes(t *testing.T) { // nolint
+	t.Parallel()
+	env := NewIntegrationTestEnv(t)
+	defer CloseIntegrationTestEnv(env, nil)
+
+	for address, nodeID := range env.Client.GetNetwork() {
+		require.NoError(t, env.Client.Ping(nodeID), "node %s at %s should be reachable", nodeID, address)
+	}
+
+	// PingAll swallows errors, so a rising use count is the proof it probed - valid only if healthy first.
+	usesBefore := make(map[AccountID]int64)
+	for address, nodeID := range env.Client.GetNetwork() {
+		node, ok := env.Client.network._GetNodeForAccountID(nodeID)
+		require.True(t, ok)
+		require.True(t, node._IsHealthy(), "node %s at %s should be healthy before PingAll", nodeID, address)
+		usesBefore[nodeID] = node._GetUseCount()
+	}
+
+	env.Client.PingAll()
+	for address, nodeID := range env.Client.GetNetwork() {
+		node, ok := env.Client.network._GetNodeForAccountID(nodeID)
+		require.True(t, ok)
+		assert.Greater(t, node._GetUseCount(), usesBefore[nodeID],
+			"PingAll must probe node %s at %s", nodeID, address)
+		// Only transport failures back a node off, so this catches unreachability, not every failure.
+		assert.True(t, node._IsHealthy(), "node %s at %s should stay healthy after PingAll", nodeID, address)
+	}
+}
+
+// The probe works against a live node with no operator configured: a COST_ANSWER query is free and
+// unsigned, so it attaches no payment.
+func TestIntegrationClientPingWithoutOperator(t *testing.T) { // nolint
+	t.Parallel()
+	env := NewIntegrationTestEnv(t)
+	defer CloseIntegrationTestEnv(env, nil)
+
+	client, err := ClientForNetworkV2(env.Client.GetNetwork())
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, client.Close())
+	}()
+	require.Nil(t, client.operator)
+
+	for _, nodeID := range client.GetNetwork() {
+		require.NoError(t, client.Ping(nodeID))
+	}
+}