wallet: Introduce and use DummyDatabase instead of dummy BerkeleyDatabase #19102

1. 
   achow101 commented at 10:13 pm on May 28, 2020: member

   In the unit tests, we use a dummy WalletDatabase which does nothing and always returns true. This is currently implemented by creating a BerkeleyDatabase in dummy mode. This PR instead adds a DummyDatabase class which does nothing and never fails for use in the tests. CreateDummyWalletDatabase is changed to return this DummyDatabase and BerkeleyDatabase is cleaned up to remove all of the checks for IsDummy.

   Based on WalletDatabase abstract class introduced in #19334

2. fanquake added the label Wallet on May 28, 2020
3. 
   DrahtBot commented at 6:43 am on May 29, 2020: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #19619 (Remove wallet.dat path handling from wallet.cpp, rpcwallet.cpp 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.

4. 
   practicalswift commented at 2:22 pm on May 30, 2020: contributor

   Concept ACK

   Inspired by DummyDatabase I’ll try to find time to do create a FuzzedDatabase implementing the WalletDatabase interface but with data feeded by FuzzedDataProvider :)

5. achow101 force-pushed on Jun 1, 2020
6. achow101 force-pushed on Jun 2, 2020
7. DrahtBot added the label Needs rebase on Jun 2, 2020
8. achow101 force-pushed on Jun 2, 2020
9. DrahtBot removed the label Needs rebase on Jun 2, 2020
10. 
    laanwj commented at 3:20 pm on June 4, 2020: member

    Concept ACK, IsDummy was always a bit of a hack and now that there is a proper wallet database interface, creating a dummy at that level makes sense.
11. 
    luke-jr commented at 4:35 pm on June 5, 2020: member

    Do we lose testing of bdb this way?
12. 
    achow101 commented at 4:48 pm on June 5, 2020: member

    Do we lose testing of bdb this way?

    No. IsDummy was only used right at the beginning of functions so caused them to do nothing anyways. In places that may have been called but didn’t check IsDummy, they would have exited early anyways due to no m_db being set.

13. achow101 force-pushed on Jun 9, 2020
14. achow101 force-pushed on Jun 16, 2020
15. DrahtBot added the label Needs rebase on Jun 18, 2020
16. achow101 force-pushed on Jun 18, 2020
17. DrahtBot removed the label Needs rebase on Jun 18, 2020
18. achow101 force-pushed on Jun 18, 2020
19. achow101 force-pushed on Jun 19, 2020
20. meshcollider added this to the "PRs" column in a project

    ---

21. achow101 force-pushed on Jun 20, 2020
22. achow101 force-pushed on Jun 22, 2020
23. ryanofsky approved
24. 
    ryanofsky commented at 1:18 pm on June 23, 2020: member

    Code review ACK 2a03ed863b8202b57392babcf94969220d7168e6 (just last three commits). Nice cleanup. It would be good to mention in PR description this is based on #19334
25. 
    ryanofsky commented at 1:28 pm on June 23, 2020: member

    Could consider moving new classes to src/wallet/test/ since they are only used to speed up unit tests and it would be confusing if they were ever used in real code
26. 
    achow101 commented at 4:30 pm on June 23, 2020: member

    Could consider moving new classes to src/wallet/test/ since they are only used to speed up unit tests and it would be confusing if they were ever used in real code

    SalvageWallet uses DummyDatabase for a dummy wallet.

27. achow101 force-pushed on Jun 23, 2020
28. DrahtBot added the label Needs rebase on Jul 1, 2020
29. achow101 force-pushed on Jul 1, 2020
30. achow101 force-pushed on Jul 1, 2020
31. DrahtBot removed the label Needs rebase on Jul 1, 2020
32. DrahtBot added the label Needs rebase on Jul 5, 2020
33. achow101 force-pushed on Jul 6, 2020
34. DrahtBot removed the label Needs rebase on Jul 6, 2020
35. DrahtBot added the label Needs rebase on Jul 8, 2020
36. achow101 force-pushed on Jul 9, 2020
37. DrahtBot removed the label Needs rebase on Jul 9, 2020
38. achow101 force-pushed on Jul 14, 2020
39. ryanofsky approved
40. 
    ryanofsky commented at 8:35 pm on July 21, 2020: member

    Code review ACK 1937a88a76548979da8559fd3e959fda7a569901 last 3 commits. Easy review and nice cleanup

    • 23f633c19c65957f0233ec2b2c1a5555006abe37 Introduce DummyDatabase and use it in the tests (5/7)
    • 53c1f553a13077782a6bdf28d186467d320af74f Remove BDB dummy databases (6/7)
    • 1937a88a76548979da8559fd3e959fda7a569901 walletdb: Ensure that having no database handle is a failure (7/7)
41. achow101 force-pushed on Jul 23, 2020
42. ryanofsky approved
43. 
    ryanofsky commented at 7:32 pm on July 24, 2020: member

    Code review ACK d93adba3f9d3fde4c0a79a62b13e7759fb892466. No changes since last review other than rebase.
44. Introduce DummyDatabase and use it in the tests 0103d6434e
45. Remove BDB dummy databases da039d2a91
46. walletdb: Ensure that having no database handle is a failure

    Previously having no database handle could still be considered a success
    when BerkeleyDatabase and BerkeleyBatch were used for dummy database
    things. With dedicated DummyDatabase and DummyBatch classes now, these
    should fail.

    0fcff547d5
47. achow101 force-pushed on Jul 29, 2020
48. 
    instagibbs commented at 2:32 pm on July 30, 2020: member

    utACK https://github.com/bitcoin/bitcoin/pull/19102/commits/0fcff547d5b47822c13104978fda0c486e596526
49. 
    MarcoFalke commented at 3:00 pm on July 30, 2020: member

    crACK 0fcff547d5b47822c13104978fda0c486e596526 🚈

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
     1Hash: SHA512
     2
     3crACK 0fcff547d5b47822c13104978fda0c486e596526 🚈
     4-----BEGIN PGP SIGNATURE-----
     5
     6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
     7pUgGZgv+JeOhpzNalFjNtnK6xDi2rqXcqrtHiNvIa4TGToO6uY5Xh7cIyWrPq6nP
     8a93uJ8Sagwa+Wspt+MCC1bLmERIvBhfYzr2/GQUbTOlXzR0u5/B2wTNywN23vyNs
     927Rjt6cQdE+0XRMNjHPbuQaZw16CdRdMGoW1Tn8fjceRCRHQg0BZaOH9UzWBbDra
    1042UVvVdmBQiPuYtRrrFOHy0BMJefkPxiYgRoHM6oLordMYH7TnC7RZf+Rn1Kpyk+
    11/ZqGWl+eRk+Yz5eCizArmzFobcCvzY2VvUsLeg+xq1k1oIbStuCrV2QsLaYRCp4V
    12Hm180u0sq65gCljrvjnLCKQO/B3tqcR4jsQA5Vy6/PM7yZEHWLKrw4D2H/QvRG/S
    13aDZrZSO8ZX80MAc6cxLYmZ8cMiWzzet52GIFD5p+QhLhEPQAFhKLBpbQIObA6/L5
    14X4ozPsQqQj3PiL8ynUTqjDFFpb4yrCOiMBEP1HwBrrAgfMgQtYMaGPnkMC6qHgC0
    15l6UNOH28
    16=32me
    17-----END PGP SIGNATURE-----
    

    Timestamp of file with hash acb069d85216bbd6e4cfaa47f6c835d6684dd52d7006bbeee8a4fbe3e90dca14 -

50. MarcoFalke merged this on Jul 30, 2020
51. MarcoFalke closed this on Jul 30, 2020

    ---

52. meshcollider moved this from the "PRs" to the "Done" column in a project

    ---

53. deadalnix referenced this in commit 636082e6d6 on Sep 6, 2021
54. Fabcien referenced this in commit 5948655c33 on Sep 6, 2021
55. Fabcien referenced this in commit 2e5de08a80 on Sep 6, 2021
56. MarcoFalke locked this on Feb 15, 2022

Contributors
achow101 DrahtBot practicalswift laanwj luke-jr ryanofsky instagibbs MarcoFalke

Labels
Wallet