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

Added wallet name to persister #12

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Commits on Aug 13, 2024

  1. fix(example_cli): add bitcoin and rand dependencies

    The bitcoin and rand dependencies are required to build examples
    independently and not from the top level bdk workspace.
    notmandatory committed Aug 13, 2024
    Configuration menu
    Copy the full SHA
    1adf63c View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    3675a9e View commit details
    Browse the repository at this point in the history

Commits on Aug 14, 2024

  1. feat(esplora): make ext traits more flexible

    Some users would like to use esplora updates with custom `ChainOracle`s
    (not `LocalChain`). We introduce "low-level" extension methods that
    populate an update `TxGraph` with associated data.
    
    Additionally, much of the logic has been made more efficient. We make
    use of the `/tx/:txid` endpoint (`get_tx_info` method) to do a single
    call to get both the tx and tx_status. If the tx already exists, we only
    fetch the tx_status. The logic for fetching data based on outpoints has
    been reworked to be more efficient and it now supports parallelism
    Additionally, the logic for fetching data based on outpoints has been
    reworked to be more efficient and it now supports parallelism.
    
    Documentation has also been reworked.
    
    Note that this is NOT a breaking change because the API of `full_scan`
    and `sync` are not changed.
    evanlinjin committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    cad3533 View commit details
    Browse the repository at this point in the history
  2. feat(esplora): always fetch prevouts

    Prevouts are needed to calculate fees for transactions. They are
    introduced as floating txouts in the update `TxGraph`. A helper method
    `insert_prevouts` is added to insert the floating txouts using the
    `Vin`s returned from Esplora.
    
    Also replaced `anchor_from_status` with `insert_anchor_from_status` as
    we always insert the anchor into the update `TxGraph` after getting it.
    
    Also removed `bitcoin` dependency as `bdk_chain` already depends on
    `bitcoin` (and it's re-exported).
    evanlinjin committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    c93e6fd View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    16c1c2c View commit details
    Browse the repository at this point in the history
  4. feat!: rework FullScanRequest and SyncRequest

    Change `FullScanRequest` and `SyncRequest` take in a `chain_tip` as an
    option. In turn, `FullScanResult` and `SyncResult` are also changed to
    return the update `chain_tip` as an option. This allows the caller to
    opt-out of getting a `LocalChain` update.
    
    Rework `FullScanRequest` and `SyncRequest` to have better ergonomics
    when inspecting the progress of items of a sync request. Richer progress
    data is provided to the inspect closure.
    
    Introduce `FullScanRequestBuilder` and `SyncRequestBuilder`. Separating
    out request-construction and request-consumption in different structs
    simplifies the API and method names.
    
    Simplify `EsploraExt` and `EsploraAsyncExt` back to having two methods
    (`full_scan` and `sync`). The caller can still opt out of fetching a
    `LocalChain` update with the new `FullScanRequest` and `SyncRequest`.
    evanlinjin committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    44e2a79 View commit details
    Browse the repository at this point in the history
  5. fix: no premature collect

    evanlinjin committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    38f86fe View commit details
    Browse the repository at this point in the history
  6. docs(esplora): Fix typo

    evanlinjin committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    0234f70 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    96023c0 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    3eb5dd1 View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    584b10a View commit details
    Browse the repository at this point in the history
  10. refactor(chain)!: Rename spks_with_labels to spks_with_indices

    and use consistent generic parameter names across `SyncRequest` and
    `SyncRequestBuilder`.
    evanlinjin committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    6d77e2e View commit details
    Browse the repository at this point in the history
  11. Merge bitcoindevkit#1478: Allow opting out of getting LocalChain up…

    …dates with `FullScanRequest`/`SyncRequest` structures
    
    6d77e2e refactor(chain)!: Rename `spks_with_labels` to `spks_with_indices` (志宇)
    584b10a docs(esplora): README example, uncomment async import (志宇)
    3eb5dd1 fix(chain): correct `Iterator::size_hint` impl (志宇)
    96023c0 docs(chain): improve `SyncRequestBuilder::spks_with_labels` docs (志宇)
    0234f70 docs(esplora): Fix typo (志宇)
    38f86fe fix: no premature collect (志宇)
    44e2a79 feat!: rework `FullScanRequest` and `SyncRequest` (志宇)
    16c1c2c docs(esplora): Simplify crate docs (志宇)
    c93e6fd feat(esplora): always fetch prevouts (志宇)
    cad3533 feat(esplora): make ext traits more flexible (志宇)
    
    Pull request description:
    
      Closes bitcoindevkit#1528
    
      ### Description
    
      Some users use `bdk_esplora` to update `bdk_chain` structures *without* a `LocalChain`. ~~This PR introduces "low-level" methods to `EsploraExt` and `EsploraAsyncExt` which populates a `TxGraph` update with associated data.~~
    
      We change `FullScanRequest`/`SyncRequest` to take in the `chain_tip` parameter as an option. Spk-based chain sources (`bdk_electrum` and `bdk_esplora`) will not fetch a chain-update if `chain_tip` is `None`, allowing callers to opt-out of receiving updates for `LocalChain`.
    
      We change `FullScanRequest`/`SyncRequest` to have better ergonomics when inspecting the progress of syncing (refer to bitcoindevkit#1528).
    
      We change `FullScanRequest`/`SyncRequest` to be constructed with a builder pattern. This is a better API since we separate request-construction and request-consumption.
    
      Additionally, much of the `bdk_esplora` logic has been made more efficient (less calls to Esplora) by utilizing the `/tx/:txid` endpoint (`get_tx_info` method). This method returns the tx and tx_status in one call. The logic for fetching updates for outpoints has been reworked to support parallelism.
    
      Documentation has also been updated.
    
      ### Notes to reviewers
    
      This PR has evolved somewhat. Initially, we were adding more methods on `EsploraExt`/`EsploraAsyncExt` to make syncing/scanning more modular. However, it turns out we can just make the `chain_tip` parameter of the request structures optional. Since we are changing the request structures, we might as well go further and improve the ergonomics of the whole structures (refer to bitcoindevkit#1528). This is where we are at with this PR.
    
      Unfortunately, the changes are now breaking. I think this is an okay tradeoff for the API improvements (before we get to stable).
    
      ### Changelog notice
    
      * Change request structures in `bdk_chain::spk_client` to be constructed via a builder pattern, make providing a `chain_tip` optional, and have better ergonomics for inspecting progress while syncing.
      * Change `bdk_esplora` to be more efficient by reducing the number of calls via the `/tx/:txid` endpoint. The logic for fetching outpoint updates has been reworked to support parallelism.
      * Change `bdk_esplora` to always add prev-txouts to the `TxGraph` update.
    
      ### 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
    
      #### New Features:
    
      * [ ] I've added tests for the new feature
      * [x] I've added docs for the new feature
    
    ACKs for top commit:
      ValuedMammal:
        ACK 6d77e2e
      notmandatory:
        ACK 6d77e2e
    
    Tree-SHA512: 806cb159a8801f4e33846d18e6053b65d105e452b0f3f9d639b0c3f2e48fb665e632898bf42977901526834587223b0d7ec7ba1f73bb796b5fd8fe91e6f287f7
    notmandatory committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    cc84872 View commit details
    Browse the repository at this point in the history
  12. refactor(wallet)!: rename LoadParams methods

    The network and genesis_hash methods on the LoadParams struct have been
    renamed to check_network and check_genesis_hash to better reflect their
    use.
    thunderbiscuit committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    2391b76 View commit details
    Browse the repository at this point in the history
  13. Merge bitcoindevkit#1537: Use explicit names for wallet builder metho…

    …ds that validate rather than set
    
    2391b76 refactor(wallet)!: rename LoadParams methods (thunderbiscuit)
    
    Pull request description:
    
      This PR is a follow up to the dev call discussion where we decided it was better to PR these changes separate from bitcoindevkit#1533. This is a breaking change, but I think it's worth it because those will potentially cause runtime errors for users that expect one thing to happen and realize it's something else.
    
      Left out of this PR but as surfaced in the call probably worth discussing is whether these methods make sense at all or whether they should be removed entirely. What does it mean to return an error when someone loads a wallet from persistence for which the genesis hash doesn't match the one persisted? Maybe worth a new issue; this PR simply attempts to name them correctly.
    
      ### Description
      See [Q1 in comment here](bitcoindevkit#1533 (comment)) for more context into the initial question.
    
      Two of the methods on the builder that loads a wallet were checkers/validators rather than setters:
      - `network()`
      - `genesis_hash()`
    
      This is confusing for users because when loading a wallet from persistence those things are persisted and cannot be changed, and yet seemingly the option to do that is there with those methods (so now you're re-thinking what you think you know about the changeset and persistence). Moreover, the fields on the [`LoadParams` type](https://docs.rs/bdk_wallet/1.0.0-beta.1/src/bdk_wallet/wallet/params.rs.html#116-124) are correctly named `check_network` and `check_genesis_hash`. This PR simply brings those two methods in line with what they actually do and set on the struct they modify.
    
      This modification was not done on the `descriptors()` method, because that one is both a validator and a setter if the descriptors passed contain private keys.
    
      Note that I chose the names `validate_network` and `validate_genesis_hash` but I'm not married to them. If others prefer `check_network` and `check_genesis_hash`, I'm happy to fix them that way instead!
    
      ### Changelog notice
    
      ```md
      Breaking:
        - The `LoadParams` type used in the wallet load builder renamed its  `network()` and `genesis_hash` methods to `check_network()` and `check_genesis_hash`. [bitcoindevkit#1537]
    
      [bitcoindevkit#1537]: bitcoindevkit#1537
      ```
    
      ### 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
    
      #### New Features:
    
      * [ ] I've added tests for the new feature
      * [ ] I've added docs for the new feature
    
      #### Bugfixes:
    
      * [x] 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
    
    ACKs for top commit:
      notmandatory:
        ACK 2391b76
    
    Tree-SHA512: 6852ad165bab230a003b92ae0408176055f8c9101a0936d2d5a41c2a3577e258b045a7f4b796d9bab29ed261caf80b738c4338d4ff9322fbddc8c273ab0ff914
    notmandatory committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    9695296 View commit details
    Browse the repository at this point in the history
  14. Merge bitcoindevkit#1549: fix(example_cli): add bitcoin and rand depe…

    …ndencies
    
    3675a9e ci: add job to build example-crates independently (Steve Myers)
    1adf63c fix(example_cli): add bitcoin and rand dependencies (Steve Myers)
    
    Pull request description:
    
      ### Description
    
      Fix building `example_cli` by adding the bitcoin and rand dependencies so it, and the crates that depend on it, can be built independently and not only from the workspace.
    
      ### Notes to the reviewers
    
      I also added the  build-examples CI job to verify we can build examples individually.
    
      ### Changelog notice
    
      None.
    
      ### 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
    
      #### Bugfixes:
    
      * [ ] This pull request breaks the existing API
      * [x] I've added tests to reproduce the issue which are now passing
      * [ ] I'm linking the issue being fixed by this PR
    
    ACKs for top commit:
      ValuedMammal:
        ACK 3675a9e
      storopoli:
        ACK 3675a9e
    
    Tree-SHA512: c88fdf6cde72959c88c9f4563834824c573afedb1e5136b0f902d919b42b0de18424fb0d05f275c63a0c05da8062dc53ad5825bdc8dc4b12441890e1b799378b
    notmandatory committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    e0822d7 View commit details
    Browse the repository at this point in the history

Commits on Aug 15, 2024

  1. feat(wallet)!: introduce WalletPersister

    This replaces `bdk_chain::PersistWith` which wanted to persist anything
    (not only `Wallet`), hence, requiring a whole bunch of generic
    parameters.
    
    Having `WalletPersister` dedicated to persisting `Wallet` simplifies the
    trait by a lot.
    
    In addition, `AsyncWalletPersister` has proper lifetime bounds whereas
    `bdk_chain::PersistAsyncWith` did not.
    evanlinjin committed Aug 15, 2024
    Configuration menu
    Copy the full SHA
    039622f View commit details
    Browse the repository at this point in the history
  2. feat(chain,wallet)!: publicize .init_sqlite_tables changeset methods

    Changeset methods `.persist_to_sqlite` and `from_sqlite` no longer
    internally call `.init_sqlite_tables`. Instead, it is up to the caller
    to call `.init_sqlite_tables` beforehand.
    
    This allows us to utilize `WalletPersister::initialize`, instead of
    calling `.init_sqlite_tables` every time we persist/load.
    evanlinjin committed Aug 15, 2024
    Configuration menu
    Copy the full SHA
    06a9d6c View commit details
    Browse the repository at this point in the history
  3. feat(wallet)!: remove dependency on bdk_chain::Staged

    Introduce `Wallet::staged_mut` method.
    evanlinjin committed Aug 15, 2024
    Configuration menu
    Copy the full SHA
    a9c5f76 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    0616057 View commit details
    Browse the repository at this point in the history
  5. feat(wallet)!: add persister (P) type param to PersistedWallet<P>

    This forces the caller to use the same persister type that they used for
    loading/creating when calling `.persist` on `PersistedWallet`.
    
    This is not totally fool-proof since we can have multiple instances of
    the same persister type persisting to different databases. However, it
    does further enforce some level of safety.
    evanlinjin committed Aug 15, 2024
    Configuration menu
    Copy the full SHA
    9600293 View commit details
    Browse the repository at this point in the history

Commits on Aug 16, 2024

  1. Configuration menu
    Copy the full SHA
    f434c93 View commit details
    Browse the repository at this point in the history

Commits on Aug 19, 2024

  1. Configuration menu
    Copy the full SHA
    c7464b1 View commit details
    Browse the repository at this point in the history