From 96c5c3f5114ab9f643668577826179ea5291480e Mon Sep 17 00:00:00 2001 From: Doruk Tan Ozturk Date: Wed, 8 Jul 2026 11:09:27 +0200 Subject: [PATCH] node/solana: base58-decode websocket account pubkey The Pythnet account-subscription websocket path built the account PublicKey with solana.PublicKeyFromBytes([]byte(value.Pubkey)), but value.Pubkey is a base58-encoded string. Casting the string to a byte slice reinterprets the base58 text as raw key bytes, producing an incorrect PublicKey. This key is used as the observation TxID on the account-based path, so the effect is a wrong/garbled TxID in emitted observations and logs. It does not affect the VAA digest (derived from emitter, sequence and payload), so this is a correctness/observability fix, not a consensus or security change. Decode the base58 string with solana.PublicKeyFromBase58 and surface any decode error, matching how pubkey strings are parsed elsewhere in the watcher. Add table-driven coverage that decodes real base58 account addresses, asserts the decoded key round-trips to its address while the old byte cast yields a different key, and drives processAccountSubscriptionData end to end so the emitted TxID matches the decoded pubkey and a non-base58 pubkey surfaces an error. --- node/pkg/watchers/solana/client.go | 9 ++- node/pkg/watchers/solana/client_test.go | 74 ++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/node/pkg/watchers/solana/client.go b/node/pkg/watchers/solana/client.go index 707f500391a..717489b29b6 100644 --- a/node/pkg/watchers/solana/client.go +++ b/node/pkg/watchers/solana/client.go @@ -1154,7 +1154,14 @@ func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, data [ return nil } - acc := solana.PublicKeyFromBytes([]byte(value.Pubkey)) + // value.Pubkey is a base58-encoded string, so it must be decoded rather than + // reinterpreted as raw bytes. + acc, pubkeyErr := solana.PublicKeyFromBase58(value.Pubkey) + if pubkeyErr != nil { + s.logger.Error("failed to decode account pubkey", zap.String("account", value.Pubkey), zap.Error(pubkeyErr)) + p2p.DefaultRegistry.AddErrorCount(s.chainID, 1) + return pubkeyErr + } // NOTE: We don't care about the number of observations here so the return value is ignored. // The called function will still publish observations if it is successful. s.processMessageAccount(s.logger, messageAccountData, acc, isReobservation, solana.Signature{}, false) diff --git a/node/pkg/watchers/solana/client_test.go b/node/pkg/watchers/solana/client_test.go index ef30d58ef7b..946c40f547a 100644 --- a/node/pkg/watchers/solana/client_test.go +++ b/node/pkg/watchers/solana/client_test.go @@ -550,7 +550,10 @@ func TestProcessMessageAccount(t *testing.T) { func TestProcessAccountSubscriptionData(t *testing.T) { // Scenario: subscription messages are validated and decoded; invalid input yields errors or no-ops. rawContract := "worm2ZoG2kUd4vFXhvjh93UUH596ayRfgQ2MgjNMTth" - pubkey := "01234567890123456789012345678901" + // pubkey is a real base58-encoded Solana account address (the SPL Token program). It must be + // base58-decoded to reach the correct account key that becomes the observation TxID. + pubkey := "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA" + wantTxID := solana.MustPublicKeyFromBase58(pubkey).Bytes() proposal := testMessagePublicationAccount([]byte("hello"), 32) validAccountData := encodeMessagePublicationAccount(t, accountPrefixReliable, proposal) @@ -625,6 +628,13 @@ func TestProcessAccountSubscriptionData(t *testing.T) { name: "truncated_data", data: buildSubscriptionPayload(t, rawContract, pubkey, []byte{0x01, 0x02}), }, + { + // A pubkey that is not valid base58 must surface a decode error rather than + // silently reinterpreting the string as raw key bytes. + name: "invalid_pubkey", + data: buildSubscriptionPayload(t, rawContract, "01234567890123456789012345678901", validAccountData), + wantErr: true, + }, { name: "valid_message", data: buildSubscriptionPayload(t, rawContract, pubkey, validAccountData), @@ -646,7 +656,11 @@ func TestProcessAccountSubscriptionData(t *testing.T) { require.NoError(t, err) if tc.wantMsg { require.Equal(t, 1, len(msgC)) - <-msgC + msg := <-msgC + require.NotNil(t, msg) + // On the account path the observation TxID is the account key. It must match the + // base58-decoded pubkey, not the raw bytes of the base58 string. + assert.Equal(t, wantTxID, msg.TxID) } else { assert.Equal(t, 0, len(msgC)) } @@ -654,6 +668,62 @@ func TestProcessAccountSubscriptionData(t *testing.T) { } } +// TestAccountSubscriptionPubkeyDecode proves that the account pubkey delivered on the +// account-subscription websocket path is a base58-encoded string that must be decoded with +// solana.PublicKeyFromBase58. Casting the string to a byte slice with +// solana.PublicKeyFromBytes([]byte(pubkey)) reinterprets the base58 text as raw key bytes and +// yields a different, incorrect key. That key becomes the observation TxID on the account path, +// so the miscast produces a garbled TxID. +func TestAccountSubscriptionPubkeyDecode(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pubkey string + wantErr bool + }{ + { + // SPL Token program address, a real on-chain Solana account. + name: "token_program", + pubkey: "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA", + }, + { + // Wormhole SVM core bridge address. + name: "core_bridge", + pubkey: "worm2ZoG2kUd4vFXhvjh93UUH596ayRfgQ2MgjNMTth", + }, + { + // '0' is not part of the base58 alphabet, so decoding must fail rather than + // silently succeeding as the byte-cast path did. + name: "not_base58", + pubkey: "01234567890123456789012345678901", + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + decoded, err := solana.PublicKeyFromBase58(tc.pubkey) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + // Re-encoding the decoded key reproduces the original base58 string, proving it + // decoded to the account the string actually names. + assert.Equal(t, tc.pubkey, decoded.String()) + + // The previous implementation cast the base58 string to bytes instead of decoding it. + // That path yields a different key, which is the bug this fix addresses. + miscast := solana.PublicKeyFromBytes([]byte(tc.pubkey)) + assert.NotEqual(t, decoded, miscast) + }) + } +} + func TestProcessInstructionEarlyReturns(t *testing.T) { // Scenario: instruction filtering should return early for non-matching cases. commitmentMismatchData := encodePostMessageData(t, 42, []byte("hi"), consistencyLevelConfirmed)