From d48890491deb98fc0df46ed51894aa30d6b49f03 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Wed, 27 Nov 2024 21:25:06 -0600 Subject: [PATCH] look into Solana program log message to determine SPL token failure --- pkg/contracts/solana/instruction.go | 53 +++++++++++++++++-- pkg/contracts/solana/instruction_test.go | 43 +++++++++++++++ zetaclient/chains/solana/observer/outbound.go | 15 +++--- .../chains/solana/observer/outbound_test.go | 2 +- 4 files changed, 100 insertions(+), 13 deletions(-) diff --git a/pkg/contracts/solana/instruction.go b/pkg/contracts/solana/instruction.go index 9bd216d338..d7f3d48f90 100644 --- a/pkg/contracts/solana/instruction.go +++ b/pkg/contracts/solana/instruction.go @@ -2,6 +2,8 @@ package solana import ( "fmt" + "slices" + "strings" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" @@ -10,6 +12,15 @@ import ( "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 @@ type OutboundInstruction interface { // 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 @@ func (inst *WithdrawInstructionParams) TokenAmount() uint64 { 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 @@ func (inst *WithdrawSPLInstructionParams) TokenAmount() uint64 { 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 @@ func (inst *WhitelistInstructionParams) TokenAmount() uint64 { 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 @@ func ParseInstructionWhitelist(instruction solana.CompiledInstruction) (*Whiteli 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 +} diff --git a/pkg/contracts/solana/instruction_test.go b/pkg/contracts/solana/instruction_test.go index 4901f59476..0e9a5ca0c0 100644 --- a/pkg/contracts/solana/instruction_test.go +++ b/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)) + }) + } +} diff --git a/zetaclient/chains/solana/observer/outbound.go b/zetaclient/chains/solana/observer/outbound.go index b0131c7b77..6ebfc45d2c 100644 --- a/zetaclient/chains/solana/observer/outbound.go +++ b/zetaclient/chains/solana/observer/outbound.go @@ -158,8 +158,11 @@ func (ob *Observer) VoteOutboundIfConfirmed(ctx context.Context, cctx *crosschai // 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 @@ func (ob *Observer) CheckFinalizedTx( return nil, false } - txNonce := inst.GatewayNonce() - // recover ECDSA signer from instruction signerECDSA, err := inst.Signer() if err != nil { @@ -314,8 +315,8 @@ func (ob *Observer) CheckFinalizedTx( } // 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 @@ func ParseGatewayInstruction( 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 @@ func ParseGatewayInstruction( 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) diff --git a/zetaclient/chains/solana/observer/outbound_test.go b/zetaclient/chains/solana/observer/outbound_test.go index 699253ff3d..17c8909cdb 100644 --- a/zetaclient/chains/solana/observer/outbound_test.go +++ b/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) }) }