Skip to content

Commit

Permalink
look into Solana program log message to determine SPL token failure
Browse files Browse the repository at this point in the history
  • Loading branch information
ws4charlie committed Nov 28, 2024
1 parent 08ff881 commit d488904
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 13 deletions.
53 changes: 48 additions & 5 deletions pkg/contracts/solana/instruction.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package solana

import (
"fmt"
"slices"
"strings"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
43 changes: 43 additions & 0 deletions pkg/contracts/solana/instruction_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package solana_test

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -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))
})
}
}
15 changes: 8 additions & 7 deletions zetaclient/chains/solana/observer/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand All @@ -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))
}
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion zetaclient/chains/solana/observer/outbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

0 comments on commit d488904

Please sign in to comment.