Skip to content

Commit

Permalink
Remove pkg/coordinator package (#3659)
Browse files Browse the repository at this point in the history
Here we remove the `pkg/coordinator` by moving their contents to
`pkg/maintainer/wallet`. This package seems to be no longer needed so
this operation makes the package structure and dependency graph simpler.
This refactoring was done in several steps:

### Step 1: Make the nomenclature in `pkg/maintainer/wallet` more
precise

The tBTC v2 system defines two types of sweeps: deposit sweeps and moved
funds sweeps. In the first step, we narrowed the existing nomenclature
used within the wallet maintainer to remove ambiguity and make it clear
that the existing code refers to deposit sweeps.

### Step 2: Move the deposit sweep code from `pkg/coordinator` to
`pkg/maintainer/wallet` package

In the second step, we moved the deposit sweep code living so far in the
`pkg/coordinator`. By the way, some
simplifications were made in order to make the public API cleaner and
more readable. After this change, the `pkg/maintainer/wallet` package
exposes the following public API:
- `FindDeposits` that allows to find deposits according to the given
criteria
- `FindDepositsToSweep` that finds a batch of deposits that are most
suitable to become part of a deposit sweep
- `ProposeDepositsSweep` which submits a deposit sweep proposal to the
`WalletCoordinator` contract
- `EstimateDepositsSweepFee` that estimates the deposit sweep
transaction fee
- `DepositReference` which is a set of data allowing to identify and
refer to a deposit
- `Deposit` that holds some detailed data about a deposit
- `ParseDepositsReferences` that allows creating an array of
`DepositReference` based on the provided input string
- `ValidateDepositReferenceString` that allows to validate whether the
given string can be used to create a valid `DepositReference` instance

This step also involved moving unit tests stressing the deposit sweep
code described above.

### Step 3: Move the redemption code from `pkg/coordinator` to
`pkg/maintainer/wallet` package

In this step, we moved the redemption code from `pkg/coordinator` to the
`pkg/maintainer/wallet` package. This was similar to the previous step
but much simpler. After this step, the public API of the
`pkg/maintainer/wallet` package was extended with the following items:
- `FindPendingRedemptions` which allows to find pending redemption
requests
- `ProposeRedemption` which submits a redemption proposal to the
`WalletCoordinator` contract
- `EstimateRedemptionFee` that estimates the redemption transaction fee

This step also involved moving unit tests stressing the redemption code
described above.

### Step 4: Move the `coordinator.Chain` interface

In the fourth step, we moved the `coordinator.Chain` interface by
merging it with the `maintainer/wallet.Chain` interface.
This change clarifies the chain expectations defined by the
`pkg/maintainer/wallet` package. We also moved the remaining chain types
living so far in the `pkg/coordinator` (`coordinator.
NewWalletRegisteredEvent` and `coordinator.
NewWalletRegisteredEventFilter`) to the `pkg/tbtc` package. This package
should be the right place for them according to the recent discussion
(see
#3650 (comment)),
at least until #3652 is
done.

### Step 5: Remove `pkg/coordinator` package

In the last step, we removed the `pkg/coordinator` package and made sure
all references from the `cmd` package were correctly replaced.
  • Loading branch information
pdyraga authored Jun 29, 2023
2 parents f48d676 + 5ee2d46 commit f6f1982
Show file tree
Hide file tree
Showing 36 changed files with 1,498 additions and 1,625 deletions.
239 changes: 222 additions & 17 deletions cmd/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ package cmd

import (
"fmt"
"os"
"regexp"
"sort"
"strconv"
"text/tabwriter"

"github.com/spf13/cobra"

Expand All @@ -10,8 +15,8 @@ import (
"github.com/keep-network/keep-core/pkg/bitcoin"
"github.com/keep-network/keep-core/pkg/bitcoin/electrum"
"github.com/keep-network/keep-core/pkg/chain/ethereum"
"github.com/keep-network/keep-core/pkg/coordinator"
"github.com/keep-network/keep-core/pkg/maintainer/spv"
walletmtr "github.com/keep-network/keep-core/pkg/maintainer/wallet"
)

var (
Expand Down Expand Up @@ -82,7 +87,10 @@ var listDepositsCommand = cobra.Command{
return fmt.Errorf("failed to find head flag: %v", err)
}

_, tbtcChain, _, _, _, err := ethereum.Connect(ctx, clientConfig.Ethereum)
_, tbtcChain, _, _, _, err := ethereum.Connect(
ctx,
clientConfig.Ethereum,
)
if err != nil {
return fmt.Errorf(
"could not connect to Ethereum chain: [%v]",
Expand All @@ -100,28 +108,76 @@ var listDepositsCommand = cobra.Command{
var err error
walletPublicKeyHash, err = newWalletPublicKeyHash(wallet)
if err != nil {
return fmt.Errorf("failed to extract wallet public key hash: %v", err)
return fmt.Errorf(
"failed to extract wallet public key hash: %v",
err,
)
}
}

return coordinator.ListDeposits(
deposits, err := walletmtr.FindDeposits(
tbtcChain,
btcChain,
walletPublicKeyHash,
head,
hideSwept,
false,
)
if err != nil {
return fmt.Errorf(
"failed to get deposits: [%w]",
err,
)
}

if len(deposits) == 0 {
return fmt.Errorf("no deposits found")
}

if err := printDepositsTable(deposits); err != nil {
return fmt.Errorf("failed to print deposits table: %v", err)
}

return nil
},
}

func printDepositsTable(deposits []*walletmtr.Deposit) error {
w := tabwriter.NewWriter(os.Stdout, 2, 4, 1, ' ', tabwriter.AlignRight)
fmt.Fprintf(w, "index\twallet\tvalue (BTC)\tdeposit key\trevealed deposit data\tconfirmations\tswept\t\n")

for i, deposit := range deposits {
fmt.Fprintf(w, "%d\t%s\t%.5f\t%s\t%s\t%d\t%t\t\n",
i,
hexutils.Encode(deposit.WalletPublicKeyHash[:]),
deposit.AmountBtc,
deposit.DepositKey,
fmt.Sprintf(
"%s:%d:%d",
deposit.FundingTxHash.Hex(bitcoin.ReversedByteOrder),
deposit.FundingOutputIndex,
deposit.RevealBlock,
),
deposit.Confirmations,
deposit.IsSwept,
)
}

if err := w.Flush(); err != nil {
return fmt.Errorf("failed to flush the writer: %v", err)
}

return nil
}

var proposeDepositsSweepCommand = cobra.Command{
Use: "propose-deposits-sweep",
Short: "propose deposits sweep",
Long: proposeDepositsSweepCommandDescription,
TraverseChildren: true,
Args: func(cmd *cobra.Command, args []string) error {
for i, arg := range args {
if err := coordinator.ValidateDepositString(arg); err != nil {
if err := validateDepositReferenceString(arg); err != nil {
return fmt.Errorf(
"argument [%d] failed validation: %v",
i,
Expand Down Expand Up @@ -183,14 +239,14 @@ var proposeDepositsSweepCommand = cobra.Command{
}
}

var deposits []*coordinator.DepositSweepDetails
var deposits []*walletmtr.DepositReference
if len(args) > 0 {
deposits, err = coordinator.ParseDepositsToSweep(args)
deposits, err = parseDepositsReferences(args)
if err != nil {
return fmt.Errorf("failed extract wallet public key hash: %v", err)
}
} else {
walletPublicKeyHash, deposits, err = coordinator.FindDepositsToSweep(
walletPublicKeyHash, deposits, err = walletmtr.FindDepositsToSweep(
tbtcChain,
btcChain,
walletPublicKeyHash,
Expand All @@ -209,7 +265,7 @@ var proposeDepositsSweepCommand = cobra.Command{
)
}

return coordinator.ProposeDepositsSweep(
return walletmtr.ProposeDepositsSweep(
tbtcChain,
btcChain,
walletPublicKeyHash,
Expand All @@ -220,12 +276,93 @@ var proposeDepositsSweepCommand = cobra.Command{
},
}

var proposeDepositsSweepCommandDescription = `Submits a deposits sweep proposal to
the chain.
Expects --wallet and --fee flags along with deposits to sweep provided
as arguments.
var (
depositsFormatDescription = "Deposits details should be provided as strings containing: \n" +
" - bitcoin transaction hash (unprefixed bitcoin transaction hash in reverse (RPC) order), \n" +
" - bitcoin transaction output index, \n" +
" - ethereum block number when the deposit was revealed to the chain. \n" +
"The properties should be separated by semicolons, in the following format: \n" +
depositReferenceFormatPattern + "\n" +
"e.g. bd99d1d0a61fd104925d9b7ac997958aa8af570418b3fde091f7bfc561608865:1:8392394"

depositReferenceFormatPattern = "<unprefixed bitcoin transaction hash>:" +
"<bitcoin transaction output index>:" +
"<ethereum reveal block number>"

depositReferenceFormatRegexp = regexp.MustCompile(`^([[:xdigit:]]+):(\d+):(\d+)$`)

proposeDepositsSweepCommandDescription = "Submits a deposits sweep proposal " +
"to the chain. Expects --wallet and --fee flags along with deposits to " +
"sweep provided as arguments.\n" +
depositsFormatDescription
)

// parseDepositsReferences decodes a list of deposits references.
func parseDepositsReferences(
depositsRefsStings []string,
) ([]*walletmtr.DepositReference, error) {
depositsRefs := make([]*walletmtr.DepositReference, len(depositsRefsStings))

for i, depositRefString := range depositsRefsStings {
matched := depositReferenceFormatRegexp.FindStringSubmatch(depositRefString)
// Check if number of resolved entries match expected number of groups
// for the given regexp.
if len(matched) != 4 {
return nil, fmt.Errorf(
"failed to parse deposit: [%s]",
depositRefString,
)
}

txHash, err := bitcoin.NewHashFromString(matched[1], bitcoin.ReversedByteOrder)
if err != nil {
return nil, fmt.Errorf(
"invalid bitcoin transaction hash [%s]: %v",
matched[1],
err,
)
}

outputIndex, err := strconv.ParseInt(matched[2], 10, 32)
if err != nil {
return nil, fmt.Errorf(
"invalid bitcoin transaction output index [%s]: %v",
matched[2],
err,
)
}

revealBlock, err := strconv.ParseUint(matched[3], 10, 32)
if err != nil {
return nil, fmt.Errorf(
"invalid reveal block number [%s]: %v",
matched[3],
err,
)
}

depositsRefs[i] = &walletmtr.DepositReference{
FundingTxHash: txHash,
FundingOutputIndex: uint32(outputIndex),
RevealBlock: revealBlock,
}
}

return depositsRefs, nil
}

` + coordinator.DepositsFormatDescription
// validateDepositReferenceString validates format of the string containing a
// deposit reference.
func validateDepositReferenceString(depositRefString string) error {
if !depositReferenceFormatRegexp.MatchString(depositRefString) {
return fmt.Errorf(
"[%s] doesn't match pattern: %s",
depositRefString,
depositReferenceFormatPattern,
)
}
return nil
}

var proposeRedemptionCommand = cobra.Command{
Use: "propose-redemption",
Expand Down Expand Up @@ -282,7 +419,7 @@ var proposeRedemptionCommand = cobra.Command{
}
}

walletPublicKeyHash, redemptions, err := coordinator.FindPendingRedemptions(
walletPublicKeyHash, redemptions, err := walletmtr.FindPendingRedemptions(
tbtcChain,
walletPublicKeyHash,
redemptionMaxSize,
Expand All @@ -299,7 +436,7 @@ var proposeRedemptionCommand = cobra.Command{
)
}

return coordinator.ProposeRedemption(
return walletmtr.ProposeRedemption(
tbtcChain,
btcChain,
walletPublicKeyHash,
Expand Down Expand Up @@ -336,12 +473,80 @@ var estimateDepositsSweepFeeCommand = cobra.Command{
return fmt.Errorf("could not connect to Electrum chain: [%v]", err)
}

return coordinator.EstimateDepositsSweepFee(
fees, err := walletmtr.EstimateDepositsSweepFee(
tbtcChain,
btcChain,
depositsCount,
)
if err != nil {
return fmt.Errorf("cannot estimate deposits sweep fee: [%v]", err)
}

err = printDepositsSweepFeeTable(fees)
if err != nil {
return fmt.Errorf("cannot print fees table: [%v]", err)
}

return nil
},
}

// printDepositsSweepFeeTable prints estimated fees for specific deposits counts
// to the standard output. For example:
//
// ---------------------------------------------
// deposits count total fee (satoshis) sat/vbyte
// 1 201 1
// 2 292 1
// 3 384 1
// ---------------------------------------------
func printDepositsSweepFeeTable(
fees map[int]struct {
TotalFee int64
SatPerVByteFee int64
},
) error {
writer := tabwriter.NewWriter(
os.Stdout,
2,
4,
1,
' ',
tabwriter.AlignRight,
)

_, err := fmt.Fprintf(writer, "deposits count\ttotal fee (satoshis)\tsat/vbyte\t\n")
if err != nil {
return err
}

var depositsCountKeys []int
for depositsCountKey := range fees {
depositsCountKeys = append(depositsCountKeys, depositsCountKey)
}

sort.Slice(depositsCountKeys, func(i, j int) bool {
return depositsCountKeys[i] < depositsCountKeys[j]
})

for _, depositsCountKey := range depositsCountKeys {
_, err := fmt.Fprintf(
writer,
"%v\t%v\t%v\t\n",
depositsCountKey,
fees[depositsCountKey].TotalFee,
fees[depositsCountKey].SatPerVByteFee,
)
if err != nil {
return err
}
}

if err := writer.Flush(); err != nil {
return fmt.Errorf("failed to flush the writer: %v", err)
}

return nil
}

var estimateDepositsSweepFeeCommandDescription = "Estimates the satoshi " +
Expand Down
6 changes: 3 additions & 3 deletions cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,9 +343,9 @@ func initMaintainerFlags(command *cobra.Command, cfg *config.Config) {
)

command.Flags().DurationVar(
&cfg.Maintainer.WalletCoordination.SweepInterval,
"walletCoordination.sweepInterval",
wallet.DefaultSweepInterval,
&cfg.Maintainer.WalletCoordination.DepositSweepInterval,
"walletCoordination.depositSweepInterval",
wallet.DefaultDepositSweepInterval,
"The time interval in which unswept deposits are checked.",
)

Expand Down
6 changes: 3 additions & 3 deletions cmd/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,9 @@ var cmdFlagsTests = map[string]struct {
expectedValueFromFlag: 7 * time.Hour,
defaultValue: 3 * time.Hour,
},
"maintainer.walletCoordination.sweepInterval": {
readValueFunc: func(c *config.Config) interface{} { return c.Maintainer.WalletCoordination.SweepInterval },
flagName: "--walletCoordination.sweepInterval",
"maintainer.walletCoordination.depositSweepInterval": {
readValueFunc: func(c *config.Config) interface{} { return c.Maintainer.WalletCoordination.DepositSweepInterval },
flagName: "--walletCoordination.depositSweepInterval",
flagValue: "35h",
expectedValueFromFlag: 35 * time.Hour,
defaultValue: 48 * time.Hour,
Expand Down
4 changes: 2 additions & 2 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ func TestReadConfigFromFile(t *testing.T) {
readValueFunc: func(c *Config) interface{} { return c.Maintainer.WalletCoordination.RedemptionInterval },
expectedValue: 13 * time.Hour,
},
"Maintainer.WalletCoordination.SweepInterval": {
readValueFunc: func(c *Config) interface{} { return c.Maintainer.WalletCoordination.SweepInterval },
"Maintainer.WalletCoordination.DepositSweepInterval": {
readValueFunc: func(c *Config) interface{} { return c.Maintainer.WalletCoordination.DepositSweepInterval },
expectedValue: 64 * time.Hour,
},
"Maintainer.Spv.Enabled": {
Expand Down
Loading

0 comments on commit f6f1982

Please sign in to comment.