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: abort cctx on dust amount in revert outbound (Bitcoin and Solana) #3149

Merged
merged 6 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -26,6 +26,7 @@
* [3041](https://github.com/zeta-chain/node/pull/3041) - replace libp2p public DHT with private gossip peer discovery and connection gater for inbound connections
* [3106](https://github.com/zeta-chain/node/pull/3106) - prevent blocked CCTX on out of gas during omnichain calls
* [3139](https://github.com/zeta-chain/node/pull/3139) - fix config resolution in orchestrator
* [3149](https://github.com/zeta-chain/node/pull/3149) - abort the cctx if dust amount is detected in the revert outbound

## v21.0.0

Expand Down
3 changes: 2 additions & 1 deletion cmd/zetae2e/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,8 @@ func localE2ETest(cmd *cobra.Command, _ []string) {
e2etests.TestSolanaDepositName,
e2etests.TestSolanaWithdrawName,
e2etests.TestSolanaDepositAndCallName,
e2etests.TestSolanaDepositAndCallRefundName,
e2etests.TestSolanaDepositAndCallRevertName,
e2etests.TestSolanaDepositAndCallRevertWithDustName,
e2etests.TestSolanaDepositRestrictedName,
e2etests.TestSolanaWithdrawRestrictedName,
// TODO move under admin tests
Expand Down
33 changes: 20 additions & 13 deletions e2e/e2etests/e2etests.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,17 @@ const (
/*
* Solana tests
*/
TestSolanaDepositName = "solana_deposit"
TestSolanaWithdrawName = "solana_withdraw"
TestSolanaDepositAndCallName = "solana_deposit_and_call"
TestSolanaDepositAndCallRefundName = "solana_deposit_and_call_refund"
TestSolanaDepositRestrictedName = "solana_deposit_restricted"
TestSolanaWithdrawRestrictedName = "solana_withdraw_restricted"
TestSPLDepositName = "spl_deposit"
TestSPLDepositAndCallName = "spl_deposit_and_call"
TestSPLWithdrawName = "spl_withdraw"
TestSPLWithdrawAndCreateReceiverAtaName = "spl_withdraw_and_create_receiver_ata"
TestSolanaDepositName = "solana_deposit"
TestSolanaWithdrawName = "solana_withdraw"
TestSolanaDepositAndCallName = "solana_deposit_and_call"
TestSolanaDepositAndCallRevertName = "solana_deposit_and_call_revert"
TestSolanaDepositAndCallRevertWithDustName = "solana_deposit_and_call_revert_with_dust"
TestSolanaDepositRestrictedName = "solana_deposit_restricted"
TestSolanaWithdrawRestrictedName = "solana_withdraw_restricted"
TestSPLDepositName = "spl_deposit"
TestSPLDepositAndCallName = "spl_deposit_and_call"
TestSPLWithdrawName = "spl_withdraw"
TestSPLWithdrawAndCreateReceiverAtaName = "spl_withdraw_and_create_receiver_ata"

/**
* TON tests
Expand Down Expand Up @@ -453,12 +454,18 @@ var AllE2ETests = []runner.E2ETest{
TestSPLWithdrawAndCreateReceiverAta,
),
runner.NewE2ETest(
TestSolanaDepositAndCallRefundName,
"deposit SOL into ZEVM and call a contract that reverts; should refund",
TestSolanaDepositAndCallRevertName,
"deposit SOL into ZEVM and call a contract that reverts",
[]runner.ArgDefinition{
{Description: "amount in lamport", DefaultValue: "1200000"},
},
TestSolanaDepositAndCallRefund,
TestSolanaDepositAndCallRevert,
),
runner.NewE2ETest(
TestSolanaDepositAndCallRevertWithDustName,
"deposit SOL into ZEVM; revert with dust amount that aborts the CCTX",
[]runner.ArgDefinition{},
TestSolanaDepositAndCallRevertWithDust,
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
),
runner.NewE2ETest(
TestSolanaDepositRestrictedName,
Expand Down
18 changes: 9 additions & 9 deletions e2e/e2etests/test_bitcoin_deposit_and_call_revert_with_dust.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package e2etests

import (
"strings"

"github.com/stretchr/testify/require"

"github.com/zeta-chain/node/e2e/runner"
Expand Down Expand Up @@ -38,16 +40,14 @@ func TestBitcoinDepositAndCallRevertWithDust(r *runner.E2ERunner, args []string)
// Send BTC to TSS address with a dummy memo
// zetacore should revert cctx if call is made on a non-existing address
nonExistReceiver := sample.EthAddress()
badMemo := append(nonExistReceiver.Bytes(), []byte("gibberish-memo")...)
txHash, err := r.SendToTSSFromDeployerWithMemo(amount, utxos, badMemo)
anyMemo := append(nonExistReceiver.Bytes(), []byte("gibberish-memo")...)
txHash, err := r.SendToTSSFromDeployerWithMemo(amount, utxos, anyMemo)
require.NoError(r, err)
require.NotEmpty(r, txHash)

// wait for the cctx to be mined
cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, txHash.String(), r.CctxClient, r.Logger, r.CctxTimeout)
r.Logger.CCTX(*cctx, "deposit_and_revert")
utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_Reverted)

// check the test was effective: the revert outbound amount is less than the dust amount
require.Less(r, cctx.GetCurrentOutboundParam().Amount.Uint64(), uint64(constant.BTCWithdrawalDustAmount))
// ASSERT
// Now we want to make sure the cctx is aborted with expected error message
cctx := utils.WaitCctxAbortedByInboundHash(r.Ctx, r, txHash.String(), r.CctxClient)
require.True(r, cctx.GetCurrentOutboundParam().Amount.Uint64() < constant.BTCWithdrawalDustAmount)
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
require.True(r, strings.Contains(cctx.CctxStatus.ErrorMessage, crosschaintypes.ErrInvalidWithdrawalAmount.Error()))
}
34 changes: 34 additions & 0 deletions e2e/e2etests/test_solana_deposit_and_call_revert_with_dust.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package e2etests

import (
"math/big"
"strings"

"github.com/stretchr/testify/require"

"github.com/zeta-chain/node/e2e/runner"
"github.com/zeta-chain/node/e2e/utils"
"github.com/zeta-chain/node/pkg/constant"
"github.com/zeta-chain/node/testutil/sample"
crosschaintypes "github.com/zeta-chain/node/x/crosschain/types"
)

// TestSolanaDepositAndCallRevertWithDust tests Solana deposit and call that reverts with a dust amount in the revert outbound.
func TestSolanaDepositAndCallRevertWithDust(r *runner.E2ERunner, args []string) {
require.Len(r, args, 0)

// deposit the rent exempt amount which will result in a dust amount (after fee deduction) in the revert outbound
depositAmount := big.NewInt(constant.SolanaWalletRentExempt)

// ACT
// execute the deposit and call transaction
nonExistReceiver := sample.EthAddress()
data := []byte("dust lamports should abort cctx")
sig := r.SOLDepositAndCall(nil, nonExistReceiver, depositAmount, data)
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved

// ASSERT
// Now we want to make sure cctx is aborted.
cctx := utils.WaitCctxAbortedByInboundHash(r.Ctx, r, sig.String(), r.CctxClient)
require.True(r, cctx.GetCurrentOutboundParam().Amount.Uint64() < constant.SolanaWalletRentExempt)
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
require.True(r, strings.Contains(cctx.CctxStatus.ErrorMessage, crosschaintypes.ErrInvalidWithdrawalAmount.Error()))
}
4 changes: 2 additions & 2 deletions e2e/e2etests/test_solana_deposit_refund.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
crosschaintypes "github.com/zeta-chain/node/x/crosschain/types"
)

// TestSolanaDepositAndCallRefund tests deposit of lamports calling a example contract
func TestSolanaDepositAndCallRefund(r *runner.E2ERunner, args []string) {
// TestSolanaDepositAndCallRevert tests deposit of lamports calling a example contract that reverts.
func TestSolanaDepositAndCallRevert(r *runner.E2ERunner, args []string) {
require.Len(r, args, 1)

// parse deposit amount (in lamports)
Expand Down
14 changes: 14 additions & 0 deletions e2e/utils/zetacore.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,20 @@ func WaitCctxRevertedByInboundHash(
return cctxs[0]
}

// WaitCctxAbortedByInboundHash waits until cctx is aborted by inbound hash.
func WaitCctxAbortedByInboundHash(
ctx context.Context,
t require.TestingT,
hash string,
c CCTXClient,
) crosschaintypes.CrossChainTx {
// wait for cctx to be aborted
cctxs := WaitCctxByInboundHash(ctx, t, hash, c, MatchStatus(crosschaintypes.CctxStatus_Aborted))
require.Len(t, cctxs, 1)
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved

return cctxs[0]
}

// WaitCctxByInboundHash waits until cctx appears by inbound hash.
func WaitCctxByInboundHash(
ctx context.Context,
Expand Down
13 changes: 13 additions & 0 deletions x/crosschain/keeper/cctx_orchestrator_validate_outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@
return cosmoserrors.Wrap(err, "AddRevertOutbound")
}

// pay revert outbound gas fee
err = k.PayGasAndUpdateCctx(
ctx,
cctx.InboundParams.SenderChainId,
Expand All @@ -192,6 +193,18 @@
if err != nil {
return err
}

// validate data of the revert outbound
err = k.validateZRC20Withdrawal(
lumtis marked this conversation as resolved.
Show resolved Hide resolved
ctx,
cctx.GetCurrentOutboundParam().ReceiverChainId,
cctx.GetCurrentOutboundParam().Amount.BigInt(),
[]byte(cctx.GetCurrentOutboundParam().Receiver),
)
if err != nil {
return err
}

Check warning on line 206 in x/crosschain/keeper/cctx_orchestrator_validate_outbound.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/cctx_orchestrator_validate_outbound.go#L205-L206

Added lines #L205 - L206 were not covered by tests

err = k.SetObserverOutboundInfo(ctx, cctx.InboundParams.SenderChainId, cctx)
if err != nil {
return err
Expand Down
Loading