Cli updates to release #432

sapinb · 2024-10-27T05:59:05Z

Description

Cherrypick cli related changes from main to release

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Refactor
New or updated tests

Checklist

I have performed a self-review of my code.
I have commented my code where necessary.
I have updated the documentation if needed.
My changes do not introduce new warnings.
I have added tests that prove my changes are effective or that my feature works.
New and existing tests pass with my changes.

Related Issues

codecov · 2024-10-27T06:17:08Z

Codecov Report

Attention: Patch coverage is 0% with 301 lines in your changes missing coverage. Please review.

Project coverage is 57.04%. Comparing base (a14d926) to head (c9886cd).
Report is 5 commits behind head on releases/0.1.0.

Files with missing lines	Patch %	Lines
bin/strata-cli/src/seed/password.rs	0.00%	53 Missing ⚠️
bin/strata-cli/src/signet.rs	0.00%	52 Missing ⚠️
bin/strata-cli/src/cmd/faucet.rs	0.00%	35 Missing ⚠️
bin/strata-cli/src/settings.rs	0.00%	23 Missing ⚠️
bin/strata-cli/src/seed.rs	0.00%	22 Missing ⚠️
bin/strata-cli/src/cmd/deposit.rs	0.00%	21 Missing ⚠️
bin/strata-cli/src/cmd/recover.rs	0.00%	17 Missing ⚠️
bin/strata-cli/src/main.rs	0.00%	17 Missing ⚠️
bin/strata-cli/src/cmd/send.rs	0.00%	15 Missing ⚠️
bin/strata-cli/src/cmd/drain.rs	0.00%	11 Missing ⚠️
... and 7 more

@@                Coverage Diff                 @@
##           releases/0.1.0     #432      +/-   ##
==================================================
- Coverage           57.16%   57.04%   -0.13%     
==================================================
  Files                 256      261       +5     
  Lines               27048    27638     +590     
==================================================
+ Hits                15462    15765     +303     
- Misses              11586    11873     +287

Files with missing lines	Coverage Δ
bin/strata-cli/src/cmd/mod.rs	`0.00% <ø> (ø)`
bin/strata-cli/src/cmd/reset.rs	`0.00% <ø> (ø)`
bin/strata-cli/src/constants.rs	`100.00% <ø> (ø)`
bin/strata-cli/src/net_type.rs	`0.00% <ø> (ø)`
bin/strata-client/src/extractor.rs	`96.52% <ø> (ø)`
bin/strata-cli/src/cmd/receive.rs	`0.00% <0.00%> (ø)`
bin/strata-cli/src/cmd/config.rs	`0.00% <0.00%> (ø)`
bin/strata-cli/src/cmd/change_pwd.rs	`0.00% <0.00%> (ø)`
bin/strata-cli/src/cmd/scan.rs	`0.00% <0.00%> (ø)`
bin/strata-cli/src/cmd/withdraw.rs	`0.00% <0.00%> (ø)`
... and 12 more

... and 19 files with indirect coverage changes

Zk2u

Wait for this to merge #423

storopoli · 2024-10-28T10:15:54Z

We also need to update the private-docs cargo install command

storopoli

We can bring #423 it's squash merged

storopoli · 2024-10-28T14:18:04Z

I've brought #423 in a743f65.
Let's wait for CI

storopoli

ACK a743f65

* feat(strata-cli)!: add hash version and optional password * feat(strata-cli): warn user about password strength

) * fix(strata-cli): better warnings of low-entropy pass * refactor(strata-cli): password warnings into a function

… refresh -> recover, config file + more (#382) * feat: manual signet fee rates + full scan command * fix: use `u64` for feerates + rename refresh to recover * fix: disable faucet L2 functionality * feat: add network + bridge pubkey settings & use config file from env var * chore: remove all strata logic from faucet * feat: add config command to show config file path * fix: featuregate strata faucet code * chore: rename `l2_http_endpoint` to `strata_endpoint` * fix: patch featuregated to use `strata_endpoint` * fix: remove password requirement from reset cmd * chore: update bridge pubkey * fix: drain command * fix: "bridge-in" and "bridge-out" to "deposit" and "withdraw" * chore: update withdraw cmd * chore: updating comments.+ logs * chore: comments * chore: update bridge strata address * chore: hide endpoints * fix: printing config file location with no config * fix: various fixes, recovery descriptor autocleanup * fix: remove `seed_encryption_key` caching for `Password` * feat: zeroize password * chore: derive `Eq` as well as `PartialEq` on `NetworkType` Co-authored-by: Jose Storopoli <jose@storopoli.io> * fix(cli): feerate logic revamp * chore(cli): fix clippy --------- Co-authored-by: Jose Storopoli <jose@storopoli.io>

…423) * feat: bitcoind wallet syncing * feat: feerate + broadcast via `SyncBackend` * feat:(cli) `SignetBackend` * feat:(cli) `spawn_bitcoin_core` wrapper * feat:(cli) trait based `SignetBackend` * refactor:(cli) update `SignetBackend` to not use `&mut Wallet` * doc: fix doclink --------- Co-authored-by: Jose Storopoli <jose@storopoli.io>

* ci: update to bitcoin 28.0 * fix(btcio): non-breaking changes in GetBlockChainInfo Shouldn't be a breaking change for us, since we are not using any of the changed stuff. See: https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-28.0.md#updated-rpcs

It does not make sense to have this dep since we're already half-baking our own custom types on top of it and using only a tiny minority of the types as being the ones provided by bitcoind_json_rpc_types.

storopoli · 2024-10-28T15:11:21Z

Done in #437

delbonis

Panics should only be used when they'd be triggered as of an actual bug in our code, an assumption being violated. Not something that can be caused by some normal user error. See ticket.

delbonis · 2024-10-28T16:34:16Z

bin/strata-cli/src/cmd/balance.rs

-        l1w.sync(&esplora).await.unwrap();
+        let mut l1w =
+            SignetWallet::new(&seed, settings.network, settings.signet_backend.clone()).unwrap();
+        l1w.sync().await.unwrap();


Fix this .unwrap().

delbonis · 2024-10-28T16:34:26Z

bin/strata-cli/src/cmd/balance.rs

+        let l2w = StrataWallet::new(&seed, &settings.strata_endpoint).unwrap();
        let _ = term.write_line("Getting balance...");
        let balance = l2w.get_balance(l2w.default_signer_address()).await.unwrap();


Fix these .unwrap()s.

delbonis · 2024-10-28T16:34:50Z

bin/strata-cli/src/cmd/deposit.rs

+    let requested_strata_address =
+        strata_address.map(|a| StrataAddress::from_str(&a).expect("bad strata address"));
+    let mut l1w =
+        SignetWallet::new(&seed, settings.network, settings.signet_backend.clone()).unwrap();
+    let l2w = StrataWallet::new(&seed, &settings.strata_endpoint).unwrap();

-    l1w.sync(&esplora).await.unwrap();
+    l1w.sync().await.unwrap();


Fix these unwraps/expects.

delbonis · 2024-10-28T16:35:04Z

bin/strata-cli/src/cmd/deposit.rs

        .expect("valid descriptor");

    let mut temp_wallet = Wallet::create_single(desc.clone())
-        .network(NETWORK)
+        .network(settings.network)
        .create_wallet_no_persist()
        .expect("valid wallet");


Probably should fix these expects.

delbonis · 2024-10-28T16:35:19Z

bin/strata-cli/src/cmd/deposit.rs

+        .signet_backend
+        .broadcast_tx(&tx)
+        .await
+        .expect("successful broadcast");


Fix this expect.

delbonis · 2024-10-28T16:42:57Z

bin/strata-cli/src/signet.rs

+
+    handle
+        .await
+        .expect("thread to be fine")


Fix this expect.

delbonis · 2024-10-28T16:43:09Z

bin/strata-cli/src/signet.rs

+
+    handle
+        .await
+        .expect("thread to be fine")


Fix this expect.

delbonis · 2024-10-28T16:43:18Z

bin/strata-cli/src/signet.rs

+            WalletUpdate::SpkSync(update) => {
+                wallet.apply_update(update).expect("update to connect")
+            }
+            WalletUpdate::SpkScan(update) => {
+                wallet.apply_update(update).expect("update to connect")
+            }
+            WalletUpdate::NewBlock(ev) => {
+                let height = ev.block_height();
+                let connected_to = ev.connected_to();
+                wallet
+                    .apply_block_connected_to(&ev.block, height, connected_to)
+                    .expect("block to be added")


Fix these expects.

delbonis · 2024-10-28T16:45:01Z

bin/strata-cli/src/signet/backend.rs

+#[derive(Debug)]
+pub struct InvalidFee;


Should split error types out into their own module, ideally.

delbonis · 2024-10-28T16:46:26Z

bin/strata-cli/src/signet/persist.rs

+/// Wrapper around the built-in rusqlite db that allows
+/// [`PersistedWallet`](crate::signet::PersistedWallet) to be shared across multiple threads by
+/// lazily initializing per core connections to the sqlite db and keeping them in local thread
+/// storage instead of sharing the connection across cores.
+///
+/// WARNING: [`set_data_dir`] **MUST** be called and set before using [`Persister`].
+#[derive(Debug)]
+pub struct Persister;
+
+static DATA_DIR: OnceLock<PathBuf> = OnceLock::new();
+
+/// Sets the data directory static for the thread local DB.
+///
+/// Must be called before accessing [`Persister`].
+///
+/// Can only be set once - will return whether value was set.
+pub fn set_data_dir(data_dir: PathBuf) -> bool {
+    DATA_DIR.set(data_dir).is_ok()
+}
+
+thread_local! {
+    static DB: Rc<RefCell<Connection>> = RefCell::new(Connection::open(SignetWallet::db_path("default", DATA_DIR.get().expect("data dir to be set"))).unwrap()).into();
+}


This data should be passed around as struct fields along with the data associated with it, not in globals/tls.

storopoli · 2024-10-28T16:49:42Z

@delbonis can you check #437? that was the one we merged.

sapinb requested review from prajwolrg, storopoli and Zk2u October 27, 2024 05:59

Zk2u requested changes Oct 27, 2024

View reviewed changes

storopoli requested changes Oct 28, 2024

View reviewed changes

storopoli requested a review from Zk2u October 28, 2024 14:19

storopoli previously approved these changes Oct 28, 2024

View reviewed changes

storopoli and others added 5 commits October 28, 2024 11:20

feat(strata-cli)!: add hash version and optional password (#392)

fd8100f

* feat(strata-cli)!: add hash version and optional password * feat(strata-cli): warn user about password strength

fix(strata-cli): better handling of low-entropy passwords warnings (#397

1e418d2

) * fix(strata-cli): better warnings of low-entropy pass * refactor(strata-cli): password warnings into a function

fix:(cli) default to broadcast min when esplora doesn't return a fee

86ac7bf

storopoli force-pushed the cli-updates-to-release branch from a743f65 to 5b5b49b Compare October 28, 2024 14:20

storopoli and others added 6 commits October 28, 2024 11:24

Zeroize seed encryption key (#411)

49e4ffe

Zeroize seed (#412)

ca204a4

Add sanity test for key generation

0299d2f

chore: bump bitcoin to 0.32.3 (#425)

41e61a0

refactor(btcio): remove bitcoind_json_rpc_types

8adc816

It does not make sense to have this dep since we're already half-baking our own custom types on top of it and using only a tiny minority of the types as being the ones provided by bitcoind_json_rpc_types.

storopoli dismissed their stale review via 8adc816 October 28, 2024 14:29

chore: add missing bdk_bitcoind_rpc

bee7348

storopoli mentioned this pull request Oct 28, 2024

backport: CLI updates #437

Merged

13 tasks

storopoli closed this Oct 28, 2024

storopoli deleted the cli-updates-to-release branch October 28, 2024 15:11

delbonis reviewed Oct 28, 2024

View reviewed changes

delbonis restored the cli-updates-to-release branch October 28, 2024 16:48

storopoli deleted the cli-updates-to-release branch October 28, 2024 16:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cli updates to release #432

Cli updates to release #432

sapinb commented Oct 27, 2024

codecov bot commented Oct 27, 2024

Zk2u left a comment

storopoli commented Oct 28, 2024

storopoli left a comment

storopoli commented Oct 28, 2024

storopoli left a comment

storopoli commented Oct 28, 2024

delbonis left a comment

delbonis Oct 28, 2024

delbonis Oct 28, 2024

delbonis Oct 28, 2024

delbonis Oct 28, 2024

delbonis Oct 28, 2024

delbonis Oct 28, 2024

delbonis Oct 28, 2024

delbonis Oct 28, 2024

delbonis Oct 28, 2024

delbonis Oct 28, 2024

storopoli commented Oct 28, 2024

		#[derive(Debug)]
		pub struct InvalidFee;

Cli updates to release #432

Cli updates to release #432

Conversation

sapinb commented Oct 27, 2024

Description

Type of Change

Checklist

Related Issues

codecov bot commented Oct 27, 2024

Codecov Report

Zk2u left a comment

Choose a reason for hiding this comment

storopoli commented Oct 28, 2024

storopoli left a comment

Choose a reason for hiding this comment

storopoli commented Oct 28, 2024

storopoli left a comment

Choose a reason for hiding this comment

storopoli commented Oct 28, 2024

delbonis 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

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

storopoli commented Oct 28, 2024