Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion node/pkg/watchers/solana/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
74 changes: 72 additions & 2 deletions node/pkg/watchers/solana/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand All @@ -646,14 +656,74 @@ 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))
}
})
}
}

// 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)
Expand Down