Add a electrum syncing example by rajarshimaitra · Pull Request #77 · LLFourn/bdk_core_staging

rajarshimaitra · 2022-12-07T05:12:57Z

This PR adds an electrum syncing example.. Fixes #72

The electrum sync is done in two steps.

wallet_txid_scan returns a candidate KeychainScan with TxGraph data.
The candidate is compared with tracker's SparseChain, and list of new tx data to fetch is derived from the ChangeSet.
New transactions are fetched in batch and added into the KeychainScan update.
Apply the final KeychainScan on tracker and DB.

codecov-commenter · 2022-12-07T05:19:20Z

Codecov Report

Merging #77 (6051596) into master (fbc88da) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #77   +/-   ##
=======================================
  Coverage   59.39%   59.39%           
=======================================
  Files           8        8           
  Lines         266      266           
=======================================
  Hits          158      158           
  Misses        108      108

Impacted Files	Coverage Δ
bdk_chain/src/chain_graph.rs	`0.00% <ø> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

rajarshimaitra · 2022-12-07T11:38:09Z

Restructured the commit to handle #72 requirements.

Make Chaingraph consistent, by adding few assertions at the apply_update() API.
Make the electrum sync in two steps. Get all the txids, find new ones, fetch tx data, apply update..

rajarshimaitra · 2022-12-07T11:43:33Z

Me and @evanlinjin tend to disagree on the approach of the consistency assertion.. While to me it seems the best place to apply it is the update API, Evan thinks there could be a better approach.. Opening this up for discussion and reviews..

rajarshimaitra · 2022-12-11T14:38:55Z

Rebased on current master, and updated the example syncing with recent invariants around ChainGraph. Similar to RPC syncing, the first stage returns a SparseChain. The nex to_fetch_tx id set is derived using determine_changeset between the returned sparsechain and existing sparaechain within the tracker..

bdk_electrum_example/src/electrum.rs

evanlinjin

Good work so far!

bdk_electrum_example/src/electrum.rs

bdk_electrum_example/src/main.rs

rajarshimaitra · 2022-12-14T10:09:43Z

Updated with the above suggestions.

No default reorg depth mechanism. Takes in a ref of existing chain, and inserts checkpoint until the last common check point is found. And then push the current tip into update. In case of reorg, all checkpoints after the last common block is added into the update structure, and while SparseChain::apply_update new checkpoints will replace old checkpoints. No error is thrown.
No default stop gap.
A new function added in the main to collect all the common code between Scan and Sync.
Transaction fetching is done opportunistically, for only txids that we haven't seen before. For transactions that only changed in Index tx data is fetched from existing TxGraph of the tracker.

Q: What to do when we only have TxNode::Partial in graph, and index changes of that transaction? 2 possible options.

We can fetch such transactions again from the blockchain before applying update.
Have mechanism to insert_tx() with TXNode::Partial

LLFourn · 2022-12-14T10:30:55Z

Q: What to do when we only have TxNode::Partial in graph, and index changes of that transaction? 2 possible options.
* We can fetch such transactions again from the blockchain before applying update.

* Have mechanism to `insert_tx()` with `TXNode::Partial`

Haven't reviewed just comment: we don't keep the index for TxNode::Partial.

rajarshimaitra · 2022-12-14T10:33:20Z

Haven't reviewed just comment: we don't keep the index for TxNode::Partial.

Done..

LLFourn

Thanks @rajarshimaitra. Looks great. A few minor requests to finish this off. The main thing that's missing is to protect against re-org during sync.

bdk_electrum_example/src/electrum.rs

bdk_electrum_example/src/main.rs

rajarshimaitra · 2022-12-15T13:17:24Z

It seems like there is still a bit of duplicate code. Isn't it the case that everything after getting the sparse chain and applying the last active indexes will be the same?

Yes.. I tried to add this into a separate function but that's making the code more complicated, because many trait bounds needs to be satisfied for the KeychainStore::append_changeset(), which are handled in bdk_cli.. It still could be done, but after doing it it made the code more complex than it needs to be, and thought would be detriment for readers reading this as example, and its only removing 3 lines. So kept it as it is for now..

bdk_electrum_example/src/electrum.rs

LLFourn

I just took another pass at this. The list of things I've proposed to fix here is final now I think.

bdk_electrum_example/src/electrum.rs

bdk_electrum_example/src/main.rs

rajarshimaitra · 2022-12-22T08:16:47Z

Thanks @LLFourn for the comments.. Updated as suggested and used the new API.. modulo #77 (comment).. Code looks much simpler..

LLFourn

Look great @rajarshimaitra. Main things:

use batch calls to electrum
Intead of matching every error and then throwing a new anyhow error just use anyhow::Context::context
Remove function complete_update in favor of just getting the new_sparsechain from the match arms and doing the update from there like what @evanlinjin does here: https://github.com/LLFourn/bdk_core_staging/pull/107/files#diff-6f57072cac1464050942f5bf972a9a8f76cb82a151f93f5b5d16cab1515f925aR74

bdk_electrum_example/src/main.rs

bdk_electrum_example/src/electrum.rs

bdk_electrum_example/src/main.rs

rajarshimaitra · 2022-12-23T09:36:07Z

@LLFourn updated with the suggestions, module #77 (comment).

Let me know if this looks good and you want the the commit history squashed..

rajarshimaitra · 2022-12-30T11:56:14Z

Sqaushed and rebased on master..

LLFourn · 2023-01-05T05:06:23Z

I just did some work to get this working:

break at the right time in the loop for stop gap
add signet electrum server
print out scnning progress
Make client configurable
Stop returning the sparse chain from inflate_changeset because it
makes it harder to ? the error and you can just clone it if you still
need it on failure.

Check it out @rajarshimaitra.

This is working and probably ready to merge with final review from @evanlinjin.

- break at the right time in the loop - add signet electrum server - print out syncing progress - Make client configurable - Stop returning the sparse chain from inflate_changeset because it makes it harder to ? the error and you can just clone it if you still need it on failure.

rajarshimaitra · 2023-01-06T05:12:03Z

ACK 3531907

Thanks, @LLFourn for the fixes.. Code looks much better..

Added a nit fix..

Testing out the full thing..

evanlinjin · 2023-01-06T05:17:45Z

@rajarshimaitra You forced-pushed away the changes...

rajarshimaitra · 2023-01-06T05:18:17Z

Holy shit..

evanlinjin · 2023-01-06T05:18:47Z

@rajarshimaitra Fixed.

rajarshimaitra · 2023-01-06T05:19:23Z

Sorry dude.. Intended to just push a nit.. Fat fingered..

evanlinjin

ACK 3531907

Just some nits

bdk_electrum_example/src/electrum.rs

bdk_electrum_example/src/main.rs

* Remove unnecessary code * Check `index` is greater than `last_active_index` before changing `last_active_index`

rajarshimaitra mentioned this pull request Dec 7, 2022

ChainGraph API for electrum #72

Closed

rajarshimaitra force-pushed the electrum-example branch 3 times, most recently from ea42c4b to 3542c3d Compare December 7, 2022 11:37

rajarshimaitra force-pushed the electrum-example branch 3 times, most recently from 03b7d94 to 605abf8 Compare December 11, 2022 14:37

rajarshimaitra mentioned this pull request Dec 11, 2022

Electrum sync 2 #81

Closed

LLFourn reviewed Dec 12, 2022

View reviewed changes

bdk_electrum_example/src/electrum.rs Outdated Show resolved Hide resolved

LLFourn reviewed Dec 12, 2022

View reviewed changes

bdk_electrum_example/src/electrum.rs Outdated Show resolved Hide resolved

LLFourn reviewed Dec 12, 2022

View reviewed changes

bdk_electrum_example/src/electrum.rs Outdated Show resolved Hide resolved

LLFourn mentioned this pull request Dec 13, 2022

Implement updates that returns position in block and merkle proofs #85

Open

evanlinjin suggested changes Dec 13, 2022

View reviewed changes

bdk_electrum_example/src/electrum.rs Outdated Show resolved Hide resolved

bdk_electrum_example/src/main.rs Outdated Show resolved Hide resolved

LLFourn reviewed Dec 14, 2022

View reviewed changes

bdk_electrum_example/src/main.rs Outdated Show resolved Hide resolved

rajarshimaitra force-pushed the electrum-example branch from 605abf8 to 7ebd627 Compare December 14, 2022 10:02

rajarshimaitra force-pushed the electrum-example branch from 7ebd627 to a39916e Compare December 14, 2022 10:22

rajarshimaitra force-pushed the electrum-example branch 2 times, most recently from ba92c75 to 056979e Compare December 14, 2022 10:33

LLFourn suggested changes Dec 15, 2022

View reviewed changes

LLFourn reviewed Dec 16, 2022

View reviewed changes

bdk_electrum_example/src/electrum.rs Show resolved Hide resolved

LLFourn reviewed Dec 16, 2022

View reviewed changes

bdk_electrum_example/src/electrum.rs Outdated Show resolved Hide resolved

LLFourn reviewed Dec 21, 2022

View reviewed changes

rajarshimaitra force-pushed the electrum-example branch 2 times, most recently from 837b0e7 to e582098 Compare December 22, 2022 08:15

rajarshimaitra force-pushed the electrum-example branch 2 times, most recently from 42e151d to b60d4e1 Compare December 22, 2022 09:03

LLFourn reviewed Dec 23, 2022

View reviewed changes

rajarshimaitra force-pushed the electrum-example branch from 2d6c375 to 07cd6bb Compare December 30, 2022 11:55

rajarshimaitra force-pushed the electrum-example branch from 07cd6bb to c6f0aef Compare December 30, 2022 11:56

rajarshimaitra mentioned this pull request Jan 4, 2023

demonstrate syncing with block_position indexing #119

Open

LLFourn approved these changes Jan 5, 2023

View reviewed changes

rajarshimaitra and others added 4 commits January 6, 2023 10:53

Add a electrum_sync example

4b6d154

s/bdk_core/bdk_chain/g

371c60b

Clean up comments

3531907

LLFourn force-pushed the electrum-example branch from f5fe15a to 3531907 Compare January 5, 2023 23:54

rajarshimaitra force-pushed the electrum-example branch from 3531907 to c6f0aef Compare January 6, 2023 05:09

evanlinjin mentioned this pull request Jan 6, 2023

Electrum example #122

Closed

evanlinjin force-pushed the electrum-example branch from c6f0aef to 3531907 Compare January 6, 2023 05:18

evanlinjin approved these changes Jan 6, 2023

View reviewed changes

bdk_electrum_example/src/electrum.rs Outdated Show resolved Hide resolved

bdk_electrum_example/src/electrum.rs Show resolved Hide resolved

bdk_electrum_example/src/electrum.rs Outdated Show resolved Hide resolved

bdk_electrum_example/src/main.rs Show resolved Hide resolved

Small electrum example fixes

6051596

* Remove unnecessary code * Check `index` is greater than `last_active_index` before changing `last_active_index`

evanlinjin merged commit b271db7 into LLFourn:master Jan 6, 2023

Conversation

rajarshimaitra commented Dec 7, 2022

Uh oh!

codecov-commenter commented Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rajarshimaitra commented Dec 7, 2022

Uh oh!

rajarshimaitra commented Dec 7, 2022

Uh oh!

rajarshimaitra commented Dec 11, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajarshimaitra commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LLFourn commented Dec 14, 2022

Uh oh!

rajarshimaitra commented Dec 14, 2022

Uh oh!

LLFourn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajarshimaitra commented Dec 15, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LLFourn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajarshimaitra commented Dec 22, 2022

Uh oh!

LLFourn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajarshimaitra commented Dec 23, 2022

Uh oh!

rajarshimaitra commented Dec 30, 2022

Uh oh!

LLFourn commented Jan 5, 2023

Uh oh!

rajarshimaitra commented Jan 6, 2023

Uh oh!

evanlinjin commented Jan 6, 2023

codecov-commenter commented Dec 7, 2022 •

edited

Loading

rajarshimaitra commented Dec 14, 2022 •

edited

Loading