Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid endless tx rescan when the inbound vote message is invalid #3184

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4653b46
avoid endless rescan when the inbound vote message is invalid
ws4charlie Nov 19, 2024
1e1e254
add changelog entry
ws4charlie Nov 19, 2024
93fefe7
Merge branch 'develop' into avoid-btc-solana-inbound-endless-rescan-o…
ws4charlie Nov 19, 2024
5e4e6d7
rename CheckEventProcessability as IsEventProcessable
ws4charlie Nov 20, 2024
426547b
rename InboundProcessability as InboundCategory
ws4charlie Nov 20, 2024
a3a09c8
remove btc inbound duplidate log fields
ws4charlie Nov 20, 2024
b003410
remove duplicate log fields; add some function comments to improve re…
ws4charlie Nov 20, 2024
8954ecd
Merge branch 'develop' of https://github.com/zeta-chain/node into avo…
ws4charlie Nov 20, 2024
6c12632
Merge branch 'develop' into avoid-btc-solana-inbound-endless-rescan-o…
ws4charlie Nov 21, 2024
c8b50eb
move ValidateBasic checking right before posting the vote msg
ws4charlie Nov 22, 2024
33fd7d3
Merge branch 'develop' of https://github.com/zeta-chain/node into avo…
ws4charlie Nov 22, 2024
2cbe254
cleanup changelog
ws4charlie Nov 22, 2024
a7688f6
fix unit test
ws4charlie Nov 22, 2024
a83ba03
Merge branch 'develop' into avoid-btc-solana-inbound-endless-rescan-o…
ws4charlie Nov 22, 2024
c933432
Merge branch 'develop' into avoid-btc-solana-inbound-endless-rescan-o…
ws4charlie Nov 22, 2024
97ce52a
added InboundCategoryUnknown; logs fields cleaning; renaming
ws4charlie Nov 25, 2024
d723e09
Merge branch 'develop' of https://github.com/zeta-chain/node into avo…
ws4charlie Nov 25, 2024
82a08fa
remove uncessary log print
ws4charlie Nov 25, 2024
5644b3c
wrap a few log prints into fields
ws4charlie Nov 25, 2024
74778b0
Merge branch 'develop' into avoid-btc-solana-inbound-endless-rescan-o…
ws4charlie Nov 25, 2024
5f46375
move zeta tx hash to log field
ws4charlie Nov 25, 2024
fe26d13
Merge branch 'develop' of https://github.com/zeta-chain/node into avo…
ws4charlie Nov 25, 2024
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
12 changes: 12 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# CHANGELOG

## Unreleased

### Features

### Tests

### Refactor

### Fixes

* [3184](https://github.com/zeta-chain/node/pull/3184) - zetaclient should not retry if inbound vote message validation fails

## v23.0.0

### Features
Expand Down
2 changes: 1 addition & 1 deletion testutil/sample/crosschain.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ func ZetaAccounting(t *testing.T, index string) types.ZetaAccounting {

func InboundVote(coinType coin.CoinType, from, to int64) types.MsgVoteInbound {
return types.MsgVoteInbound{
Creator: "",
Creator: Bech32AccAddress().String(),
Sender: EthAddress().String(),
SenderChainId: Chain(from).ChainId,
Receiver: EthAddress().String(),
Expand Down
2 changes: 1 addition & 1 deletion x/crosschain/types/message_vote_inbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const MaxMessageLength = 10240
// InboundVoteOption is a function that sets some option on the inbound vote message
type InboundVoteOption func(*MsgVoteInbound)

// WithMemoRevertOptions sets the revert options for inbound vote message
// WithRevertOptions sets the revert options for inbound vote message
func WithRevertOptions(revertOptions RevertOptions) InboundVoteOption {
return func(msg *MsgVoteInbound) {
msg.RevertOptions = revertOptions
Expand Down
25 changes: 16 additions & 9 deletions zetaclient/chains/base/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ func (ob *Observer) ReadLastTxScannedFromDB() (string, error) {
return lastTx.Hash, nil
}

// PostVoteInbound posts a vote for the given vote message
// PostVoteInbound posts a vote for the given vote message and returns the ballot.
func (ob *Observer) PostVoteInbound(
ctx context.Context,
msg *crosschaintypes.MsgVoteInbound,
Expand All @@ -477,19 +477,26 @@ func (ob *Observer) PostVoteInbound(
var (
txHash = msg.InboundHash
coinType = msg.CoinType
chainID = ob.Chain().ChainId
)

zetaHash, ballot, err := ob.ZetacoreClient().PostVoteInbound(ctx, gasLimit, retryGasLimit, msg)

// prepare logger fields
lf := map[string]any{
"inbound.chain_id": chainID,
"inbound.coin_type": coinType.String(),
"inbound.external_tx_hash": txHash,
"inbound.ballot_index": ballot,
"inbound.zeta_tx_hash": zetaHash,
logs.FieldMethod: "PostVoteInbound",
logs.FieldTx: txHash,
logs.FieldCoinType: coinType.String(),
}

// make sure the message is valid to avoid unnecessary retries
if err := msg.ValidateBasic(); err != nil {
ob.logger.Inbound.Warn().Err(err).Fields(lf).Msg("invalid inbound vote message")
return "", nil
}

// post vote to zetacore
zetaHash, ballot, err := ob.ZetacoreClient().PostVoteInbound(ctx, gasLimit, retryGasLimit, msg)
lf[logs.FieldZetaTx] = zetaHash
lf[logs.FieldBallot] = ballot

switch {
case err != nil:
ob.logger.Inbound.Error().Err(err).Fields(lf).Msg("inbound detected: error posting vote")
Expand Down
20 changes: 20 additions & 0 deletions zetaclient/chains/base/observer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"os"
"strings"
"testing"
"time"

Expand All @@ -16,6 +17,7 @@ import (
"github.com/zeta-chain/node/pkg/chains"
"github.com/zeta-chain/node/pkg/coin"
"github.com/zeta-chain/node/testutil/sample"
crosschaintypes "github.com/zeta-chain/node/x/crosschain/types"
observertypes "github.com/zeta-chain/node/x/observer/types"
"github.com/zeta-chain/node/zetaclient/chains/base"
"github.com/zeta-chain/node/zetaclient/chains/interfaces"
Expand Down Expand Up @@ -626,6 +628,24 @@ func TestPostVoteInbound(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "sampleBallotIndex", ballot)
})

t.Run("should not post vote if message basic validation fails", func(t *testing.T) {
// create observer
ob := createObserver(t, chains.Ethereum, defaultAlertLatency)

// create mock zetacore client
zetacoreClient := mocks.NewZetacoreClient(t)
ob = ob.WithZetacoreClient(zetacoreClient)

// create sample message with long Message
msg := sample.InboundVote(coin.CoinType_Gas, chains.Ethereum.ChainId, chains.ZetaChainMainnet.ChainId)
msg.Message = strings.Repeat("1", crosschaintypes.MaxMessageLength+1)

// post vote inbound
ballot, err := ob.PostVoteInbound(context.TODO(), &msg, 100000)
require.NoError(t, err)
require.Empty(t, ballot)
})
}

func TestAlertOnRPCLatency(t *testing.T) {
Expand Down
48 changes: 16 additions & 32 deletions zetaclient/chains/bitcoin/observer/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,7 @@
"github.com/zeta-chain/node/zetaclient/compliance"
"github.com/zeta-chain/node/zetaclient/config"
"github.com/zeta-chain/node/zetaclient/logs"
)

// InboundProcessability is an enum representing the processability of an inbound
type InboundProcessability int

const (
// InboundProcessabilityGood represents a processable inbound
InboundProcessabilityGood InboundProcessability = iota

// InboundProcessabilityDonation represents a donation inbound
InboundProcessabilityDonation

// InboundProcessabilityComplianceViolation represents a compliance violation
InboundProcessabilityComplianceViolation
clienttypes "github.com/zeta-chain/node/zetaclient/types"
)

// BTCInboundEvent represents an incoming transaction event
Expand Down Expand Up @@ -62,11 +49,11 @@
TxHash string
}

// Processability returns the processability of the inbound event
func (event *BTCInboundEvent) Processability() InboundProcessability {
// Category returns the category of the inbound event
func (event *BTCInboundEvent) Category() clienttypes.InboundCategory {
// compliance check on sender and receiver addresses
if config.ContainRestrictedAddress(event.FromAddress, event.ToAddress) {
return InboundProcessabilityComplianceViolation
return clienttypes.InboundCategoryRestricted
}

// compliance check on receiver, revert/abort addresses in standard memo
Expand All @@ -76,16 +63,16 @@
event.MemoStd.RevertOptions.RevertAddress,
event.MemoStd.RevertOptions.AbortAddress,
) {
return InboundProcessabilityComplianceViolation
return clienttypes.InboundCategoryRestricted
}
}

// donation check
if bytes.Equal(event.MemoBytes, []byte(constant.DonationMessage)) {
return InboundProcessabilityDonation
return clienttypes.InboundCategoryDonation
}

return InboundProcessabilityGood
return clienttypes.InboundCategoryGood
}

// DecodeMemoBytes decodes the contained memo bytes as either standard or legacy memo
Expand Down Expand Up @@ -164,25 +151,22 @@
return nil
}

// CheckEventProcessability checks if the inbound event is processable
func (ob *Observer) CheckEventProcessability(event BTCInboundEvent) bool {
// check if the event is processable
switch result := event.Processability(); result {
case InboundProcessabilityGood:
// IsEventProcessable checks if the inbound event is processable
func (ob *Observer) IsEventProcessable(event BTCInboundEvent) bool {
logFields := map[string]any{logs.FieldTx: event.TxHash}

switch category := event.Category(); category {
case clienttypes.InboundCategoryGood:
return true
case InboundProcessabilityDonation:
logFields := map[string]any{
logs.FieldChain: ob.Chain().ChainId,
logs.FieldTx: event.TxHash,
}
case clienttypes.InboundCategoryDonation:
ob.Logger().Inbound.Info().Fields(logFields).Msgf("thank you rich folk for your donation!")
return false
case InboundProcessabilityComplianceViolation:
case clienttypes.InboundCategoryRestricted:
compliance.PrintComplianceLog(ob.logger.Inbound, ob.logger.Compliance,
false, ob.Chain().ChainId, event.TxHash, event.FromAddress, event.ToAddress, "BTC")
return false
default:
ob.Logger().Inbound.Error().Msgf("unreachable code got InboundProcessability: %v", result)
ob.Logger().Inbound.Error().Fields(logFields).Msgf("unreachable code got InboundCategory: %v", category)

Check warning on line 169 in zetaclient/chains/bitcoin/observer/event.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/event.go#L169

Added line #L169 was not covered by tests
return false
}
}
Expand Down
31 changes: 16 additions & 15 deletions zetaclient/chains/bitcoin/observer/event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/zeta-chain/node/zetaclient/keys"
"github.com/zeta-chain/node/zetaclient/testutils"
"github.com/zeta-chain/node/zetaclient/testutils/mocks"
clienttypes "github.com/zeta-chain/node/zetaclient/types"
)

// createTestBtcEvent creates a test BTC inbound event
Expand All @@ -41,7 +42,7 @@ func createTestBtcEvent(
}
}

func Test_CheckProcessability(t *testing.T) {
func Test_Category(t *testing.T) {
// setup compliance config
cfg := config.Config{
ComplianceConfig: sample.ComplianceConfig(),
Expand All @@ -52,26 +53,26 @@ func Test_CheckProcessability(t *testing.T) {
tests := []struct {
name string
event *observer.BTCInboundEvent
expected observer.InboundProcessability
expected clienttypes.InboundCategory
}{
{
name: "should return InboundProcessabilityGood for a processable inbound event",
name: "should return InboundCategoryGood for a processable inbound event",
event: &observer.BTCInboundEvent{
FromAddress: "tb1quhassyrlj43qar0mn0k5sufyp6mazmh2q85lr6ex8ehqfhxpzsksllwrsu",
ToAddress: testutils.TSSAddressBTCAthens3,
},
expected: observer.InboundProcessabilityGood,
expected: clienttypes.InboundCategoryGood,
},
{
name: "should return InboundProcessabilityComplianceViolation for a restricted sender address",
name: "should return InboundCategoryRestricted for a restricted sender address",
event: &observer.BTCInboundEvent{
FromAddress: sample.RestrictedBtcAddressTest,
ToAddress: testutils.TSSAddressBTCAthens3,
},
expected: observer.InboundProcessabilityComplianceViolation,
expected: clienttypes.InboundCategoryRestricted,
},
{
name: "should return InboundProcessabilityComplianceViolation for a restricted receiver address in standard memo",
name: "should return InboundCategoryRestricted for a restricted receiver address in standard memo",
event: &observer.BTCInboundEvent{
FromAddress: "tb1quhassyrlj43qar0mn0k5sufyp6mazmh2q85lr6ex8ehqfhxpzsksllwrsu",
ToAddress: testutils.TSSAddressBTCAthens3,
Expand All @@ -81,10 +82,10 @@ func Test_CheckProcessability(t *testing.T) {
},
},
},
expected: observer.InboundProcessabilityComplianceViolation,
expected: clienttypes.InboundCategoryRestricted,
},
{
name: "should return InboundProcessabilityComplianceViolation for a restricted revert address in standard memo",
name: "should return InboundCategoryRestricted for a restricted revert address in standard memo",
event: &observer.BTCInboundEvent{
FromAddress: "tb1quhassyrlj43qar0mn0k5sufyp6mazmh2q85lr6ex8ehqfhxpzsksllwrsu",
ToAddress: testutils.TSSAddressBTCAthens3,
Expand All @@ -96,22 +97,22 @@ func Test_CheckProcessability(t *testing.T) {
},
},
},
expected: observer.InboundProcessabilityComplianceViolation,
expected: clienttypes.InboundCategoryRestricted,
},
{
name: "should return InboundProcessabilityDonation for a donation inbound event",
name: "should return InboundCategoryDonation for a donation inbound event",
event: &observer.BTCInboundEvent{
FromAddress: "tb1quhassyrlj43qar0mn0k5sufyp6mazmh2q85lr6ex8ehqfhxpzsksllwrsu",
ToAddress: testutils.TSSAddressBTCAthens3,
MemoBytes: []byte(constant.DonationMessage),
},
expected: observer.InboundProcessabilityDonation,
expected: clienttypes.InboundCategoryDonation,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := tt.event.Processability()
result := tt.event.Category()
require.Equal(t, tt.expected, result)
})
}
Expand Down Expand Up @@ -301,7 +302,7 @@ func Test_ValidateStandardMemo(t *testing.T) {
}
}

func Test_CheckEventProcessability(t *testing.T) {
func Test_IsEventProcessable(t *testing.T) {
// can use any bitcoin chain for testing
chain := chains.BitcoinMainnet
params := mocks.MockChainParams(chain.ChainId, 10)
Expand Down Expand Up @@ -344,7 +345,7 @@ func Test_CheckEventProcessability(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := ob.CheckEventProcessability(tt.event)
result := ob.IsEventProcessable(tt.event)
require.Equal(t, tt.result, result)
})
}
Expand Down
25 changes: 7 additions & 18 deletions zetaclient/chains/bitcoin/observer/inbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,21 +282,7 @@
return msg.Digest(), nil
}

zetaHash, ballot, err := ob.ZetacoreClient().PostVoteInbound(
ctx,
zetacore.PostVoteInboundGasLimit,
zetacore.PostVoteInboundExecutionGasLimit,
msg,
)
if err != nil {
ob.logger.Inbound.Error().Err(err).Msg("error posting to zetacore")
return "", err
} else if zetaHash != "" {
ob.logger.Inbound.Info().Msgf("BTC deposit detected and reported: PostVoteInbound zeta tx hash: %s inbound %s ballot %s fee %v",
zetaHash, txHash, ballot, event.DepositorFee)
}

return msg.Digest(), nil
return ob.PostVoteInbound(ctx, msg, zetacore.PostVoteInboundExecutionGasLimit)

Check warning on line 285 in zetaclient/chains/bitcoin/observer/inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/inbound.go#L285

Added line #L285 was not covered by tests
}

// FilterAndParseIncomingTx given txs list returned by the "getblock 2" RPC command, return the txs that are relevant to us
Expand Down Expand Up @@ -332,12 +318,15 @@
}

// GetInboundVoteFromBtcEvent converts a BTCInboundEvent to a MsgVoteInbound to enable voting on the inbound on zetacore
//
// Returns:
// - a valid MsgVoteInbound message, or
// - nil if no valid message can be created for whatever reasons:
// invalid data, not processable, invalid amount, etc.
func (ob *Observer) GetInboundVoteFromBtcEvent(event *BTCInboundEvent) *crosschaintypes.MsgVoteInbound {
// prepare logger fields
lf := map[string]any{
logs.FieldModule: logs.ModNameInbound,
logs.FieldMethod: "GetInboundVoteFromBtcEvent",
logs.FieldChain: ob.Chain().ChainId,
logs.FieldTx: event.TxHash,
}

Expand All @@ -349,7 +338,7 @@
}

// check if the event is processable
if !ob.CheckEventProcessability(*event) {
if !ob.IsEventProcessable(*event) {
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion zetaclient/chains/bitcoin/signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@
if err != nil {
logger.Warn().
Err(err).
Msgf("SignConnectorOnReceive error: nonce %d chain %d", outboundTssNonce, params.ReceiverChainId)
Msgf("SignWithdrawTx error: nonce %d chain %d", outboundTssNonce, params.ReceiverChainId)

Check warning on line 453 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L453

Added line #L453 was not covered by tests
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
return
}
logger.Info().
Expand Down
Loading
Loading