fix: look into Solana program log message to determine SPL token withdrawal failure

changelog.md

@@ -14,6 +14,7 @@
 
 * [3206](https://github.com/zeta-chain/node/pull/3206) - skip Solana unsupported transaction version to not block inbound observation
 * [3184](https://github.com/zeta-chain/node/pull/3184) - zetaclient should not retry if inbound vote message validation fails
+* [3231](https://github.com/zeta-chain/node/pull/3231) - zetaclient look into solana program logs to determine SPL token withdrawal failure
 
 ## v23.0.0
 

pkg/contracts/solana/instruction.go

@@ -2,6 +2,8 @@
 
 import (
 	"fmt"
+	"slices"
+	"strings"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
@@ -10,6 +12,15 @@
 	"github.com/pkg/errors"
 )
 
+const (
+	// MsgWithdrawSPLTokenSuccess is the success message for withdraw_spl_token instruction
+	// #nosec G101 not a hardcoded credential
+	MsgWithdrawSPLTokenSuccess = "withdraw spl token successfully"
+
+	// MsgWithdrawSPLTokenNonExistentAta is the log message printed when recipient ATA does not exist
+	MsgWithdrawSPLTokenNonExistentAta = "recipient ATA account does not exist"
+)
+
 // InitializeParams contains the parameters for a gateway initialize instruction
 type InitializeParams struct {
 	// Discriminator is the unique identifier for the initialize instruction
@@ -62,6 +73,9 @@
 
 	// TokenAmount returns the amount of the instruction
 	TokenAmount() uint64
+
+	// Failed returns true if the instruction logs indicate failure
+	Failed(logMessages []string) bool
 }
 
 var _ OutboundInstruction = (*WithdrawInstructionParams)(nil)
@@ -106,6 +120,11 @@
 	return inst.Amount
 }
 
+// Failed always returns false for a 'withdraw' without checking the logs
+func (inst *WithdrawInstructionParams) Failed(_ []string) bool {
+	return false
+}
+
 // ParseInstructionWithdraw tries to parse the instruction as a 'withdraw'.
 // It returns nil if the instruction can't be parsed as a 'withdraw'.
 func ParseInstructionWithdraw(instruction solana.CompiledInstruction) (*WithdrawInstructionParams, error) {
@@ -166,19 +185,31 @@
 	return inst.Amount
 }
 
-// ParseInstructionWithdraw tries to parse the instruction as a 'withdraw'.
-// It returns nil if the instruction can't be parsed as a 'withdraw'.
+// Failed returns true if the logs of the 'withdraw_spl_token' instruction indicate failure.
+//
+// Note: SPL token transfer cannot be done if the recipient ATA does not exist.
+func (inst *WithdrawSPLInstructionParams) Failed(logMessages []string) bool {
+	// Assumption: only one of the two messages will be present in the logs.
+	// If both messages are present, it could imply a program bug or a malicious attack.
+	// In such case, the function treats the transaction as successful to minimize the attack surface,
+	// bacause a fabricated failure could be used to trick zetacore into refunding the withdrawer (if implemented in the future).
+	return !containsLogMessage(logMessages, MsgWithdrawSPLTokenSuccess) &&
+		containsLogMessage(logMessages, MsgWithdrawSPLTokenNonExistentAta)
+}
+
+// ParseInstructionWithdrawSPL tries to parse the instruction as a 'withdraw_spl_token'.
+// It returns nil if the instruction can't be parsed as a 'withdraw_spl_token'.
 func ParseInstructionWithdrawSPL(instruction solana.CompiledInstruction) (*WithdrawSPLInstructionParams, error) {
-	// try deserializing instruction as a 'withdraw'
+	// try deserializing instruction as a 'withdraw_spl_token'
 	inst := &WithdrawSPLInstructionParams{}
 	err := borsh.Deserialize(inst, instruction.Data)
 	if err != nil {
 		return nil, errors.Wrap(err, "error deserializing instruction")
 	}
 
-	// check the discriminator to ensure it's a 'withdraw' instruction
+	// check the discriminator to ensure it's a 'withdraw_spl_token' instruction
 	if inst.Discriminator != DiscriminatorWithdrawSPL {
-		return nil, fmt.Errorf("not a withdraw instruction: %v", inst.Discriminator)
+		return nil, fmt.Errorf("not a withdraw_spl_token instruction: %v", inst.Discriminator)
 	}
 
 	return inst, nil
@@ -234,6 +265,11 @@
 	return 0
 }
 
+// Failed always returns false for a 'whitelist_spl_mint' without checking the logs
+func (inst *WhitelistInstructionParams) Failed(_ []string) bool {
+	return true
+}
+
 // ParseInstructionWhitelist tries to parse the instruction as a 'whitelist_spl_mint'.
 // It returns nil if the instruction can't be parsed as a 'whitelist_spl_mint'.
 func ParseInstructionWhitelist(instruction solana.CompiledInstruction) (*WhitelistInstructionParams, error) {
@@ -251,3 +287,10 @@
 
 	return inst, nil
 }
+
+// containsLogMessage returns true if any of the log messages contains the 'msgSearch'
+func containsLogMessage(logMessages []string, msgSearch string) bool {
+	return slices.IndexFunc(logMessages, func(msg string) bool {
+		return strings.Contains(msg, msgSearch)
+	}) != -1
+}

pkg/contracts/solana/instruction_test.go

@@ -1,6 +1,7 @@
 package solana_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -78,3 +79,45 @@ func Test_RecoverSigner(t *testing.T) {
 	require.NotEqual(t, ethcommon.Address{}, signer)
 	require.NotEqual(t, testSigner, signer.String())
 }
+
+func Test_WithdrawSPLInstructionParams_Failed(t *testing.T) {
+	tests := []struct {
+		name        string
+		logMessages []string
+		want        bool
+	}{
+		{
+			name: "failed - only non-existent ATA account message found",
+			logMessages: []string{
+				"Program log: Instruction: WithdrawSPLToken",
+				fmt.Sprintf("Program log: %s", contracts.MsgWithdrawSPLTokenNonExistentAta),
+			},
+			want: true,
+		},
+		{
+			name: "succeeded - only success message found",
+			logMessages: []string{
+				"Program log: Instruction: WithdrawSPLToken",
+				fmt.Sprintf("Program log: %s", contracts.MsgWithdrawSPLTokenSuccess),
+			},
+			want: false,
+		},
+		{
+			// This case should NEVER happen by design of the gateway contract.
+			name: "succeeded - found both success message and non-existent ATA account message",
+			logMessages: []string{
+				"Program log: Instruction: WithdrawSPLToken",
+				fmt.Sprintf("Program log: %s", contracts.MsgWithdrawSPLTokenSuccess),
+				fmt.Sprintf("Program log: %s", contracts.MsgWithdrawSPLTokenNonExistentAta),
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			inst := contracts.WithdrawSPLInstructionParams{}
+			require.Equal(t, tt.want, inst.Failed(tt.logMessages))
+		})
+	}
+}

zetaclient/chains/solana/observer/outbound.go

@@ -158,8 +158,11 @@
 	// the amount and status of the outbound
 	outboundAmount := new(big.Int).SetUint64(inst.TokenAmount())
 
-	// status was already verified as successful in CheckFinalizedTx
+	// look into the log messages to determine the status of the outbound
 	outboundStatus := chains.ReceiveStatus_success
+	if inst.Failed(txResult.Meta.LogMessages) {
+		outboundStatus = chains.ReceiveStatus_failed
+	}
 
 	// compliance check, special handling the cancelled cctx
 	if compliance.IsCctxRestricted(cctx) {
@@ -297,8 +300,6 @@
 		return nil, false
 	}
 
-	txNonce := inst.GatewayNonce()
-
 	// recover ECDSA signer from instruction
 	signerECDSA, err := inst.Signer()
 	if err != nil {
@@ -314,8 +315,8 @@
 	}
 
 	// check tx nonce
-	if txNonce != nonce {
-		logger.Error().Msgf("tx nonce %d is not matching tracker nonce", txNonce)
+	if inst.GatewayNonce() != nonce {
+		logger.Error().Msgf("tx nonce %d is not matching tracker nonce", inst.GatewayNonce())
 		return nil, false
 	}
 
@@ -334,7 +335,7 @@
 		return nil, errors.Wrap(err, "error unmarshaling transaction")
 	}
 
-	// there should be only one single instruction ('withdraw' or 'withdraw_spl_token')
+	// there should be only one single instruction ('withdraw' or 'withdraw_spl_token' or 'whitelist_spl_mint')
 	if len(tx.Message.Instructions) != 1 {
 		return nil, fmt.Errorf("want 1 instruction, got %d", len(tx.Message.Instructions))
 	}
@@ -351,7 +352,7 @@
 		return nil, fmt.Errorf("programID %s is not matching gatewayID %s", programID, gatewayID)
 	}
 
-	// parse the instruction as a 'withdraw' or 'withdraw_spl_token'
+	// parse the instruction as a 'withdraw' or 'withdraw_spl_token' or 'whitelist_spl_mint'
 	switch coinType {
 	case coin.CoinType_Gas:
 		return contracts.ParseInstructionWithdraw(instruction)

zetaclient/chains/solana/observer/outbound_test.go

@@ -507,7 +507,7 @@ func Test_ParseInstructionWithdrawSPL(t *testing.T) {
 		inst, err := contracts.ParseInstructionWithdrawSPL(instruction)
 
 		// ASSERT
-		require.ErrorContains(t, err, "not a withdraw instruction")
+		require.ErrorContains(t, err, "not a withdraw_spl_token instruction")
 		require.Nil(t, inst)
 	})
 }