Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RPC Wallet Example #1106

Closed

Conversation

vladimirfomene
Copy link
Contributor

@vladimirfomene vladimirfomene commented Aug 31, 2023

Description

This is a demonstration of how to create a wallet with a RPC syncing mechanism. This builds on #1041.

Notes to the reviewers

Will love to hear your feedback on this current implementation.

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

vladimirfomene and others added 14 commits August 26, 2023 22:23
We remove `ElectrumUpdate` and return tuples instead for `ElectrumExt`
methods. We introduce the `IncompleteTxGraph` structure to specifically
hodl the incomplete `TxGraph`.

This change is motivated by @LLFourn's comment: bitcoindevkit@794bf37#r1305432603
Co-authored-by: Steve Myers <steve@notmandatory.org>
Also introduce `bitcoind_rpc` cli example.

Add tests:

* `test_sync_local_chain` ensures that `Emitter::emit_block` emits
  blocks in order, even after reorg.
* `test_into_tx_graph` ensures that `into_tx_graph` behaves
  appropriately for both mempool and block updates. It should also
filter txs and map anchors correctly.
Replaced `into_tx_graph_update` with `indexed_tx_graph_update`. The
latter returns a vec of `(tx, anchors, last_seen)` to be passed into
`IndexedTxGraph::insert_relevant_txs`.
@vladimirfomene vladimirfomene force-pushed the add_rpc_wallet branch 2 times, most recently from 8e89e13 to f8f3ad2 Compare September 4, 2023 07:06
@nondiremanuel
Copy link

Better if it goes in alpha.2 together with #1041, but in case we can move it to alpha.3

@notmandatory
Copy link
Member

I'm moving this to alpha.3 since #1041 already has a non-wallet example and this one will be easier to do once that's merged.

@notmandatory
Copy link
Member

@vladimirfomene if you this it's still relevant, go ahead and copy/paste in the README from #1019 after you rebase this PR.

@evanlinjin
Copy link
Member

Replaced by #1172

@notmandatory notmandatory modified the milestones: 1.0.0-alpha.3, 1.0.0-alpha.4, 0.29.1, 1.0.0-beta.0 Nov 13, 2023
@notmandatory
Copy link
Member

Moved this to beta release so we can focus final alpha releases on functional changes, leaving additional test/example/doc changes to beta milestone.

@notmandatory
Copy link
Member

Closing since it's replaced by #1172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants