Skip to content

Commit

Permalink
chore: Use int64 for satoshi amount configs (#51)
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 authored Oct 14, 2024
1 parent e136a5f commit 61ba909
Show file tree
Hide file tree
Showing 15 changed files with 121 additions and 114 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [#53](https://github.com/babylonlabs-io/btc-staker/pull/53) Use only quorum of
signatures when building unbonding transaction witness

### Misc Improvements

* [51](https://github.com/babylonlabs-io/btc-staker/pull/51) Use int64
for satoshi amount related values.

## v0.7.0

### Api breaking
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.22.3 as builder
FROM golang:1.22.3 AS builder

# Install cli tools for building and final image
RUN apt-get update && apt-get install -y make git bash gcc curl jq
Expand Down
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
18 changes: 1 addition & 17 deletions cmd/stakercli/daemon/daemoncommands.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const (
limitFlag = "limit"
fpPksFlag = "finality-providers-pks"
stakingTransactionHashFlag = "staking-transaction-hash"
feeRateFlag = "fee-rate"
stakerAddressFlag = "staker-address"
)

Expand Down Expand Up @@ -168,10 +167,6 @@ var unbondCmd = cli.Command{
Usage: "Hash of original staking transaction in bitcoin hex format",
Required: true,
},
cli.IntFlag{
Name: feeRateFlag,
Usage: "fee rate to pay for unbonding tx in sats/kb",
},
},
Action: unbond,
}
Expand Down Expand Up @@ -372,18 +367,7 @@ func unbond(ctx *cli.Context) error {

stakingTransactionHash := ctx.String(stakingTransactionHashFlag)

feeRate := ctx.Int(feeRateFlag)

if feeRate < 0 {
return cli.NewExitError("Fee rate must be non-negative", 1)
}

var fr *int = nil
if feeRate > 0 {
fr = &feeRate
}

result, err := client.UnbondStaking(sctx, stakingTransactionHash, fr)
result, err := client.UnbondStaking(sctx, stakingTransactionHash)
if err != nil {
return err
}
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
9 changes: 3 additions & 6 deletions itest/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1512,8 +1512,7 @@ func TestStakingUnbonding(t *testing.T) {

tm.waitForStakingTxState(t, txHash, proto.TransactionState_DELEGATION_ACTIVE)

feeRate := 2000
resp, err := tm.StakerClient.UnbondStaking(context.Background(), txHash.String(), &feeRate)
resp, err := tm.StakerClient.UnbondStaking(context.Background(), txHash.String())
require.NoError(t, err)

unbondingTxHash, err := chainhash.NewHashFromStr(resp.UnbondingTxHash)
Expand Down Expand Up @@ -1590,8 +1589,7 @@ func TestUnbondingRestartWaitingForSignatures(t *testing.T) {

tm.waitForStakingTxState(t, txHash, proto.TransactionState_DELEGATION_ACTIVE)

feeRate := 2000
unbondResponse, err := tm.StakerClient.UnbondStaking(context.Background(), txHash.String(), &feeRate)
unbondResponse, err := tm.StakerClient.UnbondStaking(context.Background(), txHash.String())
require.NoError(t, err)
unbondingTxHash, err := chainhash.NewHashFromStr(unbondResponse.UnbondingTxHash)
require.NoError(t, err)
Expand Down Expand Up @@ -1818,8 +1816,7 @@ func TestRecoverAfterRestartDuringWithdrawal(t *testing.T) {
tm.waitForStakingTxState(t, txHash, proto.TransactionState_DELEGATION_ACTIVE)

// Unbond staking transaction and wait for it to be included in mempool
feeRate := 2000
unbondResponse, err := tm.StakerClient.UnbondStaking(context.Background(), txHash.String(), &feeRate)
unbondResponse, err := tm.StakerClient.UnbondStaking(context.Background(), txHash.String())
require.NoError(t, err)
unbondingTxHash, err := chainhash.NewHashFromStr(unbondResponse.UnbondingTxHash)
require.NoError(t, err)
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
Loading

0 comments on commit 61ba909

Please sign in to comment.