Skip to content

Commit

Permalink
chore: Use int64 for fee config and other type fixes
Browse files Browse the repository at this point in the history
The libraries that we depend to use int64 for satoshi amount
denominations. To be fully compatible with them,
it is better if we use similar types instead of doing conversions.

Further, this PR fixes some other type conversions and
minor errors found by gosec.
  • Loading branch information
vitsalis committed Oct 12, 2024
1 parent b053ac8 commit 16687b8
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 75 deletions.
43 changes: 22 additions & 21 deletions babylonclient/babyloncontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,9 @@ func (bc *BabylonController) Stop() error {
}

func (bc *BabylonController) Params() (*StakingParams, error) {
// TODO: uint64 are quite silly types for these params, probably uint8 or uint16 would be enough
// as we do not expect finalization to be more than 255 or in super extreme 65535
// TODO: it would probably be good to have separate methods for those
var bccParams *bcctypes.Params
if err := retry.Do(func() error {

response, err := bc.bbnClient.BTCCheckpointParams()
if err != nil {
return err
Expand Down Expand Up @@ -174,15 +171,15 @@ func (bc *BabylonController) Params() (*StakingParams, error) {
return nil, err
}

if bccParams.CheckpointFinalizationTimeout > math.MaxUint16 {
return nil, fmt.Errorf("checkpoint finalization timeout is bigger than uint16: %w", ErrInvalidValueReceivedFromBabylonNode)
}

minUnbondingTime := sdkmath.Max[uint16](
uint16(bccParams.CheckpointFinalizationTimeout),
stakingTrackerParams.MinUnbondingTime,
minUnbondingTimeU64 := sdkmath.Max[uint64](
bccParams.CheckpointFinalizationTimeout,
uint64(stakingTrackerParams.MinUnbondingTime),
)

if minUnbondingTimeU64 > math.MaxUint16 {
return nil, fmt.Errorf("minimum unbonding time should fit in a uint16")
}

return &StakingParams{
ConfirmationTimeBlocks: uint32(bccParams.BtcConfirmationDepth),
FinalizationTimeoutBlocks: uint32(bccParams.CheckpointFinalizationTimeout),
Expand All @@ -191,7 +188,7 @@ func (bc *BabylonController) Params() (*StakingParams, error) {
MinSlashingTxFeeSat: stakingTrackerParams.MinSlashingFee,
SlashingRate: stakingTrackerParams.SlashingRate,
CovenantQuruomThreshold: stakingTrackerParams.CovenantQuruomThreshold,
MinUnbondingTime: minUnbondingTime,
MinUnbondingTime: uint16(minUnbondingTimeU64),
UnbondingFee: stakingTrackerParams.UnbondingFee,
MinStakingTime: stakingTrackerParams.MinStakingTime,
MaxStakingTime: stakingTrackerParams.MaxStakingTime,
Expand Down Expand Up @@ -470,15 +467,15 @@ func (bc *BabylonController) QueryStakingTracker() (*StakingTrackerResponse, err
return nil, err
}

// check this early than covenant config makes sense, so that rest of the
// check early that the covenant config makes sense, so that rest of the
// code can assume that:
// 1. covenant quorum is less or equal to number of covenant pks
// 2. covenant pks are not empty
if len(response.Params.CovenantPks) == 0 {
return nil, fmt.Errorf("empty list of covenant pks: %w", ErrInvalidValueReceivedFromBabylonNode)
}

if response.Params.CovenantQuorum > uint32(len(response.Params.CovenantPks)) {
if int(response.Params.CovenantQuorum) > len(response.Params.CovenantPks) {
return nil, fmt.Errorf("covenant quorum is bigger than number of covenant pks: %w", ErrInvalidValueReceivedFromBabylonNode)
}

Expand All @@ -492,15 +489,18 @@ func (bc *BabylonController) QueryStakingTracker() (*StakingTrackerResponse, err
covenantPks = append(covenantPks, covenantBtcPk)
}

if response.Params.MinUnbondingTimeBlocks > math.MaxUint16 {
minUnbondingTimeBlocksU32 := response.Params.MinUnbondingTimeBlocks
if minUnbondingTimeBlocksU32 > math.MaxUint16 {
return nil, fmt.Errorf("min unbonding time is bigger than uint16: %w", ErrInvalidValueReceivedFromBabylonNode)
}

if response.Params.MinStakingTimeBlocks > math.MaxUint16 {
minStakingTimeBlocksU32 := response.Params.MinStakingTimeBlocks
if minStakingTimeBlocksU32 > math.MaxUint16 {
return nil, fmt.Errorf("min staking time is bigger than uint16: %w", ErrInvalidValueReceivedFromBabylonNode)
}

if response.Params.MaxStakingTimeBlocks > math.MaxUint16 {
maxStakingTimeBlocksU32 := response.Params.MaxStakingTimeBlocks
if maxStakingTimeBlocksU32 > math.MaxUint16 {
return nil, fmt.Errorf("max staking time is bigger than uint16: %w", ErrInvalidValueReceivedFromBabylonNode)
}

Expand All @@ -523,10 +523,10 @@ func (bc *BabylonController) QueryStakingTracker() (*StakingTrackerResponse, err
CovenantPks: covenantPks,
MinSlashingFee: btcutil.Amount(response.Params.MinSlashingTxFeeSat),
CovenantQuruomThreshold: response.Params.CovenantQuorum,
MinUnbondingTime: uint16(response.Params.MinUnbondingTimeBlocks),
MinUnbondingTime: uint16(minUnbondingTimeBlocksU32),
UnbondingFee: btcutil.Amount(response.Params.UnbondingFeeSat),
MinStakingTime: uint16(response.Params.MinStakingTimeBlocks),
MaxStakingTime: uint16(response.Params.MaxStakingTimeBlocks),
MinStakingTime: uint16(minStakingTimeBlocksU32),
MaxStakingTime: uint16(maxStakingTimeBlocksU32),
MinStakingValue: btcutil.Amount(response.Params.MinStakingValueSat),
MaxStakingValue: btcutil.Amount(response.Params.MaxStakingValueSat),
}, nil
Expand Down Expand Up @@ -798,14 +798,15 @@ func (bc *BabylonController) QueryDelegationInfo(stakingTxHash *chainhash.Hash)
return retry.Unrecoverable(fmt.Errorf("malformed unbonding transaction: %s: %w", err.Error(), ErrInvalidValueReceivedFromBabylonNode))
}

if resp.BtcDelegation.UnbondingTime > math.MaxUint16 {
unbondingTimeU32 := resp.BtcDelegation.UnbondingTime
if unbondingTimeU32 > math.MaxUint16 {
return retry.Unrecoverable(fmt.Errorf("malformed unbonding time: %d: %w", resp.BtcDelegation.UnbondingTime, ErrInvalidValueReceivedFromBabylonNode))
}

udi = &UndelegationInfo{
UnbondingTransaction: tx,
CovenantUnbondingSignatures: coventSigInfos,
UnbondingTime: uint16(resp.BtcDelegation.UnbondingTime),
UnbondingTime: uint16(unbondingTimeU32),
}
}

Expand Down
10 changes: 5 additions & 5 deletions babylonclient/msgsender.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ var (
type sendDelegationRequest struct {
utils.Request[*pv.RelayerTxResponse]
dg *DelegationData
requiredInclusionBlockDepth uint64
requiredInclusionBlockDepth uint32
}

func newSendDelegationRequest(
dg *DelegationData,
requiredInclusionBlockDepth uint64,
requiredInclusionBlockDepth uint32,
) sendDelegationRequest {
return sendDelegationRequest{
Request: utils.NewRequest[*pv.RelayerTxResponse](),
Expand Down Expand Up @@ -100,7 +100,7 @@ func (b *BabylonMsgSender) Stop() {

// isBabylonBtcLcReady checks if Babylon BTC light client is ready to receive delegation
func (b *BabylonMsgSender) isBabylonBtcLcReady(
requiredInclusionBlockDepth uint64,
requiredInclusionBlockDepth uint32,
req *DelegationData,
) error {
// no need to consult Babylon if we send delegation without inclusion proof
Expand All @@ -121,7 +121,7 @@ func (b *BabylonMsgSender) isBabylonBtcLcReady(
return fmt.Errorf("error while getting delegation data: %w", err)
}

if depth < requiredInclusionBlockDepth {
if uint32(depth) < requiredInclusionBlockDepth {
return fmt.Errorf("btc lc not ready, required depth: %d, current depth: %d: %w", requiredInclusionBlockDepth, depth, ErrBabylonBtcLightClientNotReady)
}

Expand Down Expand Up @@ -247,7 +247,7 @@ func (m *BabylonMsgSender) handleSentToBabylon() {

func (m *BabylonMsgSender) SendDelegation(
dg *DelegationData,
requiredInclusionBlockDepth uint64,
requiredInclusionBlockDepth uint32,
) (*pv.RelayerTxResponse, error) {
req := newSendDelegationRequest(dg, requiredInclusionBlockDepth)

Expand Down
4 changes: 2 additions & 2 deletions babylonclient/pop.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

type BabylonBtcPopType int
type BabylonBtcPopType uint32

const (
SchnorrType BabylonBtcPopType = iota
Expand All @@ -39,7 +39,7 @@ func NewBabylonPop(t BabylonBtcPopType, btcSigOverBbnAddr []byte) (*BabylonPop,
}, nil
}

// NewBabylonBip322Pop build proper BabylonPop in BIP322 style, it verifies the
// NewBabylonBip322Pop build proper BabylonPop in BIP322 style, it verifies
// the bip322 signature validity
func NewBabylonBip322Pop(
msg []byte,
Expand Down
2 changes: 1 addition & 1 deletion cmd/stakercli/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func dumpCfg(c *cli.Context) error {
dir, _ := path.Split(configPath)

if _, err := os.Stat(dir); os.IsNotExist(err) {
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
if err := os.MkdirAll(dir, os.ModeDir); err != nil {
return cli.NewExitError(
fmt.Sprintf("could not create config directory: %s", err.Error()),
1,
Expand Down
11 changes: 11 additions & 0 deletions cmd/stakercli/transaction/parsers.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,14 @@ func parseTagFromHex(tagHex string) ([]byte, error) {

return tag, nil
}

func parseCovenantQuorumFromCliCtx(ctx *cli.Context) (uint32, error) {
covenantQuorumUint64 := ctx.Uint64(covenantQuorumFlag)
if covenantQuorumUint64 == 0 {
return 0, fmt.Errorf("covenant quorum should be greater than 0")
}
if covenantQuorumUint64 > math.MaxUint32 {
return 0, fmt.Errorf("covenant quorum should be less or equal to %d", math.MaxUint32)
}
return uint32(covenantQuorumUint64), nil
}
22 changes: 16 additions & 6 deletions cmd/stakercli/transaction/transactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,10 @@ func createPhase1StakingTransaction(ctx *cli.Context) error {
return err
}

covenantQuorum := uint32(ctx.Uint64(covenantQuorumFlag))
covenantQuorum, err := parseCovenantQuorumFromCliCtx(ctx)
if err != nil {
return err
}

_, tx, err := btcstaking.BuildV0IdentifiableStakingOutputsAndTx(
tag,
Expand Down Expand Up @@ -402,12 +405,14 @@ func checkPhase1StakingTransaction(ctx *cli.Context) error {
}

covenantMembersPks, err := parseCovenantKeysFromCliCtx(ctx)

if err != nil {
return err
}

covenantQuorum := uint32(ctx.Uint64(covenantQuorumFlag))
covenantQuorum, err := parseCovenantQuorumFromCliCtx(ctx)
if err != nil {
return err
}

stakingTx, err := btcstaking.ParseV0StakingTx(
tx,
Expand Down Expand Up @@ -439,9 +444,14 @@ func checkPhase1StakingTransaction(ctx *cli.Context) error {
}
}

timeBlocks := ctx.Int64(helpers.StakingTimeBlocksFlag)
if timeBlocks > 0 && uint16(timeBlocks) != stakingTx.OpReturnData.StakingTime {
return fmt.Errorf("staking time in tx %d do not match with flag %d", stakingTx.OpReturnData.StakingTime, timeBlocks)
if ctx.Int64(helpers.StakingTimeBlocksFlag) != 0 {
timeBlocks, err := parseLockTimeBlocksFromCliCtx(ctx, helpers.StakingTimeBlocksFlag)
if err != nil {
return err
}
if timeBlocks != stakingTx.OpReturnData.StakingTime {
return fmt.Errorf("staking time in tx %d do not match with flag %d", stakingTx.OpReturnData.StakingTime, timeBlocks)
}
}

txAmount := stakingTx.StakingOutput.Value
Expand Down
4 changes: 2 additions & 2 deletions staker/babylontypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ type inclusionInfo struct {

type sendDelegationRequest struct {
txHash chainhash.Hash
// optional field, if not provided, delegation will be send to Babylon without
// optional field, if not provided, delegation will be sent to Babylon without
// the inclusion proof
inclusionInfo *inclusionInfo
requiredInclusionBlockDepth uint64
requiredInclusionBlockDepth uint32
}

func (app *StakerApp) buildOwnedDelegation(
Expand Down
24 changes: 12 additions & 12 deletions staker/stakerapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/btcsuite/btcd/chaincfg/chainhash"
"github.com/btcsuite/btcd/txscript"
"github.com/btcsuite/btcd/wire"
"github.com/btcsuite/btcwallet/wallet/txrules"
"github.com/cometbft/cometbft/crypto/tmhash"
sdk "github.com/cosmos/cosmos-sdk/types"
notifier "github.com/lightningnetwork/lnd/chainntnfs"
Expand Down Expand Up @@ -98,9 +97,6 @@ const (

defaultWalletUnlockTimeout = 15

// Set minimum fee to 1 sat/byte, as in standard rules policy
MinFeePerKb = txrules.DefaultRelayFeePerKb

// If we fail to send unbonding tx to btc for any reason we will retry in this time
unbondingSendRetryTimeout = 1 * time.Minute

Expand Down Expand Up @@ -293,9 +289,13 @@ func (app *StakerApp) Start() error {
}

// we registered for notifications with `nil` so we should receive best block
// immeadiatly
// immediately
select {
case block := <-blockEventNotifier.Epochs:
if block.Height < 0 {
startErr = errors.New("block height is negative")
return
}
app.currentBestBlockHeight.Store(uint32(block.Height))
case <-app.quit:
startErr = errors.New("staker app quit before finishing start")
Expand Down Expand Up @@ -541,7 +541,7 @@ func (app *StakerApp) checkTransactionsStatus() error {
}

// In our scan we only record transactions which state need to be checked, as`ScanTrackedTransactions`
// is long running read transaction, it could dead lock with write transactions which we would need
// is long-running read transaction, it could deadlock with write transactions which we would need
// to use to update transaction state.
err = app.txTracker.ScanTrackedTransactions(func(tx *stakerdb.StoredTransaction) error {
// TODO : We need to have another stare like UnstakeTransaction sent and store
Expand Down Expand Up @@ -638,7 +638,7 @@ func (app *StakerApp) checkTransactionsStatus() error {
req := &sendDelegationRequest{
txHash: *txHashCopy,
inclusionInfo: nil,
requiredInclusionBlockDepth: uint64(stakingParams.ConfirmationTimeBlocks),
requiredInclusionBlockDepth: stakingParams.ConfirmationTimeBlocks,
}

app.wg.Add(1)
Expand Down Expand Up @@ -742,7 +742,7 @@ func (app *StakerApp) checkTransactionsStatus() error {
inclusionBlock: details.Block,
inclusionProof: iclusionProof,
},
requiredInclusionBlockDepth: uint64(stakingParams.ConfirmationTimeBlocks),
requiredInclusionBlockDepth: stakingParams.ConfirmationTimeBlocks,
}

app.wg.Add(1)
Expand Down Expand Up @@ -1363,7 +1363,7 @@ func (app *StakerApp) handleStakingEvents() {
req := &sendDelegationRequest{
txHash: ev.stakingTxHash,
inclusionInfo: nil,
requiredInclusionBlockDepth: uint64(ev.requiredDepthOnBtcChain),
requiredInclusionBlockDepth: ev.requiredDepthOnBtcChain,
}

storedTx, stakerAddress := app.mustGetTransactionAndStakerAddress(&ev.stakingTxHash)
Expand Down Expand Up @@ -1516,7 +1516,7 @@ func (app *StakerApp) handleStakingEvents() {
inclusionBlock: ev.inlusionBlock,
inclusionProof: proof,
},
requiredInclusionBlockDepth: uint64(ev.blockDepth),
requiredInclusionBlockDepth: ev.blockDepth,
}

storedTx, stakerAddress := app.mustGetTransactionAndStakerAddress(&ev.stakingTxHash)
Expand Down Expand Up @@ -2135,7 +2135,7 @@ func (app *StakerApp) ListActiveFinalityProviders(limit uint64, offset uint64) (
return app.babylonClient.QueryFinalityProviders(limit, offset)
}

// Initiates whole unbonding process. Whole process looks like this:
// UnbondStaking initiates whole unbonding process. Whole process looks like this:
// 1. Unbonding data is build based on exsitng staking transaction data
// 2. Unbonding data is sent to babylon as part of undelegete request
// 3. If request is successful, unbonding transaction is registred in db and
Expand All @@ -2146,7 +2146,7 @@ func (app *StakerApp) ListActiveFinalityProviders(limit uint64, offset uint64) (
// This function returns control to the caller after step 3. Later is up to the caller
// to check what is state of unbonding transaction
func (app *StakerApp) UnbondStaking(
stakingTxHash chainhash.Hash, feeRate *btcutil.Amount) (*chainhash.Hash, error) {
stakingTxHash chainhash.Hash, _ *btcutil.Amount) (*chainhash.Hash, error) {
// check we are not shutting down
select {
case <-app.quit:
Expand Down
Loading

0 comments on commit 16687b8

Please sign in to comment.