CLI command to list tBTC deposits #3545

nkuba · 2023-05-02T12:18:42Z

We added a CLI command to list the tBTC deposits. The command queries the chain for revealed deposits and outputs their details to stdout.

To list all deposits run:

keep-client coordinator deposits

Following flags can be used with the command:

--wallet <address> - filter deposits only for a given wallet
--hide-swept - doesn't show deposits that were already swept
--sort-amount - sort output by the deposit amount
--head <number> - show <number> first deposits
--tail <number> - show <number> last deposits

The command requires ethereum and bitcoin node details to be provided in the config.

Funding Transaction

The command outputs the Bitcoin funding transaction details in a specific format:
<transaction hash>:<output index> (e.g. a3d1781b59d5e8680772a8bb7f897c4ff0459d3465d7fa678f80a4f0ec900574:1). The same format will be required by another CLI command to propose deposits sweep.

Example

Sample execution of the command for Goerli network:

✗ keep-client --config ./configs/forked/config.toml --goerli \
  coordinator \
  deposits \
  --wallet 0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e \
  --sort-amount \
  --head 5

Output:

 index                                     wallet value (BTC)                                                        deposit key                                                funding transaction confirmations swept
     0 0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e     0.01000 0x06cdc15cbf3f024d05d164b9fcabfe3a6af7180b20ed2b685cf3a2241426667f c0a91a81b401aae3bc639ef24bc686e1b59b83472a9b571510c34c03cc60414a:0             0 false
     1 0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e     0.01000 0xc0612aed21365628908474f634a60124eb4ab3440f133602aaedb06310a6275d 63cb18fdbe4ceb8b0747dbfcb11989605f78cffbe4379b9047124118cfc20aef:0         14212 false
     2 0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e     0.01000 0xc77584f754d98abef75223e7dadfc02484c87052670de6038514a9b951e94156 46a9b50dd44f11364d9924bd7995830cd7cd1a772910f22ed7ab507137653c6d:0         14199 false
     3 0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e     0.01000 0x89835e76759b3e0f2ca5057c0738d6f792083be4aaf8e064d9733720f9973e0c d07564117859bf7b3ee02bd6659d846d1ea5c4b7c204295ab1270948be615047:0         14052 false
     4 0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e     0.01000 0x78cd69a78750f31d49a4693281a97448581361c7e93a9a70ea3a4931199b6c87 8356227b87fb191d61cff2455cd3f8c33ae208c69a7698b5baf1a331db6d4d8f:1         13981 false

We added a CLI command to list the tBTC deposits. The command queries the chain for revealed deposits and outputs their details to stdout. To list all deposits run: ``` keep-client wallet deposits ``` Following flags can be used with the command: - `--wallet <address>` - filter deposits only for given wallet - `--hide-swept` - doesn't show deposits that were already swept - `--sort-amount` - sort output by the deposit amount The command requires `ethereum` node details to be provided in the config.

go scan was complaining about unhandled error

cmd/wallet/deposits.go

pdyraga · 2023-05-04T21:06:47Z

cmd/wallet/deposits.go

+		filter.WalletPublicKeyHash = [][20]byte{walletPublicKeyHash}
+	}
+
+	depositRevealedEvent, err := tbtcChain.PastDepositRevealedEvents(filter)


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.

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

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

cmd/wallet/deposits.go

cmd/wallet.go

cmd/wallet/deposits.go

lukasz-zimnoch · 2023-05-05T08:06:31Z

cmd/wallet.go

+}
+
+var depositsCommand = cobra.Command{
+	Use:              "deposits",


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

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

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.

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

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.

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:

list deposits (this PR)

sweep (CLI command to submit a deposit sweep proposal #3549)

increase sweep fee

list redemptions (will also have to be automated)

redeem (will also have to be automated)

increase redemption fee

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

Discussion continued in #3549 (comment)

Get number of bitcoin confirmations for the given transaction and output it in the table.

The flags can be used to query specific number of top or bottom deposits. By default deposits are sorted by the creation block, so with `--head X` user can select X oldest deposits and with `--tail Y` get Y newest deposits. When `--sort-amount --head X` flags are used only X smalest deposits are returned and with `--sort-amount --tail Y` only Y biggest deposits are returned.

The function can be used externally to build the deposit key based on the rules specific for the given chain implementation.

We define hexutils pacakge with Decode and Encode functions that are based on https://pkg.go.dev/github.com/ethereum/go-ethereum/common/hexutil package.

pdyraga · 2023-05-08T14:56:00Z

pkg/internal/hexutils/hexutils.go

+)
+
+// Decode decodes a hex string with 0x prefix.
+func Decode(input string) ([]byte, error) {


Non-blocking of course but just noting we could move this code to keep-common at some point and use it here.

Side note: we can use strings.TrimPrefix to do prefix removal.

We would need to trim 0x and 0X, so I'm not sure which approach would be better.

pdyraga

Tested locally and works as expected. There are some ongoing discussions but I find them non-blocking, so let's address them in #3549.

pdyraga · 2023-05-08T14:58:00Z

pkg/internal/hexutils/hexutils_test.go

+			if !reflect.DeepEqual(err, test.wantErr) {
+				t.Fatalf("unexpected error\nexpected: %v\nactual:   %v", test.wantErr, err)
+			}
+			if !bytes.Equal(test.want, dec) {


We could use testutils.AssertBytesEqual here.

pdyraga · 2023-05-08T15:15:20Z

cmd/wallet.go

+}
+
+var depositsCommand = cobra.Command{
+	Use:              "deposits",


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:

list deposits (this PR)

sweep (CLI command to submit a deposit sweep proposal #3549)

increase sweep fee

list redemptions (will also have to be automated)

redeem (will also have to be automated)

increase redemption fee

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

We added a CLI command to submit a deposit sweep proposal. The command verifies the proposal with the same rules as nodes will do and submits the proposal to the chain. To submit a deposit sweep proposal run: ``` keep-client coordinator propose-deposits-sweep --wallet <wallet_address> --fee <fee> <deposit_1> <deposit_2> ... ``` Following flags can be used with the command: - `--wallet <address>` - wallet to sweep - `--fee` - fee to use for the sweep transaction The command requires `ethereum` and `bitcoin.electrum` node details to be provided in the config. ### Deposits The command requires the deposits details to be provided as variadic arguments in a specific format: `<unprefixed bitcoin transaction hash>:<bitcoin transaction output index>:<ethereum reveal block number>` e.g. `bd99d1d0a61fd104925d9b7ac997958aa8af570418b3fde091f7bfc561608865:1:8392394` ## Sample command ``` keep-client --config ./configs/forked/config.toml \ --goerli \ coordinator \ propose-deposits-sweep \ --wallet 0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e \ --fee 12 \ da3ec4f5620f686d5013c37d49a411fa49daac5077c98809402572f481c6ea0:0:8502238 \ 214a85db0871b537d4cd649a8d74aecdbee112919983ad22fb85cfa2704a3593:0:8502224 \ 3704d01b48212ddacc77e613be0491978a8c19ad11af6b8536eacf9e9953eb13:0:8502070 ``` ## Testing ~To test this feature `WalletCoordinator` contract is required to be deployed. It is not yet available on Goerli or Mainnet. To have the contract available you can run a hardhat node as Goerli fork (https://github.com/nkuba/hardhat-forked-node) and deploy the `WalletCoordinator` contract there.~ Depends on: #3545

nkuba requested review from tomaszslabon, pdyraga and lukasz-zimnoch May 2, 2023 13:30

nkuba self-assigned this May 2, 2023

nkuba added the 📟 client label May 2, 2023

Handle w.Flush() error

f933ecb

go scan was complaining about unhandled error

nkuba mentioned this pull request May 4, 2023

CLI command to submit a deposit sweep proposal #3549

Merged

pdyraga reviewed May 4, 2023

View reviewed changes

lukasz-zimnoch requested changes May 5, 2023

View reviewed changes

lukasz-zimnoch reviewed May 5, 2023

View reviewed changes

nkuba added 3 commits May 5, 2023 11:36

Update variables naming

82b381c

Add TODO about bitcoin confirmations

3e13eec

Compare SweptAt with 0

fc1cf3a

nkuba requested review from lukasz-zimnoch and pdyraga May 5, 2023 11:23

nkuba added 7 commits May 8, 2023 13:06

Get Bitcoin transaction confirmations

4d28143

Get number of bitcoin confirmations for the given transaction and output it in the table.

Expose BuildDepositKey function

6e4e90a

The function can be used externally to build the deposit key based on the rules specific for the given chain implementation.

Move walletcmd to pkg/coordinator

2d10093

Define internal hexutils package

3f4e2b3

We define hexutils pacakge with Decode and Encode functions that are based on https://pkg.go.dev/github.com/ethereum/go-ethereum/common/hexutil package.

Rename command wallet to coordinator

a717cc2

Use append instead of copy

a52261a

pdyraga reviewed May 8, 2023

View reviewed changes

pdyraga approved these changes May 8, 2023

View reviewed changes

lukasz-zimnoch approved these changes May 8, 2023

View reviewed changes

lukasz-zimnoch merged commit 5566ba0 into main May 8, 2023

lukasz-zimnoch deleted the wallet-cli-deposits branch May 8, 2023 18:54

lukasz-zimnoch mentioned this pull request May 9, 2023

Sweeping prototype #3512

Closed

6 tasks

pdyraga added this to the v2.0.0-m3 milestone May 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI command to list tBTC deposits #3545

CLI command to list tBTC deposits #3545

nkuba commented May 2, 2023 •

edited

Loading

pdyraga May 4, 2023

nkuba May 5, 2023

pdyraga May 8, 2023

lukasz-zimnoch May 5, 2023

nkuba May 5, 2023

lukasz-zimnoch May 8, 2023

nkuba May 8, 2023

lukasz-zimnoch May 8, 2023

pdyraga May 8, 2023

nkuba May 9, 2023

pdyraga May 8, 2023

lukasz-zimnoch May 8, 2023

nkuba May 9, 2023

pdyraga left a comment

pdyraga May 8, 2023

nkuba May 9, 2023

pdyraga May 8, 2023

CLI command to list tBTC deposits #3545

CLI command to list tBTC deposits #3545

Conversation

nkuba commented May 2, 2023 • edited Loading

Funding Transaction

Example

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Get All Deposits (no filter):

Get Wallet's Deposits (with wallet filter):

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdyraga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkuba commented May 2, 2023 •

edited

Loading