Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

RPC chain example #89

Closed
wants to merge 10 commits into from
Closed

Conversation

evanlinjin
Copy link
Collaborator

@evanlinjin evanlinjin commented Dec 14, 2022

RPC Chain example.

Copy link
Owner

@LLFourn LLFourn left a comment

Choose a reason for hiding this comment

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

Thanks @evanlinjin. Quick review of how bdk core APIs are used. I think it can be simplified quite a bit by changing the API to return last_active_index.

I would love to see the sync/scan command to have a --live option where after doing the sync it prints out some basic stuff like balance and utxos maybe and then sits there polling for new blocks and prints out details about new transactions that come in that spend/create new wallet utxos.

Since the codebase is getting more mature now it'd be great if you could add docs and even example usage for new APIs (or even old ones). I find writing an explanation of the API helps me figure out early if the API actually makes sense.

bdk_rpc_example/src/main.rs Show resolved Hide resolved
bdk_rpc_example/src/main.rs Outdated Show resolved Hide resolved
bdk_keychain/src/keychain_txout_index.rs Outdated Show resolved Hide resolved
bdk_keychain/src/keychain_txout_index.rs Outdated Show resolved Hide resolved
bdk_keychain/src/keychain_txout_index.rs Show resolved Hide resolved
bdk_rpc_example/src/main.rs Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2022

Codecov Report

Merging #89 (35ef8ec) into master (ab0de04) will increase coverage by 0.99%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   50.33%   51.32%   +0.99%     
==========================================
  Files           9        9              
  Lines         602      602              
==========================================
+ Hits          303      309       +6     
+ Misses        299      293       -6     
Impacted Files Coverage Δ
bdk_core/src/chain_graph.rs 0.00% <ø> (ø)
bdk_core/src/sparse_chain.rs 78.57% <ø> (ø)
bdk_core/src/tx_graph.rs 64.23% <0.00%> (+4.37%) ⬆️

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

@evanlinjin evanlinjin force-pushed the rpc-example branch 4 times, most recently from 5163a44 to a8f2223 Compare December 15, 2022 17:40
bdk_rpc_example/src/main.rs Outdated Show resolved Hide resolved
bdk_rpc_example/src/main.rs Show resolved Hide resolved
if keychain_tracker.txout_index.is_relevant(&tx) {
update.insert_tx(tx.clone(), Some(height))?;
}
keychain_tracker.txout_index.scan(&tx);
Copy link
Owner

Choose a reason for hiding this comment

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

I think this is unsound -- you have to scan the txs first before deciding if it's relevant. scan the block when it comes in and each tx individually when the mempool comes in. From the docs of is_relevant:

    /// Whether any of the inputs of this transaction spend a txout tracked or whether any output
    /// matches one of our script pubkeys.
    ///
    /// It is easily possible to misuse this method and get false negatives by calling it before you
    /// have scanned the `TxOut`s the transaction is spending. For example if you want to filter out
    /// all the transactions in a block that are irrelevant you **must first scan all the
    /// transactions in the block** and only then use this method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But we are scanning transactions in order so the previous outputs will be scanned before the current transaction right?

Copy link
Owner

Choose a reason for hiding this comment

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

Oh that's a good point but that only applies to the block txs. In the mempool are you guaranteed some kind of logical traversal order when you get the response. Maybe? But we shouldn't depend on it.

@evanlinjin
Copy link
Collaborator Author

@LLFourn I'm not a fan of having changesets for last_used_indicies

@evanlinjin evanlinjin marked this pull request as ready for review December 19, 2022 17:03
@evanlinjin evanlinjin changed the title WIP: Inital work on RPC chain example RPC chain example Dec 19, 2022
* `store_all_up_to` was using `Iter::any` which is short-circuiting
* `derive_until_unused_gap` was wrong
@evanlinjin
Copy link
Collaborator Author

Replaced by #157

All of @LLFourn 's pointers here should be addressed.

@evanlinjin evanlinjin closed this Jan 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants