-
Notifications
You must be signed in to change notification settings - Fork 307
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
refactor(electrum) put the tx cache in electrum #1453
Conversation
105bbed
to
76683ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK
47f14f8
to
5285149
Compare
Previously there was a tx cache that you passed in as part of the sync request. This seems bad and the example show'd that you should copy all your transactions from the transaction graph into the sync request every time you sync'd. If you forgot to do this then you would always download everything. Instead just do a plain old simple cache inside the electrum client. This way at least you only download transactions once. You can pre-populate the cache with a method also and I did this in the examples.
Also: update `wallet_electrum` example to use the method.
5285149
to
2d2656a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self-ACK 2d2656a
@LLFourn I re-exported a method transaction_broadcast
(as per @notmandatory's suggestion) and renamed the first commit's message to match convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 2d2656a
ec36c7e feat(example): use changeset staging with rpc polling example (志宇) 19328d4 feat(wallet)!: change persist API to use `StageExt` and `StageExtAsync` (志宇) 2e40b01 feat(chain): reintroduce a way to stage changesets before persisting (志宇) 36e82ec chore(chain): relax `miniscript` feature flag scope (志宇) 9e97ac0 refactor(persist): update file_store, sqlite, wallet to use bdk_chain::persist (Steve Myers) 54b0c11 feat(persist): add PersistAsync trait and StagedPersistAsync struct (Steve Myers) aa640ab refactor(persist): rename PersistBackend to Persist, move to chain crate (Steve Myers) Pull request description: ### Description Sorry to submit another refactor PR for the persist related stuff, but I think it's worth revisiting. My primary motivations are: 1. remove `db` from `Wallet` so users have the ability to use `async` storage crates, for example using `sqlx`. I updated docs and examples to let users know they are responsible for persisting changes. 2. remove the `anyhow` dependency everywhere (except as a dev test dependency). It really doesn't belong in a lib and by removing persistence from `Wallet` it isn't needed. 3. remove the `bdk_persist` crate and revert back to the original design with generic error types. I kept the `Debug` and `Display` constrains on persist errors so they could still be used with the `anyhow!` macro. ### Notes to the reviewers I also replaced/renamed old `Persist` with `StagedPersist` struct inspired by #1453, it is only used in examples. The `Wallet` handles it's own staging. ### Changelog notice Changed - Removed `db` from `Wallet`, users are now responsible for persisting changes, see docs and examples. - Removed the `bdk_persist` crate and moved logic back to `bdk_chain::persist` module. - Renamed `PersistBackend` trait to `Persist` - Replaced `Persist` struct with `StagedPersist` ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing ACKs for top commit: ValuedMammal: ACK ec36c7e Tree-SHA512: 380b94ae42411ea174720b7c185493c640425f551742f25576a6259a51c1037b441a238c6043f4fdfbf1490aa15f948a34139f1850d0c18d285110f6a9f36018
Previously there was a
TxCache
that you passed in as part of the sync request. There are lots of downsides to this:Instead just do a plain old simple cache inside the electrum client. This way at least you only download transactions once. You can pre-populate the cache with a method also and I did this in the examples.