Skip to content

Conversation

@achow101
Copy link
Member

@achow101 achow101 commented Dec 2, 2022

Based on #26626

This PR changes the legacy to descriptor wallet migration from creating a new combo() for every individual key to creating one large combo() that has a list of every key. This improves the performance of the migrated wallet as it will only have one DescriptorSPKM for all of the non-HD keys.

Note that this does not affect previously migrated wallets. Such wallets will still have a combo() for every key.

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 2, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28333 (wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively by achow101)
  • #22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot added the Wallet label Dec 2, 2022
@achow101 achow101 marked this pull request as draft December 2, 2022 20:12
@achow101 achow101 force-pushed the migrate-nonhd-key-list branch from 5d4005b to 8e60255 Compare December 2, 2022 22:25
@achow101 achow101 force-pushed the migrate-nonhd-key-list branch from 8e60255 to ce7c534 Compare December 12, 2022 16:19
@achow101 achow101 force-pushed the migrate-nonhd-key-list branch from ce7c534 to ff810de Compare January 26, 2023 00:03
@achow101 achow101 force-pushed the migrate-nonhd-key-list branch 2 times, most recently from bdb3f63 to e60fdf3 Compare January 26, 2023 16:07
@achow101 achow101 force-pushed the migrate-nonhd-key-list branch from e60fdf3 to 4d43312 Compare February 1, 2023 21:14
@achow101 achow101 force-pushed the migrate-nonhd-key-list branch from 4d43312 to bdf0dbe Compare February 16, 2023 18:07
@achow101 achow101 force-pushed the migrate-nonhd-key-list branch from bdf0dbe to 5d205b8 Compare February 17, 2023 21:13
@darosior
Copy link
Member

Wouldn't #20018 address most performance concerns of having a large number of DescriptorSPKMs? Albeit at the cost of a slightly higher memory footprint, especially in this case.

@achow101
Copy link
Member Author

Wouldn't #20018 address most performance concerns of having a large number of DescriptorSPKMs? Albeit at the cost of a slightly higher memory footprint, especially in this case.

Yes, but these approaches aren't mutually exclusive, and I'd like to get feedback on both.

achow101 added 6 commits July 21, 2023 11:10
ListPubkeyProvider is a ranged PubkeyProvider that returns individual
public keys that it was constructed with.
A new KEY expression is introduced which takes a list of non-ranged
public keys. This expression itself is ranged and can provide the key at
a particular index. It can take any KEY as long as it is not ranged.
Instead of having several thousand individual combo descriptors, have a
single large combo descriptor with a list of keys. This will improve
performance in IsMine and other similar areas.
@achow101 achow101 force-pushed the migrate-nonhd-key-list branch from 036cf7f to dd1c9af Compare July 21, 2023 15:13
@achow101 achow101 closed this Sep 21, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Sep 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants