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 9 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
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
* [3155](https://github.com/zeta-chain/node/pull/3155) - fix potential panic in the Bitcoin inscription parsing
* [3162](https://github.com/zeta-chain/node/pull/3162) - skip depositor fee calculation if transaction does not involve TSS address
* [3179](https://github.com/zeta-chain/node/pull/3179) - support inbound trackers for v2 cctx
* [3184](https://github.com/zeta-chain/node/pull/3184) - zetaclient should not retry if inbound vote message validation fails
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved

## v21.0.0

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
42 changes: 14 additions & 28 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,24 @@
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 {
switch category := event.Category(); category {
case clienttypes.InboundCategoryGood:
return true
case InboundProcessabilityDonation:
case clienttypes.InboundCategoryDonation:
logFields := map[string]any{
logs.FieldChain: ob.Chain().ChainId,
logs.FieldTx: event.TxHash,
}
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().Msgf("unreachable code got InboundProcessability: %v", category)

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L171 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
26 changes: 19 additions & 7 deletions zetaclient/chains/bitcoin/observer/inbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,12 +332,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 +352,7 @@
}

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

Expand All @@ -361,13 +364,22 @@
}
amountInt := big.NewInt(amountSats)

// create inbound vote message contract V1 for legacy memo
// create inbound vote message contract V1 for legacy memo or standard memo
var msg *crosschaintypes.MsgVoteInbound
if event.MemoStd == nil {
return ob.NewInboundVoteFromLegacyMemo(event, amountInt)
msg = ob.NewInboundVoteFromLegacyMemo(event, amountInt)
} else {
msg = ob.NewInboundVoteFromStdMemo(event, amountInt)

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L372 was not covered by tests
}

// create inbound vote message for standard memo
return ob.NewInboundVoteFromStdMemo(event, amountInt)
// make sure the message is valid before posting to zetacore
err = msg.ValidateBasic()
if err != nil {
ob.Logger().Inbound.Error().Err(err).Fields(lf).Msg("invalid inbound vote message")
return nil
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
}

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

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/inbound.go#L378-L380

Added lines #L378 - L380 were not covered by tests

return msg
}

// GetBtcEvent returns a valid BTCInboundEvent or nil
Expand Down
4 changes: 3 additions & 1 deletion zetaclient/chains/bitcoin/observer/inbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ func Test_GetInboundVoteFromBtcEvent(t *testing.T) {

// create test observer
ob := MockBTCObserver(t, chain, params, nil)
zetacoreClient := mocks.NewZetacoreClient(t).WithKeys(&keys.Keys{}).WithZetaChain()
zetacoreClient := mocks.NewZetacoreClient(t).WithKeys(&keys.Keys{
OperatorAddress: sample.Bech32AccAddress(),
}).WithZetaChain()
ob.WithZetacoreClient(zetacoreClient)

// test cases
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
10 changes: 5 additions & 5 deletions zetaclient/chains/evm/observer/v2_inbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
"github.com/zeta-chain/node/zetaclient/zetacore"
)

// checkEventProcessability checks if the event is processable
func (ob *Observer) checkEventProcessability(
// IsEventProcessable checks if the event is processable
func (ob *Observer) IsEventProcessable(
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
sender, receiver ethcommon.Address,
txHash ethcommon.Hash,
payload []byte,
Expand Down Expand Up @@ -99,7 +99,7 @@
}

// check if the event is processable
if !ob.checkEventProcessability(event.Sender, event.Receiver, event.Raw.TxHash, event.Payload) {
if !ob.IsEventProcessable(event.Sender, event.Receiver, event.Raw.TxHash, event.Payload) {

Check warning on line 102 in zetaclient/chains/evm/observer/v2_inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/v2_inbound.go#L102

Added line #L102 was not covered by tests
continue
}

Expand Down Expand Up @@ -247,7 +247,7 @@
}

// check if the event is processable
if !ob.checkEventProcessability(event.Sender, event.Receiver, event.Raw.TxHash, event.Payload) {
if !ob.IsEventProcessable(event.Sender, event.Receiver, event.Raw.TxHash, event.Payload) {

Check warning on line 250 in zetaclient/chains/evm/observer/v2_inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/v2_inbound.go#L250

Added line #L250 was not covered by tests
continue
}

Expand Down Expand Up @@ -378,7 +378,7 @@
}

// check if the event is processable
if !ob.checkEventProcessability(event.Sender, event.Receiver, event.Raw.TxHash, event.Payload) {
if !ob.IsEventProcessable(event.Sender, event.Receiver, event.Raw.TxHash, event.Payload) {

Check warning on line 381 in zetaclient/chains/evm/observer/v2_inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/v2_inbound.go#L381

Added line #L381 was not covered by tests
continue
}

Expand Down
6 changes: 3 additions & 3 deletions zetaclient/chains/evm/observer/v2_inbound_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
eventDeposit, err := gateway.ParseDeposited(*log)
if err == nil {
// check if the event is processable
if !ob.checkEventProcessability(
if !ob.IsEventProcessable(

Check warning on line 39 in zetaclient/chains/evm/observer/v2_inbound_tracker.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/v2_inbound_tracker.go#L39

Added line #L39 was not covered by tests
eventDeposit.Sender,
eventDeposit.Receiver,
eventDeposit.Raw.TxHash,
Expand All @@ -53,7 +53,7 @@
eventDepositAndCall, err := gateway.ParseDepositedAndCalled(*log)
if err == nil {
// check if the event is processable
if !ob.checkEventProcessability(
if !ob.IsEventProcessable(

Check warning on line 56 in zetaclient/chains/evm/observer/v2_inbound_tracker.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/v2_inbound_tracker.go#L56

Added line #L56 was not covered by tests
eventDepositAndCall.Sender,
eventDepositAndCall.Receiver,
eventDepositAndCall.Raw.TxHash,
Expand All @@ -70,7 +70,7 @@
eventCall, err := gateway.ParseCalled(*log)
if err == nil {
// check if the event is processable
if !ob.checkEventProcessability(
if !ob.IsEventProcessable(

Check warning on line 73 in zetaclient/chains/evm/observer/v2_inbound_tracker.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/v2_inbound_tracker.go#L73

Added line #L73 was not covered by tests
eventCall.Sender,
eventCall.Receiver,
eventCall.Raw.TxHash,
Expand Down
Loading
Loading