From ee17d0f8281255e6e875ad17dd2feeee762b1551 Mon Sep 17 00:00:00 2001 From: Lazar Date: Mon, 14 Oct 2024 12:18:15 +0200 Subject: [PATCH 1/6] fix index-out-of-range --- submitter/relayer/relayer.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/submitter/relayer/relayer.go b/submitter/relayer/relayer.go index 4481d45..d609199 100644 --- a/submitter/relayer/relayer.go +++ b/submitter/relayer/relayer.go @@ -514,6 +514,10 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc return nil, err } + if len(rawTxResult.Transaction.TxOut) <= changePosition { + return nil, fmt.Errorf("transaction doesn't have change output %s", rawTxResult.Transaction.TxID()) + } + rl.logger.Debugf("Building a BTC tx using %s with data %x", rawTxResult.Transaction.TxID(), data) _, addresses, _, err := txscript.ExtractPkScriptAddrs( From 3a74e9fc188d6429254c4f1f4eb877ae39aaa3da Mon Sep 17 00:00:00 2001 From: Lazar Date: Mon, 14 Oct 2024 12:58:53 +0200 Subject: [PATCH 2/6] first tx handle and ignore for 2nd --- submitter/relayer/relayer.go | 52 +++++++++++++++++++++--------------- types/ckpt_info.go | 9 +++---- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/submitter/relayer/relayer.go b/submitter/relayer/relayer.go index d609199..901a244 100644 --- a/submitter/relayer/relayer.go +++ b/submitter/relayer/relayer.go @@ -487,7 +487,9 @@ func (rl *Relayer) ChainTwoTxAndSend(data1 []byte, data2 []byte) (*types.BtcTxIn func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.BtcTxInfo, error) { tx := wire.NewMsgTx(wire.TxVersion) - if firstTx != nil { + isFirstTx := firstTx != nil + + if isFirstTx { txID := firstTx.TxHash() outPoint := wire.NewOutPoint(&txID, changePosition) txIn := wire.NewTxIn(outPoint, nil, nil) @@ -514,37 +516,44 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc return nil, err } - if len(rawTxResult.Transaction.TxOut) <= changePosition { + hasChange := len(rawTxResult.Transaction.TxOut) > changePosition + // fail so we retry, first tx must have change output + if isFirstTx && !hasChange { return nil, fmt.Errorf("transaction doesn't have change output %s", rawTxResult.Transaction.TxID()) } rl.logger.Debugf("Building a BTC tx using %s with data %x", rawTxResult.Transaction.TxID(), data) - _, addresses, _, err := txscript.ExtractPkScriptAddrs( - rawTxResult.Transaction.TxOut[changePosition].PkScript, - rl.GetNetParams(), - ) + if hasChange { + _, addresses, _, err := txscript.ExtractPkScriptAddrs( + rawTxResult.Transaction.TxOut[changePosition].PkScript, + rl.GetNetParams(), + ) - if err != nil { - return nil, err - } + if err != nil { + return nil, err + } - if len(addresses) == 0 { - return nil, errors.New("no change address found") - } + if len(addresses) == 0 { + return nil, errors.New("no change address found") + } - changeAddr := addresses[0] - rl.logger.Debugf("Got a change address %v", changeAddr.String()) + rl.logger.Debugf("Got a change address %v", addresses[0].String()) + } txSize, err := calculateTxVirtualSize(rawTxResult.Transaction) if err != nil { return nil, err } - changeAmount := btcutil.Amount(rawTxResult.Transaction.TxOut[changePosition].Value) + var changeAmount btcutil.Amount + if hasChange { + changeAmount = btcutil.Amount(rawTxResult.Transaction.TxOut[changePosition].Value) + } + minRelayFee := rl.calcMinRelayFee(txSize) - if changeAmount < minRelayFee { + if hasChange && changeAmount < minRelayFee { return nil, fmt.Errorf("the value of the utxo is not sufficient for relaying the tx. Require: %v. Have: %v", minRelayFee, changeAmount) } @@ -554,7 +563,7 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc txFee = minRelayFee } // ensuring the tx fee is not higher than the utxo value - if changeAmount < txFee { + if hasChange && changeAmount < txFee { return nil, fmt.Errorf("the value of the utxo is not sufficient for paying the calculated fee of the tx. Calculated: %v. Have: %v", txFee, changeAmount) } @@ -572,7 +581,7 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc change := changeAmount - txFee - if change < dustThreshold { + if hasChange && change < dustThreshold { return nil, fmt.Errorf("change amount is %v less then dust treshold %v", change, dustThreshold) } @@ -580,10 +589,9 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc txFee, changeAmount, txSize, hex.EncodeToString(signedTxBytes.Bytes())) return &types.BtcTxInfo{ - Tx: tx, - ChangeAddress: changeAddr, - Size: txSize, - Fee: txFee, + Tx: tx, + Size: txSize, + Fee: txFee, }, nil } diff --git a/types/ckpt_info.go b/types/ckpt_info.go index 5d64c6b..0e87b89 100644 --- a/types/ckpt_info.go +++ b/types/ckpt_info.go @@ -18,9 +18,8 @@ type CheckpointInfo struct { // BtcTxInfo stores information of a BTC tx as part of a checkpoint type BtcTxInfo struct { - TxId *chainhash.Hash - Tx *wire.MsgTx - ChangeAddress btcutil.Address - Size int64 // the size of the BTC tx - Fee btcutil.Amount // tx fee cost by the BTC tx + TxId *chainhash.Hash + Tx *wire.MsgTx + Size int64 // the size of the BTC tx + Fee btcutil.Amount // tx fee cost by the BTC tx } From f52ec36b862d28497fef118846845d4007b2a415 Mon Sep 17 00:00:00 2001 From: Lazar Date: Mon, 14 Oct 2024 13:00:15 +0200 Subject: [PATCH 3/6] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9371498..d985427 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Improvements +* [#79](https://github.com/babylonlabs-io/vigilante/pull/79) handle no change output when building tx + * [#77](https://github.com/babylonlabs-io/vigilante/pull/77) add arm64 static build * [#76](https://github.com/babylonlabs-io/vigilante/pull/76) add goreleaser From db1164f43f9ac146badda69c42784d18c4d91184 Mon Sep 17 00:00:00 2001 From: Lazar Date: Mon, 14 Oct 2024 13:07:33 +0200 Subject: [PATCH 4/6] comment --- submitter/relayer/relayer.go | 1 + 1 file changed, 1 insertion(+) diff --git a/submitter/relayer/relayer.go b/submitter/relayer/relayer.go index 901a244..dfcbddb 100644 --- a/submitter/relayer/relayer.go +++ b/submitter/relayer/relayer.go @@ -516,6 +516,7 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc return nil, err } + // we want to ensure that firstTx has change output, but for the second transaction we can ignore this hasChange := len(rawTxResult.Transaction.TxOut) > changePosition // fail so we retry, first tx must have change output if isFirstTx && !hasChange { From fa2cc74bb9df3b5c6aafdab2233dd88712701b5e Mon Sep 17 00:00:00 2001 From: Lazar Date: Mon, 14 Oct 2024 16:28:11 +0200 Subject: [PATCH 5/6] manually add change output --- cmd/vigilante/cmd/submitter.go | 1 + e2etest/monitor_e2e_test.go | 1 + e2etest/submitter_e2e_test.go | 2 ++ submitter/relayer/change_address_test.go | 2 +- submitter/relayer/relayer.go | 24 +++++++++++++++++++----- submitter/submitter.go | 2 ++ 6 files changed, 26 insertions(+), 6 deletions(-) diff --git a/cmd/vigilante/cmd/submitter.go b/cmd/vigilante/cmd/submitter.go index b90acd6..a734578 100644 --- a/cmd/vigilante/cmd/submitter.go +++ b/cmd/vigilante/cmd/submitter.go @@ -79,6 +79,7 @@ func GetSubmitterCmd() *cobra.Command { cfg.Common.MaxRetryTimes, submitterMetrics, dbBackend, + cfg.BTC.WalletName, ) if err != nil { panic(fmt.Errorf("failed to create vigilante submitter: %w", err)) diff --git a/e2etest/monitor_e2e_test.go b/e2etest/monitor_e2e_test.go index bd4f1cd..844a89c 100644 --- a/e2etest/monitor_e2e_test.go +++ b/e2etest/monitor_e2e_test.go @@ -63,6 +63,7 @@ func TestMonitorBootstrap(t *testing.T) { tm.Config.Common.MaxRetryTimes, metrics.NewSubmitterMetrics(), testutil.MakeTestBackend(t), + tm.Config.BTC.WalletName, ) vigilantSubmitter.Start() diff --git a/e2etest/submitter_e2e_test.go b/e2etest/submitter_e2e_test.go index 6ec647b..81dc024 100644 --- a/e2etest/submitter_e2e_test.go +++ b/e2etest/submitter_e2e_test.go @@ -68,6 +68,7 @@ func TestSubmitterSubmission(t *testing.T) { tm.Config.Common.MaxRetryTimes, metrics.NewSubmitterMetrics(), testutil.MakeTestBackend(t), + tm.Config.BTC.WalletName, ) vigilantSubmitter.Start() @@ -144,6 +145,7 @@ func TestSubmitterSubmissionReplace(t *testing.T) { tm.Config.Common.MaxRetryTimes, metrics.NewSubmitterMetrics(), testutil.MakeTestBackend(t), + tm.Config.BTC.WalletName, ) vigilantSubmitter.Start() diff --git a/submitter/relayer/change_address_test.go b/submitter/relayer/change_address_test.go index fb7e772..591a8c6 100644 --- a/submitter/relayer/change_address_test.go +++ b/submitter/relayer/change_address_test.go @@ -49,7 +49,7 @@ func TestGetChangeAddress(t *testing.T) { cfg := config.DefaultSubmitterConfig() logger, err := config.NewRootLogger("auto", "debug") require.NoError(t, err) - testRelayer := relayer.New(wallet, []byte("bbnt"), btctxformatter.CurrentVersion, submitterAddr, + testRelayer := relayer.New(wallet, btcConfig.WalletName, []byte("bbnt"), btctxformatter.CurrentVersion, submitterAddr, submitterMetrics.RelayerMetrics, nil, &cfg, logger, testutil.MakeTestBackend(t)) // 1. only SegWit Bech32 addresses diff --git a/submitter/relayer/relayer.go b/submitter/relayer/relayer.go index dfcbddb..351e4cb 100644 --- a/submitter/relayer/relayer.go +++ b/submitter/relayer/relayer.go @@ -52,10 +52,12 @@ type Relayer struct { metrics *metrics.RelayerMetrics config *config.SubmitterConfig logger *zap.SugaredLogger + walletName string } func New( wallet btcclient.BTCWallet, + walletName string, tag btctxformatter.BabylonTag, version btctxformatter.FormatVersion, submitterAddress sdk.AccAddress, @@ -74,6 +76,7 @@ func New( return &Relayer{ Estimator: est, BTCWallet: wallet, + walletName: walletName, store: subStore, tag: tag, version: version, @@ -487,9 +490,9 @@ func (rl *Relayer) ChainTwoTxAndSend(data1 []byte, data2 []byte) (*types.BtcTxIn func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.BtcTxInfo, error) { tx := wire.NewMsgTx(wire.TxVersion) - isFirstTx := firstTx != nil + isSecondTx := firstTx != nil - if isFirstTx { + if isSecondTx { txID := firstTx.TxHash() outPoint := wire.NewOutPoint(&txID, changePosition) txIn := wire.NewTxIn(outPoint, nil, nil) @@ -518,9 +521,20 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc // we want to ensure that firstTx has change output, but for the second transaction we can ignore this hasChange := len(rawTxResult.Transaction.TxOut) > changePosition - // fail so we retry, first tx must have change output - if isFirstTx && !hasChange { - return nil, fmt.Errorf("transaction doesn't have change output %s", rawTxResult.Transaction.TxID()) + // let's manually add a change output with 546 satoshis + if !isSecondTx && !hasChange { + changeAddr, err := rl.BTCWallet.GetRawChangeAddress(rl.walletName) + if err != nil { + return nil, fmt.Errorf("err getting raw change address %w", err) + } + + changePkScript, err := txscript.PayToAddrScript(changeAddr) + if err != nil { + return nil, fmt.Errorf("failed to create script for change address: %s err %w", changeAddr, err) + } + + changeOutput := wire.NewTxOut(546, changePkScript) + rawTxResult.Transaction.AddTxOut(changeOutput) } rl.logger.Debugf("Building a BTC tx using %s with data %x", rawTxResult.Transaction.TxID(), data) diff --git a/submitter/submitter.go b/submitter/submitter.go index 86fbb7b..d72e8fb 100644 --- a/submitter/submitter.go +++ b/submitter/submitter.go @@ -45,6 +45,7 @@ func New( retrySleepTime, maxRetrySleepTime time.Duration, maxRetryTimes uint, submitterMetrics *metrics.SubmitterMetrics, db kvdb.Backend, + walletName string, ) (*Submitter, error) { logger := parentLogger.With(zap.String("module", "submitter")) var ( @@ -80,6 +81,7 @@ func New( r := relayer.New( btcWallet, + walletName, checkpointTag, btctxformatter.CurrentVersion, submitterAddr, From bfbd8e1648a78f4de155ced51e7f217cfc18c9ce Mon Sep 17 00:00:00 2001 From: Lazar Date: Tue, 15 Oct 2024 17:01:38 +0200 Subject: [PATCH 6/6] pr-comment --- submitter/relayer/relayer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submitter/relayer/relayer.go b/submitter/relayer/relayer.go index 351e4cb..d6301eb 100644 --- a/submitter/relayer/relayer.go +++ b/submitter/relayer/relayer.go @@ -533,7 +533,7 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc return nil, fmt.Errorf("failed to create script for change address: %s err %w", changeAddr, err) } - changeOutput := wire.NewTxOut(546, changePkScript) + changeOutput := wire.NewTxOut(int64(dustThreshold), changePkScript) rawTxResult.Transaction.AddTxOut(changeOutput) }