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

Examples don't show how to properly initialize a IndexedTxGraph etc from disk #1462

Closed
LLFourn opened this issue Jun 6, 2024 · 3 comments
Closed
Labels
bug Something isn't working good first issue Good for newcomers invalid This doesn't seem right
Milestone

Comments

@LLFourn
Copy link
Contributor

LLFourn commented Jun 6, 2024

See example_esplora, example_electrum and example_bitcoind_rpc_polling

  • for some reasons each of these independently apply the initial changeset to the graph (why can't it just be done inside example_cli::init).
  • The initialization is a bit of a mess where the examples insert the descriptors but don't persist them. Are we doing this intentionally? It's an interesting feature that you can actually change the descriptors in the example and not have to delete the database. If we are doing this intentionally then we should explain this.

I think with the example cli we should have an init command which takes in the descriptor assignments and persists them. It would be great if it could also generate a random bip86 descriptor too. Then future calls should require the db has been initialized -- no more environment variables.

  • can we run the cli examples in CI against a regtest node somehow?
@LLFourn LLFourn added bug Something isn't working good first issue Good for newcomers invalid This doesn't seem right labels Jun 6, 2024
@notmandatory notmandatory added this to the 1.0.0-beta milestone Jun 6, 2024
evanlinjin added a commit that referenced this issue Aug 12, 2024
771f6b9 example: Update example_cli (valued mammal)

Pull request description:

  - Adds two commands `init` and `generate`. Loading database doesn't require descriptors
  - Replaces `send` command with `psbt` (new, sign, and extract). Supports tap key spend
  - Uses `bdk-coin-select` and miniscript `plan` module

  fixes #1469
  partially addresses #1462

  ### Notes to the reviewers

  Note the `example_cli` lib now defines the `ChangeSet` and `Anchor` type whereas before these were generic

  ### Changelog notice

  * Improvements to `example_cli` that include generating descriptors and creating PSBTs

  ### 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:
  evanlinjin:
    ACK 771f6b9

Tree-SHA512: 93b9e38ec428c3ed5920e7632381dcf50e28a01ceb8e1438c87aa459563600cd79636bcf1179eefa763adce98239a60ad2894dfb2ba0f2700cc64e57ede75010
@ValuedMammal
Copy link
Contributor

@notmandatory This is mostly complete except for

  • can we run the cli examples in CI against a regtest node somehow?

therefore probably not critical to have for beta but worth keeping open.

@notmandatory
Copy link
Member

@ValuedMammal I agree running against a regtest node would be great but not essential for the 1.0.0-beta milestone, should be in a new issue so we can close this.

@ValuedMammal
Copy link
Contributor

Fixed by #1495, but I'll open an issue to carry over the remaining tasks #1618

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers invalid This doesn't seem right
Projects
Status: Done
Development

No branches or pull requests

3 participants