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.

Sign up for GitHub

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

Jump to bottom

Don't use bitcoind's RPC for the watchonly wallet descriptors checksums #322

Open
darosior opened this issue Dec 3, 2021 · 1 comment · May be fixed by #364
Open

Don't use bitcoind's RPC for the watchonly wallet descriptors checksums #322

darosior opened this issue Dec 3, 2021 · 1 comment · May be fixed by #364
Assignees
@JSwambo
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@darosior
Copy link
Member

darosior commented Dec 3, 2021

For the watchonly wallet, we import addr() descriptors to bitcoind since it does not handle Miniscript yet. A descriptor must have a checksum to be imported to bitcoind. Back when i implemented this rust-miniscript did not have support for descriptor checksums. I used the hack of calling getdescriptorinfo before importing a descriptor to gather the checksum for each and every descriptor we import (hundreds on first startup).
Now that rust-miniscript does support them it's an unnecessary hack and performance hit. Replace it with just using the checksums provided by rust-miniscript.
@darosior darosior added enhancement New feature or request good first issue Good for newcomers labels Dec 3, 2021
darosior added a commit that referenced this issue Dec 14, 2021
@darosior
Merge #302: Add not_so_dummy CPFP
d4d78e3 
2699a1b Commit xpub_converter Cargo.lock (Daniela Brozzoni)
177a551 bitcoind: Remove wrong comment (Daniela Brozzoni)
ea9b8a5 tests: Add CPFP tests (Daniela Brozzoni)
1f4b4b7 tests: Pass priority in unvault/spend utilities (Daniela Brozzoni)
b834a5d tests: Store the CPFP priv_key in cpfp_secret (Daniela Brozzoni)
f82ea23 CPFP transactions when needed (Daniela Brozzoni)
902aa66 Core utils for the CPFP transaction, fee estimation (Daniela Brozzoni)
71300df db, api: Allow CPFP priority storing (Daniela Brozzoni)
5624d26 bitcoind: panic on API breaks (Daniela Brozzoni)
f838d4a bitcoind: Isolate listunspent method (Daniela Brozzoni)
eda096d Import CPFP descriptor into Core and... ...cpfp_secret in revaultd (Daniela Brozzoni)
18494c6 WIP: Update revault_tx, revault_net (Daniela Brozzoni)

Pull request description:

  # Warning: for testing this PR you need to re-generate your CPFP descriptors using mscompiler!

  ## Still TODO:
  - [ ] Proper fee estimation! We know that core says "idk ¯\_(ツ)_/¯" quite often, let's estimate the fees by ourselves instead of doing nothing
  - [ ] a `cpfp` RPC command to manually cpfp a spend/unvault, or maybe a `prioritize` to change the `priority` status is enough

ACKs for top commit:
  darosior:
    ACK 2699a1b -- not based on the code but because of the followup in #322.

Tree-SHA512: 81754a3a41e45b66e4cc1181006c938a996f848e5bcaf64c5d5e3c11d5851c8f4e1420e30553d656e41b2022861faebcbe3eef5dda4efe7f64f172b62d42a26a
@darosior
Copy link
Member Author

I think this would increase startup time by a lot, prioritizing.

@JSwambo JSwambo linked a pull request Feb 7, 2022 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JSwambo JSwambo

Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Compute checksums for wallet descriptors without bitcoind JSwambo/revaultd
2 participants
@JSwambo @darosior