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

feat: add fee estimation #118

Merged
merged 11 commits into from
Nov 28, 2022
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
46 changes: 43 additions & 3 deletions btcclient/client_wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,49 @@ func NewWallet(cfg *config.BTCConfig) (*Client, error) {
return wallet, nil
}

// TODO make it dynamic
func (c *Client) GetTxFee() uint64 {
return uint64(c.Cfg.TxFee.ToUnit(btcutil.AmountSatoshi))
// GetTxFee returns tx fee according to its size
// if tx size is zero, it returns the default tx
// fee in config
func (c *Client) GetTxFee(txSize uint64) uint64 {
var (
feeRate float64
err error
)

// estimatesmartfee is not supported by btcd so we use estimatefee in that case
estimateRes, err := c.Client.EstimateSmartFee(c.Cfg.TargetBlockNum, &btcjson.EstimateModeEconomical)
if err == nil {
if estimateRes.FeeRate == nil {
feeRate = 0
} else {
feeRate = *estimateRes.FeeRate
}
} else {
feeRate, err = c.Client.EstimateFee(c.Cfg.TargetBlockNum)
if err != nil {
panic(err)
}
}

log.Debugf("fee rate is %v", feeRate)
feeRateAmount, err := btcutil.NewAmount(feeRate)
if err != nil {
// this means the returned fee rate is very wrong, e.g., infinity
panic(err)
}
fee := feeRateAmount.MulF64(float64(txSize))
if fee > c.Cfg.TxFeeMax {
return uint64(c.Cfg.TxFeeMax)
}
if fee < c.Cfg.TxFeeMin {
return uint64(c.Cfg.TxFeeMin)
}

return uint64(fee)
}

func (c *Client) GetMaxTxFee() uint64 {
return uint64(c.Cfg.TxFeeMax)
}

func (c *Client) GetWalletName() string {
Expand Down
3 changes: 2 additions & 1 deletion btcclient/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ type BTCWallet interface {
GetWalletPass() string
GetWalletLockTime() int64
GetNetParams() *chaincfg.Params
GetTxFee() uint64 // in the unit of satoshi
GetTxFee(txSize uint64) uint64 // in the unit of satoshi
GetMaxTxFee() uint64 // in the unit of satoshi
ListUnspent() ([]btcjson.ListUnspentResult, error)
ListReceivedByAddress() ([]btcjson.ListReceivedByAddressResult, error)
SendRawTransaction(tx *wire.MsgTx, allowHighFees bool) (*chainhash.Hash, error)
Expand Down
23 changes: 17 additions & 6 deletions config/bitcoin.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ type BTCConfig struct {
WalletName string `mapstructure:"wallet-name"`
WalletCAFile string `mapstructure:"wallet-ca-file"`
WalletLockTime int64 `mapstructure:"wallet-lock-time"` // time duration in which the wallet remains unlocked, in seconds
TxFee btcutil.Amount `mapstructure:"tx-fee"` // BTC tx fee, in BTC
TxFeeMin btcutil.Amount `mapstructure:"tx-fee-min"` // minimum tx fee, in BTC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want a minimum transaction fee? Personally I would always set this to 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having 0 might cause an error saying the tx has insufficient priority. I'm not sure of the exact min fee we should set tho.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we remove this attribute alltogether. Why would someone want to set the minimum fee into anything other than the minimum value possible (e.g. 1)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we remove this attribute alltogether. Why would someone want to set the minimum fee into anything other than the minimum value possible (e.g. 1)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The estimation might return us an extremely low tx fee which is not reasonable. We set a reasonable tx fee here to prevent this case. It could be 1, 10, or 100.

TxFeeMax btcutil.Amount `mapstructure:"tx-fee-max"` // maximum tx fee, in BTC
Copy link
Member

@vitsalis vitsalis Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe panic if TxFeeMax < TxFeeMin?

TargetBlockNum int64 `mapstructure:"target-block-num"` // this implies how soon the tx is estimated to be included in a block, e.g., 1 means the tx is estimated to be included in the next block
NetParams string `mapstructure:"net-params"`
Username string `mapstructure:"username"`
Password string `mapstructure:"password"`
Expand Down Expand Up @@ -46,14 +48,21 @@ func (cfg *BTCConfig) Validate() error {
}
}

if cfg.TargetBlockNum <= 0 {
return errors.New("target-block-num should be positive")
}

if cfg.TxFeeMin > cfg.TxFeeMax {
return errors.New("tx-fee-min is larger than tx-fee-max")
}

return nil
}

func DefaultBTCConfig() BTCConfig {
feeAmount, err := btcutil.NewAmount(0.00001)
if err != nil {
panic(err)
}
feeAmountMin, _ := btcutil.NewAmount(100)
feeAmountMax, _ := btcutil.NewAmount(10000)

return BTCConfig{
DisableClientTLS: false,
CAFile: defaultBtcCAFile,
Expand All @@ -63,7 +72,9 @@ func DefaultBTCConfig() BTCConfig {
WalletName: "default",
WalletCAFile: defaultBtcWalletCAFile,
WalletLockTime: 10,
TxFee: feeAmount,
TxFeeMin: feeAmountMin,
TxFeeMax: feeAmountMax,
TargetBlockNum: 1,
NetParams: types.BtcSimnet.String(),
Username: "rpcuser",
Password: "rpcpass",
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/golang/mock v1.6.0
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
github.com/jinzhu/copier v0.3.5
github.com/pebbe/zmq4 v1.2.9
github.com/prometheus/client_golang v1.13.0
github.com/sirupsen/logrus v1.9.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,8 @@ github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANyt
github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc=
github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/jessevdk/go-flags v0.0.0-20141203071132-1679536dcc89/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/jinzhu/copier v0.3.5 h1:GlvfUwHk62RokgqVNvYsku0TATCF7bAHVwEXoBh3iJg=
github.com/jinzhu/copier v0.3.5/go.mod h1:DfbEm0FYsaqBcKcFuvmOZb218JkPGtvSHsKg8S8hyyg=
github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=
Expand Down
4 changes: 3 additions & 1 deletion sample-vigilante-docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ btc:
no-client-tls: false
ca-file: /bitcoin/rpc.cert
endpoint: host.docker.internal:18556
tx-fee: 2500
tx-fee-min: 200 # bitcoind requires that min tx fee is 200 satoshi
tx-fee-max: 100000
target-block-num: 2
wallet-endpoint: host.docker.internal:18554
wallet-password: walletpass
wallet-name: default
Expand Down
4 changes: 3 additions & 1 deletion sample-vigilante.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ btc:
no-client-tls: false # use true for bitcoind as it does not support tls
ca-file: $TESTNET_PATH/bitcoin/rpc.cert
endpoint: localhost:18556 # use port 18443 for bitcoind regtest
tx-fee: 2500
tx-fee-min: 200 # bitcoind requires that min tx fee is 200 satoshi
tx-fee-max: 100000
target-block-num: 2
wallet-endpoint: localhost:18554
wallet-password: walletpass
wallet-name: default
Expand Down
82 changes: 34 additions & 48 deletions submitter/relayer/relayer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"bytes"
"encoding/hex"
"errors"
"time"

"github.com/babylonchain/babylon/btctxformatter"
ckpttypes "github.com/babylonchain/babylon/x/checkpointing/types"
"github.com/babylonchain/vigilante/btcclient"
Expand All @@ -14,7 +16,7 @@ import (
"github.com/btcsuite/btcd/wire"
"github.com/btcsuite/btcutil"
sdk "github.com/cosmos/cosmos-sdk/types"
"time"
"github.com/jinzhu/copier"
)

type Relayer struct {
Expand Down Expand Up @@ -195,7 +197,7 @@ func (rl *Relayer) PickHighUTXO() (*types.UTXO, error) {
}

// TODO: consider dust, reference: https://www.oreilly.com/library/view/mastering-bitcoin/9781491902639/ch08.html#tx_verification
if uint64(amount.ToUnit(btcutil.AmountSatoshi)) < rl.GetTxFee()*2 {
if uint64(amount.ToUnit(btcutil.AmountSatoshi)) < rl.GetMaxTxFee()*2 {
return nil, errors.New("insufficient fees")
}

Expand Down Expand Up @@ -227,6 +229,23 @@ func (rl *Relayer) buildTxWithData(
txIn := wire.NewTxIn(outPoint, nil, nil)
tx.AddTxIn(txIn)

// get private key
err := rl.WalletPassphrase(rl.GetWalletPass(), rl.GetWalletLockTime())
if err != nil {
return nil, nil, err
}
wif, err := rl.DumpPrivKey(utxo.Addr)
if err != nil {
return nil, nil, err
}

// add signature/witness depending on the type of the previous address
// if not segwit, add signature; otherwise, add witness
segwit, err := isSegWit(utxo.Addr)
if err != nil {
panic(err)
}

// build txout for data
builder := txscript.NewScriptBuilder()
dataScript, err := builder.AddOp(txscript.OP_RETURN).AddData(data).Script()
Expand All @@ -246,58 +265,23 @@ func (rl *Relayer) buildTxWithData(
if err != nil {
return nil, nil, err
}
// TODO
// If this will become a dynamic calculation this might lead to a different fee being used in the log and the actual transaction.
change := uint64(utxo.Amount.ToUnit(btcutil.AmountSatoshi)) - rl.GetTxFee()
log.Logger.Debugf("balance of input: %v satoshi, tx fee: %v satoshi, output value: %v",
int64(utxo.Amount.ToUnit(btcutil.AmountSatoshi)), rl.GetTxFee(), change)
tx.AddTxOut(wire.NewTxOut(int64(change), changeScript))

// sign tx
err = rl.WalletPassphrase(rl.GetWalletPass(), rl.GetWalletLockTime())
copiedTx := &wire.MsgTx{}
err = copier.Copy(copiedTx, tx)
if err != nil {
return nil, nil, err
}
wif, err := rl.DumpPrivKey(utxo.Addr)
txSize, err := calTxSize(copiedTx, utxo, changeScript, segwit, wif.PrivKey)
if err != nil {
return nil, nil, err
}
txFee := rl.GetTxFee(txSize)
change := uint64(utxo.Amount.ToUnit(btcutil.AmountSatoshi)) - txFee
tx.AddTxOut(wire.NewTxOut(int64(change), changeScript))

// add signature/witness depending on the type of the previous address
// if not segwit, add signature; otherwise, add witness
segwit, err := isSegWit(utxo.Addr)
// add unlocking script into the input of the tx
tx, err = completeTxIn(tx, segwit, wif.PrivKey, utxo)
if err != nil {
panic(err)
}
if !segwit {
sig, err := txscript.SignatureScript(
tx,
0,
utxo.ScriptPK,
txscript.SigHashAll,
wif.PrivKey,
true)
if err != nil {
return nil, nil, err
}
tx.TxIn[0].SignatureScript = sig
} else {
log.Logger.Debug("constructing witness data for SegWit tx")
sighashes := txscript.NewTxSigHashes(tx)
wit, err := txscript.WitnessSignature(
tx,
sighashes,
0,
int64(utxo.Amount),
utxo.ScriptPK,
txscript.SigHashAll,
wif.PrivKey,
true,
)
if err != nil {
return nil, nil, err
}
tx.TxIn[0].Witness = wit
return nil, nil, err
}

// serialization
Expand All @@ -306,8 +290,10 @@ func (rl *Relayer) buildTxWithData(
if err != nil {
return nil, nil, err
}

log.Logger.Debugf("Successfully composed a BTC tx, hex: %v", hex.EncodeToString(signedTxHex.Bytes()))
log.Logger.Debugf("Successfully composed a BTC tx with balance of input: %v satoshi, "+
"tx fee: %v satoshi, output value: %v, estimated tx size: %v, actual tx size: %v, hex: %v",
int64(utxo.Amount.ToUnit(btcutil.AmountSatoshi)), txFee, change, txSize, tx.SerializeSizeStripped(),
hex.EncodeToString(signedTxHex.Bytes()))
return tx, changeAddr, nil
}

Expand Down
50 changes: 50 additions & 0 deletions submitter/relayer/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ package relayer

import (
"errors"
"github.com/babylonchain/vigilante/types"
"github.com/btcsuite/btcd/btcec"
"github.com/btcsuite/btcd/txscript"
"github.com/btcsuite/btcd/wire"
"github.com/btcsuite/btcutil"
)

Expand All @@ -15,3 +19,49 @@ func isSegWit(addr btcutil.Address) (bool, error) {
return false, errors.New("non-supported address type")
}
}

func calTxSize(tx *wire.MsgTx, utxo *types.UTXO, changeScript []byte, isSegWit bool, privkey *btcec.PrivateKey) (uint64, error) {
tx.AddTxOut(wire.NewTxOut(int64(utxo.Amount), changeScript))

tx, err := completeTxIn(tx, isSegWit, privkey, utxo)
if err != nil {
return 0, err
}

return uint64(tx.SerializeSizeStripped()), nil
}

func completeTxIn(tx *wire.MsgTx, isSegWit bool, privKey *btcec.PrivateKey, utxo *types.UTXO) (*wire.MsgTx, error) {
if !isSegWit {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if/else based on isSegwit already happens in the buildTxWithData. Can it be moved to a util function to avoid code duplication?

sig, err := txscript.SignatureScript(
tx,
0,
utxo.ScriptPK,
txscript.SigHashAll,
privKey,
true,
)
if err != nil {
return nil, err
}
tx.TxIn[0].SignatureScript = sig
} else {
sighashes := txscript.NewTxSigHashes(tx)
wit, err := txscript.WitnessSignature(
tx,
sighashes,
0,
int64(utxo.Amount),
utxo.ScriptPK,
txscript.SigHashAll,
privKey,
true,
)
if err != nil {
return nil, err
}
tx.TxIn[0].Witness = wit
}

return tx, nil
}
22 changes: 18 additions & 4 deletions testutil/mocks/btcclient.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.