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

CLI command to list tBTC deposits #3545

Merged
merged 12 commits into from
May 8, 2023
1 change: 1 addition & 0 deletions cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func init() {
PingCommand,
EthereumCommand,
MaintainerCommand,
WalletCommand,
)
}

Expand Down
100 changes: 100 additions & 0 deletions cmd/wallet.go
lukasz-zimnoch marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package cmd

import (
"fmt"

"github.com/spf13/cobra"

walletcmd "github.com/keep-network/keep-core/cmd/wallet"
"github.com/keep-network/keep-core/config"
"github.com/keep-network/keep-core/pkg/chain/ethereum"
)

var (
walletFlagName = "wallet"
lukasz-zimnoch marked this conversation as resolved.
Show resolved Hide resolved
hideSweptFlagName = "hide-swept"
sortByAmountFlagName = "sort-amount"
)

// WalletCommand contains the definition of tBTC wallets tools.
var WalletCommand = &cobra.Command{
Use: "wallet",
Short: "tBTC wallets tools",
Long: "The tool exposes commands for interactions with tBTC wallets.",
TraverseChildren: true,
PersistentPreRun: func(cmd *cobra.Command, args []string) {
if err := clientConfig.ReadConfig(
configFilePath,
cmd.Flags(),
config.General, config.Ethereum, config.BitcoinElectrum,
); err != nil {
logger.Fatalf("error reading config: %v", err)
}
},
}

var depositsCommand = cobra.Command{
Use: "deposits",
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this command to list-wallet-deposits? This will make it more specific.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to keep the command name as short and simple as possible.

Copy link
Member

Choose a reason for hiding this comment

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

I meant something similar to what I mentioned in #3545 (comment). In this case, deposits are also ubiquitous, and making it at least list-deposits would make the command clear at first sight, even without referring to its description.

Copy link
Member Author

Choose a reason for hiding this comment

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

There will be just two commands: one to list the deposits and another to submit a deposit sweep proposal. The commands will be used by a handful of development team members, so I don't think there will be any ambiguity in the property naming, compared to the fact that the right transactions and fees will have to be determined by the caller of the sweep proposal command.
The reason why I want to keep all the commands and flags short is that if we combine them with arguments we will get long lines to handle in the CLI, e.g.:

keep-client wallet-coordinator propose-deposit-sweep --wallet-pkh 0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e --fee 12 bd99d1d0a61fd104925d9b7ac997958aa8af570418b3fde091f7bfc561608865:1 178628f0fd77ca04a31e771805277405288404da38eb86a0e369f2b26d45fe97:1 b71c0bbe43f23e162739b80b7bae9f0fb650e90e56fea0b92f169553505c398d:1 8356227b87fb191d61cff2455cd3f8c33ae208c69a7698b5baf1a331db6d4d8f:1 63cb18fdbe4ceb8b0747dbfcb11989605f78cffbe4379b9047124118cfc20aef:0

Copy link
Member

Choose a reason for hiding this comment

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

Eventually, the wallet coordinator will manage redemptions, moving funds, and sweeps of moved funds. Most likely, the CLI will have to handle them as well. This is why I'm flagging the need to establish a clear and consistent API of the CLI from the beginning. I'm afraid that we won't be able to keep them short in the long term so it seems that it's better to use a predictable pattern from the beginning. For example, sweeping refers to both deposits and moved funds so we can't just have a single 'sweep' command.

Copy link
Member

Choose a reason for hiding this comment

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

I would also vote for list-deposits. We will have more actions in the coordinator soon and list-deposits is in my opinion short enough and unique to not confuse it with anything else.

The actions we'll have to implement in the coordinator:

... then all the actions related to moving funds. I think it would also be nice to allow submitting heartbeat requests via the coordinator CLI.

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussion continued in #3549 (comment)

Short: "get deposits",
Long: "Gets tBTC deposits details from the chain and prints them.",
TraverseChildren: true,
RunE: func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()

wallet, err := cmd.Flags().GetString(walletFlagName)
if err != nil {
return fmt.Errorf("failed to find wallet flag: %v", err)
}

hideSwept, err := cmd.Flags().GetBool(hideSweptFlagName)
if err != nil {
return fmt.Errorf("failed to find show swept flag: %v", err)
}

sortByAmount, err := cmd.Flags().GetBool(sortByAmountFlagName)
if err != nil {
return fmt.Errorf("failed to find show swept flag: %v", err)
}

_, tbtcChain, _, _, _, err := ethereum.Connect(ctx, clientConfig.Ethereum)
if err != nil {
return fmt.Errorf(
"could not connect to Bitcoin difficulty chain: [%v]",
err,
)
}

return walletcmd.ListDeposits(tbtcChain, wallet, hideSwept, sortByAmount)
},
}

func init() {
initFlags(
WalletCommand,
&configFilePath,
clientConfig,
config.General, config.Ethereum, config.BitcoinElectrum,
)

// Wallet Command
WalletCommand.PersistentFlags().String(
walletFlagName,
"",
"wallet public key hash",
)

// Deposits Subcommand
depositsCommand.Flags().Bool(
hideSweptFlagName,
false,
"hide swept deposits",
)

depositsCommand.Flags().Bool(
sortByAmountFlagName,
false,
"sort by deposit amount",
)

WalletCommand.AddCommand(&depositsCommand)
}
189 changes: 189 additions & 0 deletions cmd/wallet/deposits.go
lukasz-zimnoch marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
package walletcmd

import (
"encoding/binary"
"encoding/hex"
"fmt"
"os"
"sort"
"text/tabwriter"

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/crypto"

"github.com/keep-network/keep-core/pkg/bitcoin"
"github.com/keep-network/keep-core/pkg/tbtc"
)

type depositEntry struct {
wallet [20]byte
lukasz-zimnoch marked this conversation as resolved.
Show resolved Hide resolved

depositKey string
revealedAtBlock uint64
lukasz-zimnoch marked this conversation as resolved.
Show resolved Hide resolved
isSwept bool

fundingTransactionHash bitcoin.Hash
fundingTransactionOutputIndex uint32
amountBtc float64
}

// ListDeposits gets deposits from the chain.
func ListDeposits(
tbtcChain tbtc.Chain,
walletStr string,
pdyraga marked this conversation as resolved.
Show resolved Hide resolved
hideSwept bool,
sortByAmount bool,
) error {
deposits, err := getDeposits(tbtcChain, walletStr)
if err != nil {
return fmt.Errorf(
"failed to get deposits: [%w]",
err,
)
}

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

// Filter
if hideSwept {
deposits = removeSwept(deposits)
}

// Order
sort.SliceStable(deposits, func(i, j int) bool {
return deposits[i].revealedAtBlock > deposits[j].revealedAtBlock
})

if sortByAmount {
sort.SliceStable(deposits, func(i, j int) bool {
return deposits[i].amountBtc < deposits[j].amountBtc
})
}

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

return nil
}

func removeSwept(deposits []depositEntry) []depositEntry {
result := []depositEntry{}
for _, deposit := range deposits {
if deposit.isSwept {
continue
}
result = append(result, deposit)
}
return result
}

func getDeposits(tbtcChain tbtc.Chain, walletStr string) ([]depositEntry, error) {
logger.Infof("reading deposits from chain...")

filter := &tbtc.DepositRevealedEventFilter{}
if len(walletStr) > 0 {
walletPublicKeyHash, err := hexToWalletPublicKeyHash(walletStr)
if err != nil {
return []depositEntry{}, fmt.Errorf("failed to extract wallet public key hash: %v", err)
}

filter.WalletPublicKeyHash = [][20]byte{walletPublicKeyHash}
}

depositRevealedEvent, err := tbtcChain.PastDepositRevealedEvents(filter)
Copy link
Member

Choose a reason for hiding this comment

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

Did you manage to test it on mainnet with the oldest wallet? Did you try calling it against geth server which is not Infura/Alchemy? If not, can you please perform such a test? We had a discussion with @lukasz-zimnoch on how long it takes to retrieve old events without passing a block range. I expect Alchemy and Infura may have some fancier cache/indexing but does it work fast enough with a standard geth? Knowing this would help us determine how fancy we need to be with calls like that and this simple CLI command is a perfect opportunity.

Copy link
Member Author

Choose a reason for hiding this comment

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

Get All Deposits (no filter):

Mainnet:
Alchemy: 0.205702 seconds
Infura: 0.523039 seconds
Geth: 265.665695 seconds

Get Wallet's Deposits (with wallet filter):

Mainnet:
Alchemy: 0.186728 seconds
Infura: 0.271587 seconds
Geth: 114.994290 seconds

Copy link
Member

Choose a reason for hiding this comment

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

As expected but reassuring our design decisions were correct. Thanks.

if err != nil {
return []depositEntry{}, fmt.Errorf(
"failed to get past deposit revealed events: [%w]",
err,
)
}

logger.Infof("found %d DepositRevealed events", len(depositRevealedEvent))

result := make([]depositEntry, len(depositRevealedEvent))
for i, event := range depositRevealedEvent {
logger.Debugf("getting details of deposit %d/%d", i+1, len(depositRevealedEvent))

depositKey := buildDepositKey(event.FundingTxHash, event.FundingOutputIndex)

depositRequest, err := tbtcChain.GetDepositRequest(event.FundingTxHash, event.FundingOutputIndex)
if err != nil {
return result, fmt.Errorf(
"failed to get deposit request: [%w]",
err,
)
}

result[i] = depositEntry{
pdyraga marked this conversation as resolved.
Show resolved Hide resolved
wallet: event.WalletPublicKeyHash,
depositKey: depositKey,
revealedAtBlock: event.BlockNumber,
isSwept: depositRequest.SweptAt.After(depositRequest.RevealedAt),
lukasz-zimnoch marked this conversation as resolved.
Show resolved Hide resolved
fundingTransactionHash: event.FundingTxHash,
fundingTransactionOutputIndex: event.FundingOutputIndex,
amountBtc: convertSatToBtc(float64(depositRequest.Amount)),
}
}

return result, nil
}

func printTable(deposits []depositEntry) error {
w := tabwriter.NewWriter(os.Stdout, 2, 4, 1, ' ', tabwriter.AlignRight)
fmt.Fprintf(w, "index\twallet\tvalue (BTC)\tdeposit key\tfunding transaction\tswept\t\n")

for i, deposit := range deposits {
fmt.Fprintf(w, "%d\t%s\t%.5f\t%s\t%s\t%v\t\n",
i,
"0x"+hex.EncodeToString(deposit.wallet[:]),
deposit.amountBtc,
deposit.depositKey,
fmt.Sprintf(
"%s:%d",
deposit.fundingTransactionHash.Hex(bitcoin.ReversedByteOrder),
deposit.fundingTransactionOutputIndex,
),
deposit.isSwept,
)
}

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

return nil
}

func hexToWalletPublicKeyHash(str string) ([20]byte, error) {
pdyraga marked this conversation as resolved.
Show resolved Hide resolved
walletHex, err := hexutil.Decode(str)
lukasz-zimnoch marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return [20]byte{}, fmt.Errorf("failed to parse arguments: %w", err)
}

var walletPublicKeyHash [20]byte
copy(walletPublicKeyHash[:], walletHex)

return walletPublicKeyHash, nil
}

func buildDepositKey(
fundingTxHash bitcoin.Hash,
fundingOutputIndex uint32,
) string {
fundingOutputIndexBytes := make([]byte, 4)
binary.BigEndian.PutUint32(fundingOutputIndexBytes, fundingOutputIndex)

depositKey := crypto.Keccak256Hash(
append(fundingTxHash[:], fundingOutputIndexBytes...),
)

return depositKey.Hex()
}
lukasz-zimnoch marked this conversation as resolved.
Show resolved Hide resolved

func convertSatToBtc(sats float64) float64 {
return sats / float64(100000000)
}
7 changes: 7 additions & 0 deletions cmd/wallet/walletcmd.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package walletcmd

import (
"github.com/ipfs/go-log"
)

var logger = log.Logger("keep-wallet-cmd")