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: allow BTC revert with dust amount #3142

Merged
merged 14 commits into from
Nov 12, 2024
1 change: 1 addition & 0 deletions cmd/zetae2e/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ func localE2ETest(cmd *cobra.Command, _ []string) {
e2etests.TestBitcoinDepositName,
e2etests.TestBitcoinDepositAndCallName,
e2etests.TestBitcoinDepositAndCallRevertName,
e2etests.TestBitcoinDepositAndCallRevertWithDustName,
e2etests.TestBitcoinStdMemoDepositName,
e2etests.TestBitcoinStdMemoDepositAndCallName,
e2etests.TestBitcoinStdMemoDepositAndCallRevertName,
Expand Down
6 changes: 6 additions & 0 deletions e2e/e2etests/e2etests.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const (
TestBitcoinDepositName = "bitcoin_deposit"
TestBitcoinDepositAndCallName = "bitcoin_deposit_and_call"
TestBitcoinDepositAndCallRevertName = "bitcoin_deposit_and_call_revert"
TestBitcoinDepositAndCallRevertWithDustName = "bitcoin_deposit_and_call_revert_with_dust"
TestBitcoinDonationName = "bitcoin_donation"
TestBitcoinStdMemoDepositName = "bitcoin_std_memo_deposit"
TestBitcoinStdMemoDepositAndCallName = "bitcoin_std_memo_deposit_and_call"
Expand Down Expand Up @@ -555,6 +556,11 @@ var AllE2ETests = []runner.E2ETest{
},
TestBitcoinDepositAndCallRevert,
),
runner.NewE2ETest(
TestBitcoinDepositAndCallRevertWithDustName,
"deposit Bitcoin into ZEVM; revert with dust amount that aborts the CCTX", []runner.ArgDefinition{},
TestBitcoinDepositAndCallRevertWithDust,
),
runner.NewE2ETest(
TestBitcoinStdMemoDepositName,
"deposit Bitcoin into ZEVM with standard memo",
Expand Down
53 changes: 53 additions & 0 deletions e2e/e2etests/test_bitcoin_deposit_and_call_revert_with_dust.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package e2etests

import (
"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"
zetabitcoin "github.com/zeta-chain/node/zetaclient/chains/bitcoin"
)

// TestBitcoinDepositAndCallRevertWithDust sends a Bitcoin deposit that reverts with a dust amount in the revert outbound.
func TestBitcoinDepositAndCallRevertWithDust(r *runner.E2ERunner, args []string) {
// Given "Live" BTC network
stop := r.MineBlocksIfLocalBitcoin()
defer stop()

require.Len(r, args, 0)

// 0.002 BTC is consumed in a deposit and revert, the dust is set to 1000 satoshis in the protocol
// Therefore the deposit amount should be 0.002 + 0.000001 = 0.00200100 should trigger the condition
// As only 100 satoshis are left after the deposit
const (
// depositAmount is 0.002001 BTC, chosen to result in 100 satoshis after deposit
// which is below the dust threshold of 1000 satoshis
depositAmount = 0.00200100
)
amount := depositAmount + zetabitcoin.DefaultDepositorFee

// Given a list of UTXOs
utxos, err := r.ListDeployerUTXOs()
require.NoError(r, err)
require.NotEmpty(r, utxos)

// ACT
// 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)
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))
}
lumtis marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 3 additions & 2 deletions zetaclient/chains/bitcoin/observer/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

"github.com/zeta-chain/node/pkg/chains"
"github.com/zeta-chain/node/pkg/coin"
"github.com/zeta-chain/node/pkg/constant"
crosschaintypes "github.com/zeta-chain/node/x/crosschain/types"
"github.com/zeta-chain/node/zetaclient/chains/bitcoin"
"github.com/zeta-chain/node/zetaclient/chains/bitcoin/rpc"
Expand Down Expand Up @@ -531,8 +532,8 @@
return errors.Wrapf(err, "checkTssOutboundResult: invalid TSS Vin in outbound %s nonce %d", hash, nonce)
}

// differentiate between normal and restricted cctx
if compliance.IsCctxRestricted(cctx) {
// differentiate between normal and cancelled cctx
if compliance.IsCctxRestricted(cctx) || params.Amount.Uint64() < constant.BTCWithdrawalDustAmount {

Check warning on line 536 in zetaclient/chains/bitcoin/observer/outbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/outbound.go#L536

Added line #L536 was not covered by tests
err = ob.checkTSSVoutCancelled(params, rawResult.Vout)
if err != nil {
return errors.Wrapf(
Expand Down
21 changes: 17 additions & 4 deletions zetaclient/chains/bitcoin/signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

"github.com/zeta-chain/node/pkg/chains"
"github.com/zeta-chain/node/pkg/coin"
"github.com/zeta-chain/node/pkg/constant"
"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"
Expand Down Expand Up @@ -413,13 +414,25 @@
gasprice.Add(gasprice, satPerByte)

// compliance check
cancelTx := compliance.IsCctxRestricted(cctx)
if cancelTx {
restrictedCCTX := compliance.IsCctxRestricted(cctx)
if restrictedCCTX {

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

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L417-L418

Added lines #L417 - L418 were not covered by tests
compliance.PrintComplianceLog(logger, signer.Logger().Compliance,
true, chain.ChainId, cctx.Index, cctx.InboundParams.Sender, params.Receiver, "BTC")
amount = 0.0 // zero out the amount to cancel the tx
}
logger.Info().Msgf("SignGasWithdraw: to %s, value %d sats", to.EncodeAddress(), params.Amount.Uint64())

// check dust amount
dustAmount := params.Amount.Uint64() < constant.BTCWithdrawalDustAmount
if dustAmount {
logger.Warn().Msgf("dust amount %d sats, canceling tx", params.Amount.Uint64())
}

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

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L424-L427

Added lines #L424 - L427 were not covered by tests

// set the amount to 0 when the tx should be cancelled
cancelTx := restrictedCCTX || dustAmount
if cancelTx {
lumtis marked this conversation as resolved.
Show resolved Hide resolved
amount = 0.0
} else {
logger.Info().Msgf("SignGasWithdraw: to %s, value %d sats", to.EncodeAddress(), params.Amount.Uint64())
}

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

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L430-L435

Added lines #L430 - L435 were not covered by tests
lumtis marked this conversation as resolved.
Show resolved Hide resolved

// sign withdraw tx
tx, err := signer.SignWithdrawTx(
Expand Down
Loading