Add example with compact block filters #153

danielabrozzoni · 2023-01-19T13:49:49Z

No description provided.

evanlinjin · 2023-01-27T12:39:35Z

I just had a brief look through, nothing seems apparent to me why balance would disappear after restart. Will properly debug later.

danielabrozzoni · 2023-01-30T11:22:09Z

Rebased on top of #157

(Still doesn't work though)

evanlinjin · 2023-01-30T13:21:53Z

Rebased on top of #157

(Still doesn't work though)

Did you also add on the fix mentioned in Discord?

evanlinjin · 2023-01-31T12:08:21Z

danielabrozzoni#2

I did a PR into yours fixing the balance disappearing after restart.

Looking good so far. In my opinion, the next steps for this PR:

Don't hold a mutable reference of the tracker when syncing. We need a structure (imo, like an iterator), where we only grab initial data from the tracker to create. Then it spits out data which the tracker can consume.

In my head, this thing that the CBF Iterator spits out can look something like this:

pub enum CbfData {
    RelevantBlock(bitcoin::Block),
    DisconnectedBlock(u32), // block height
}

Additionally, we need to somehow figure out how to properly do stop gap. We need logic that goes "uhh oh, we need more scripts for a rescan"!

Co-authored-by: Daniela Brozzoni <danielabrozzoni@protonmail.com>

danielabrozzoni · 2023-02-16T22:51:53Z

Don't hold a mutable reference of the tracker when syncing. We need a structure (imo, like an iterator), where we only grab initial data from the tracker to create. Then it spits out data which the tracker can consume.

Right, done! :)

Additionally, we need to somehow figure out how to properly do stop gap. We need logic that goes "uhh oh, we need more scripts for a rescan"!

I think this is pretty trivial to implement, but I'd prefer to do it after receiving some feedback on the current code, if possible

evanlinjin · 2023-02-18T13:38:48Z

bdk_cbf_example/src/cbf.rs

+    pub fn sync_setup(
+        &mut self,
+        scripts: impl Iterator<Item = Script>,
+        processed_height: Option<BlockId>,


Instead of processed_height as an input, what do you think about using local_chain: BTreeMap<u32, BlockHash> and iterate in reverse until we find the last point of agreement?

Nakamoto has handle::Handle::get_block_by_height so this should be doable.

LLFourn · 2023-02-20T00:07:15Z

bdk_cbf_example/src/cbf.rs

+                                // TODO: improve this!
+                                // It might be that both me and this peer
+                                // are lagging behind
+                                return Ok(CbfEvent::ChainSynced(setup.processed_height));


What about using nakamoto's Event::Synced to emit this? This seems very error prone and even an attack vector. I think you shouldn't need to keep track of setup.peer_height at all.

So when you get Event::Synced with height == Handle::get_tip() you can emit this.

LLFourn · 2023-02-20T00:39:02Z

bdk_cbf_example/src/main.rs

+            let mut update = ChainGraph::<TxHeight>::default();
+
+            if let Some(last_cp) = last_checkpoint.clone() {
+                // Otherwise the chains don't connect :)


what if they don't connect though? :)

LLFourn · 2023-02-20T00:39:04Z

bdk_cbf_example/src/main.rs

+
+                        for (tx, height) in txs {
+                            //keychain_tracker.lock().unwrap().txout_index.pad_all_with_unused(stop_gap);
+                            if keychain_tracker.txout_index.is_relevant(&tx) {


I think rather than doing this here we should simply have a way of filtering out irrelevant things out of an update. For now we can just include all transactions in the block. I made an issue to build the tools to do this properly: #193

There really isn't much harm to just acquiring a .lock() on the db and the keychain tracker and just doing an apply_update to the KeychainTracker and pushing the changeset into the database right away. This is good because it saves your progress for each block as you go.

LLFourn · 2023-02-20T00:42:41Z

bdk_cbf_example/src/main.rs

+                    }
+                    CbfEvent::BlockDisconnected(id) => {
+                        // TODO: what happens if a block gets disconnected before I process its
+                        // filter?


I really hope that BlockMatched doesn't get emitted for things that are not in the chain! Hopefully nakamoto guarantees this.

LLFourn · 2023-02-20T00:43:37Z

bdk_cbf_example/src/main.rs

+        }
+        CbfCommands::Scan => {
+            // indexing logic!
+            let mut keychain_tracker = keychain_tracker.lock().unwrap();


Try and do every lock in scoped block {...} so the reader can see where locks can be taken and for how long they need to be taken for. Here you should only need the lock for the time it takes to get the scripts you're interested in and the checkpoints. You can return them from the block and release the lock.

LLFourn · 2023-02-20T00:45:05Z

bdk_cbf_example/src/main.rs

+                chain_graph: keychain_tracker
+                    .chain_graph()
+                    .determine_changeset(&update)?,
+            };


We should aim to not do this manually after #193 is done.

LLFourn · 2023-02-20T00:49:16Z

bdk_cli_lib/src/lib.rs

@@ -19,14 +19,15 @@ use bdk_chain::{
 use bdk_coin_select::{coin_select_bnb, CoinSelector, CoinSelectorOpt, WeightedValue};
 pub use clap;
 use clap::{Parser, Subcommand};
+pub use log;


Not sure if we need this. I think CLI should try and avoid using logging unless we're going into a kind of daemon mode. I think env_logger is a good idea though but I don't see it being initialized anywhere yet.

(this might be because this is on top of @evanlinjin's PR and nothing to do with this PR).

LLFourn · 2023-02-20T00:51:34Z

bdk_cbf_example/src/main.rs

+struct CbfArgs {
+    #[arg(value_enum, default_value_t = Domains::IPv4)]
+    domains: Domains,
+}


I'd prefer it if domains were just arguments to the actual commands that use domains and to not have CbfArgs. FWIW I've complained about adding global arguments to the bdk_cli::init to @evanlinjin but it looks like he didn't remove it from that PR yet.

LLFourn

Thanks @danielabrozzoni. I'm excited about nakamoto CBF. It's looking like a great option. I think we should do #193 so we can simplify the implementation here.

evanlinjin · 2023-02-21T20:34:48Z

@LLFourn I've tried removing the global CLI args but I couldn't find a way to do it. The "general commands" need to take in a client, and so the client needs to be initiated before our chain-specific commands.

LLFourn · 2023-02-23T01:02:44Z

@evanlinjin I see so you actually need to know "domains" for broadcasting too. So let's add the ability to add params to Send (rather than globally). Then pass those params through to the closure I made in #198 for broadcasting. Then you can create the client inside the closure.

danielabrozzoni force-pushed the cbf branch from 5559d49 to 734b33d Compare January 19, 2023 13:50

evanlinjin assigned danielabrozzoni Jan 20, 2023

danielabrozzoni force-pushed the cbf branch from 734b33d to a269128 Compare January 30, 2023 11:21

danielabrozzoni force-pushed the cbf branch from a269128 to 1234337 Compare January 30, 2023 11:22

evanlinjin mentioned this pull request Feb 5, 2023

A Basic CBF node using nakamoto #14

Closed

evanlinjin and others added 6 commits February 15, 2023 17:52

RPC chain example

0d1f904

Co-authored-by: Daniela Brozzoni <danielabrozzoni@protonmail.com>

Update RPC after rebase

eb86c94

bdk_cbf_example

96eea1a

Working CBF

38d379f

Not all RBF commands require waiting for peers

80ff49c

Small improvements

c6ca3c6

danielabrozzoni force-pushed the cbf branch from 8cd4b28 to 77c4532 Compare February 15, 2023 16:53

danielabrozzoni marked this pull request as ready for review February 15, 2023 16:55

CbfClient is iterator like yay

e7972f5

danielabrozzoni force-pushed the cbf branch from 77c4532 to e7972f5 Compare February 16, 2023 22:40

evanlinjin reviewed Feb 18, 2023

View reviewed changes

LLFourn reviewed Feb 20, 2023

View reviewed changes

ValuedMammal mentioned this pull request Feb 12, 2024

Implement bdk_nakamoto chain-source crate bitcoindevkit/bdk#679

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add example with compact block filters #153

Add example with compact block filters #153

danielabrozzoni commented Jan 19, 2023 •

edited

evanlinjin commented Jan 27, 2023

danielabrozzoni commented Jan 30, 2023

evanlinjin commented Jan 30, 2023

evanlinjin commented Jan 31, 2023 •

edited

danielabrozzoni commented Feb 16, 2023

evanlinjin Feb 18, 2023

LLFourn Feb 19, 2023

LLFourn Feb 20, 2023

LLFourn Feb 20, 2023

LLFourn Feb 20, 2023 •

edited

LLFourn Feb 20, 2023

LLFourn Feb 20, 2023

LLFourn Feb 20, 2023

LLFourn Feb 20, 2023 •

edited

LLFourn Feb 20, 2023

LLFourn left a comment

evanlinjin commented Feb 21, 2023

LLFourn commented Feb 23, 2023

Add example with compact block filters #153

Are you sure you want to change the base?

Add example with compact block filters #153

Conversation

danielabrozzoni commented Jan 19, 2023 • edited

evanlinjin commented Jan 27, 2023

danielabrozzoni commented Jan 30, 2023

evanlinjin commented Jan 30, 2023

evanlinjin commented Jan 31, 2023 • edited

danielabrozzoni commented Feb 16, 2023

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

LLFourn Feb 20, 2023 • edited

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

LLFourn Feb 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LLFourn left a comment

Choose a reason for hiding this comment

evanlinjin commented Feb 21, 2023

LLFourn commented Feb 23, 2023

danielabrozzoni commented Jan 19, 2023 •

edited

evanlinjin commented Jan 31, 2023 •

edited

LLFourn Feb 20, 2023 •

edited

LLFourn Feb 20, 2023 •

edited