fuzz: wallet: add target for spkm migration #29694

brunoerg · 2024-03-21T17:35:09Z

This PR adds a fuzz target for ScriptPubKeyMan migration. It creates a LegacyDataSPKM which can have some keys/scripts/etc, and then migrate it to descriptor. I tried to keep it as compatible as possible with future legacy wallet removal.

DrahtBot · 2024-03-21T17:35:11Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/29694.

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:

#25722 (refactor: Use util::Result class for wallet loading by ryanofsky)

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.

brunoerg · 2024-05-27T17:57:39Z

cc: @achow101

src/wallet/test/fuzz/scriptpubkeyman.cpp

brunoerg · 2024-10-23T09:32:41Z

Inputs for this are available at: brunoerg/qa-assets#2

maflcko · 2024-10-23T17:28:43Z

Coverage for this target: https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/2e7ac47bbc28c2cb/300ddaa72f7e4b9b/fuzz.coverage/index.html

src/wallet/test/fuzz/scriptpubkeyman.cpp

brunoerg · 2024-11-11T16:57:52Z

Force-pushed addressing (multisig and hd chain cover):

furszy

Would be good to ensure that #31374 can be replicated here.

DrahtBot · 2024-11-26T19:49:01Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/33561912794}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

brunoerg · 2024-11-26T20:48:04Z

Force-pushed addressing #29694 (comment)

brunoerg · 2024-11-26T20:48:31Z

Would be good to ensure that #31374 can be replicated here.

Since this target is only for spkm migration, I don't think it can be replicated here.

Edit: I can add ApplyMigrationData here and reproduce the crash from #31374. I'm going to move this to draft while working on it.

brunoerg · 2025-01-23T11:39:47Z

Force-pushed adding SeedRandomStateForTest and SetMockTime for better stability.

brunoerg · 2025-01-27T19:02:05Z

Rebased

DrahtBot · 2025-02-03T16:12:27Z

https://cirrus-ci.com/task/5046981191532544?logs=ci#L1799

[22:03:02.859]  test  2025-01-27T22:03:01.984000Z TestFramework (ERROR): Assertion failed 
[22:03:02.859]                                    Traceback (most recent call last):
[22:03:02.859]                                      File "/ci_container_base/test/functional/test_framework/test_framework.py", line 135, in main
[22:03:02.859]                                        self.run_test()
[22:03:02.859]                                      File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/functional/wallet_migration.py", line 1098, in run_test
[22:03:02.859]                                        self.test_failed_migration_cleanup()
[22:03:02.859]                                      File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/functional/wallet_migration.py", line 899, in test_failed_migration_cleanup
[22:03:02.859]                                        assert all(wallet not in self.master_node.listwallets() for wallet in ["failed", "failed_watchonly", "failed_solvables"])
[22:03:02.859]                                    AssertionError

For in-memory SQLite databases, exclusive locking is generally not necessary because in-memory dbs are private to the connection that created them.

By using in-memory databases, we do not have dbs file to delete and restore.

brunoerg · 2025-02-05T16:58:24Z

Just fixed CI, ready for review!

DrahtBot added the Tests label Mar 21, 2024

brunoerg mentioned this pull request Mar 25, 2024

Proposed Timeline for Legacy Wallet and BDB removal #20160

Open

48 tasks

brunoerg mentioned this pull request Apr 17, 2024

Wallet fuzzing tracking issue #29901

Open

11 tasks

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from 114066e to d0c3ca9 Compare April 19, 2024 08:49

furszy reviewed Jun 16, 2024

View reviewed changes

src/wallet/test/fuzz/scriptpubkeyman.cpp Outdated Show resolved Hide resolved

Mazzika1 approved these changes Jun 16, 2024

View reviewed changes

brunoerg marked this pull request as draft June 25, 2024 20:37

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from d0c3ca9 to 2e7ac47 Compare September 9, 2024 19:02

DrahtBot added the CI failed label Sep 10, 2024

brunoerg marked this pull request as ready for review September 10, 2024 11:28

DrahtBot removed the CI failed label Sep 11, 2024

achow101 self-requested a review October 15, 2024 15:53

achow101 reviewed Nov 4, 2024

View reviewed changes

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from 2e7ac47 to 77c31c8 Compare November 11, 2024 16:56

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from 77c31c8 to ed1a16b Compare November 26, 2024 19:33

furszy reviewed Nov 26, 2024

View reviewed changes

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from ed1a16b to cdc9458 Compare November 26, 2024 19:48

DrahtBot added the CI failed label Nov 26, 2024

DrahtBot removed the CI failed label Nov 26, 2024

brunoerg marked this pull request as draft November 27, 2024 13:13

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch 2 times, most recently from 1421097 to 9750b3f Compare December 2, 2024 12:27

brunoerg marked this pull request as ready for review December 10, 2024 09:29

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from dd7a8f0 to 248ef32 Compare December 11, 2024 09:25

DrahtBot added the CI failed label Dec 11, 2024

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch 2 times, most recently from 0805b08 to 94c3bf1 Compare December 11, 2024 12:43

DrahtBot removed the CI failed label Dec 11, 2024

brunoerg mentioned this pull request Dec 11, 2024

add inputs for spkm_migration target bitcoin-core/qa-assets#215

Draft

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from 94c3bf1 to 5e2e916 Compare January 23, 2025 11:39

DrahtBot added the Needs rebase label Jan 25, 2025

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from 5e2e916 to 457f1d4 Compare January 27, 2025 18:53

DrahtBot added CI failed and removed Needs rebase labels Jan 27, 2025

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch 2 times, most recently from a7749a9 to cbffd66 Compare January 27, 2025 21:47

brunoerg marked this pull request as draft February 3, 2025 16:41

brunoerg added 4 commits February 5, 2025 10:03

wallet: add option to create in-memory SQLite DB in the migration

645dd3e

wallet: sqlite: there is no need to have exclusive locking when mocking

cccac21

For in-memory SQLite databases, exclusive locking is generally not necessary because in-memory dbs are private to the connection that created them.

wallet: skip backup restoration when using in-memory dbs

e449f9c

By using in-memory databases, we do not have dbs file to delete and restore.

fuzz: wallet: add target for spkm migration

0c0e79d

brunoerg force-pushed the 2024-03-fuzz-spkm-migration branch from cbffd66 to 0c0e79d Compare February 5, 2025 13:09

DrahtBot removed the CI failed label Feb 5, 2025

brunoerg marked this pull request as ready for review February 5, 2025 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fuzz: wallet: add target for spkm migration #29694

fuzz: wallet: add target for spkm migration #29694

brunoerg commented Mar 21, 2024 •

edited

Loading

DrahtBot commented Mar 21, 2024 •

edited

Loading

brunoerg commented May 27, 2024

brunoerg commented Oct 23, 2024

maflcko commented Oct 23, 2024

brunoerg commented Nov 11, 2024

furszy left a comment

DrahtBot commented Nov 26, 2024

brunoerg commented Nov 26, 2024

brunoerg commented Nov 26, 2024 •

edited

Loading

brunoerg commented Jan 23, 2025

brunoerg commented Jan 27, 2025

DrahtBot commented Feb 3, 2025

brunoerg commented Feb 5, 2025

fuzz: wallet: add target for spkm migration #29694

Are you sure you want to change the base?

fuzz: wallet: add target for spkm migration #29694

Conversation

brunoerg commented Mar 21, 2024 • edited Loading

DrahtBot commented Mar 21, 2024 • edited Loading

Code Coverage & Benchmarks

Reviews

Conflicts

brunoerg commented May 27, 2024

brunoerg commented Oct 23, 2024

maflcko commented Oct 23, 2024

brunoerg commented Nov 11, 2024

furszy left a comment

Choose a reason for hiding this comment

DrahtBot commented Nov 26, 2024

brunoerg commented Nov 26, 2024

brunoerg commented Nov 26, 2024 • edited Loading

brunoerg commented Jan 23, 2025

brunoerg commented Jan 27, 2025

DrahtBot commented Feb 3, 2025

brunoerg commented Feb 5, 2025

brunoerg commented Mar 21, 2024 •

edited

Loading

DrahtBot commented Mar 21, 2024 •

edited

Loading

brunoerg commented Nov 26, 2024 •

edited

Loading