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

pull achow101 wants to merge 3 commits into bitcoin:master from achow101:true-dummydb changing 4 files +47 −36
  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. achow101 cross-referenced this on May 28, 2020 from issue wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets by achow101
  3. fanquake added the label Wallet on May 28, 2020
  4. DrahtBot commented at 6:43 AM on May 29, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    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.

  5. DrahtBot cross-referenced this on May 29, 2020 from issue refactor: remove ::vpwallets and related global variables by ryanofsky
  6. DrahtBot cross-referenced this on May 29, 2020 from issue refactor: Move wallet methods out of chain.h and node.h by ryanofsky
  7. DrahtBot cross-referenced this on May 29, 2020 from issue test: Remove duplicate NodeContext hacks by ryanofsky
  8. DrahtBot cross-referenced this on May 29, 2020 from issue Refactor: clean up PeriodicFlush() by jnewbery
  9. DrahtBot cross-referenced this on May 29, 2020 from issue walletdb: Don't remove database transaction logs and instead error by achow101
  10. DrahtBot cross-referenced this on May 29, 2020 from issue Don't call lsn_reset in periodic flush by bvbfan
  11. DrahtBot cross-referenced this on May 29, 2020 from issue wallet: Remove boost from PeriodicFlush by MarcoFalke
  12. DrahtBot cross-referenced this on May 29, 2020 from issue refactor: Make CCheckQueue RAII-styled by hebasto
  13. DrahtBot cross-referenced this on May 29, 2020 from issue Add local thread pool to CCheckQueue by hebasto
  14. DrahtBot cross-referenced this on May 29, 2020 from issue refactor: Remove CAddressBookData::destdata by ryanofsky
  15. DrahtBot cross-referenced this on May 29, 2020 from issue Use shared pointers only in validation interface by bvbfan
  16. 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 :)

  17. DrahtBot cross-referenced this on Jun 1, 2020 from issue gui: Drop RecentRequestsTableModel dependency to WalletModel by promag
  18. achow101 force-pushed on Jun 1, 2020
  19. DrahtBot cross-referenced this on Jun 2, 2020 from issue wallettool: Add dump and createfromdump commands by achow101
  20. achow101 force-pushed on Jun 2, 2020
  21. DrahtBot added the label Needs rebase on Jun 2, 2020
  22. achow101 force-pushed on Jun 2, 2020
  23. DrahtBot removed the label Needs rebase on Jun 2, 2020
  24. 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.

  25. luke-jr commented at 4:35 PM on June 5, 2020: member

    Do we lose testing of bdb this way?

  26. 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.

  27. DrahtBot cross-referenced this on Jun 9, 2020 from issue wallet: Remove first parameter to ScanForWalletTransactions start_hash by pstratem
  28. achow101 force-pushed on Jun 9, 2020
  29. DrahtBot cross-referenced this on Jun 16, 2020 from issue wallet: Refactor BerkeleyBatch Read, Write, Erase, and Exists functions into non-template functions by achow101
  30. DrahtBot cross-referenced this on Jun 16, 2020 from issue wallet: move BDB specific classes to bdb.{cpp/h} by achow101
  31. DrahtBot cross-referenced this on Jun 16, 2020 from issue wallet: Refactor the classes in wallet/db.{cpp/h} by achow101
  32. achow101 force-pushed on Jun 16, 2020
  33. DrahtBot cross-referenced this on Jun 16, 2020 from issue Replace boost::filesystem with std::filesystem by kiminuo
  34. DrahtBot cross-referenced this on Jun 17, 2020 from issue wallet: BerkeleyDatabase make BerkeleyDatabase::Create, CreateMock, and CreateDummy non-static functions by achow101
  35. DrahtBot cross-referenced this on Jun 17, 2020 from issue wallet: BerkeleyBatch Handle cursor internally by achow101
  36. DrahtBot added the label Needs rebase on Jun 18, 2020
  37. achow101 force-pushed on Jun 18, 2020
  38. DrahtBot removed the label Needs rebase on Jun 18, 2020
  39. achow101 force-pushed on Jun 18, 2020
  40. DrahtBot cross-referenced this on Jun 19, 2020 from issue wallet: Move BerkeleyBatch static functions to BerkeleyDatabase by achow101
  41. DrahtBot cross-referenced this on Jun 19, 2020 from issue wallet: Replace CDataStream& with CDataStream&& where appropriate by MarcoFalke
  42. achow101 force-pushed on Jun 19, 2020
  43. meshcollider added this to the "PRs" column in a project
  44. DrahtBot cross-referenced this on Jun 19, 2020 from issue wallet: Refactor BerkeleyDatabase to introduce DatabaseBatch abstract class by achow101
  45. achow101 force-pushed on Jun 20, 2020
  46. DrahtBot cross-referenced this on Jun 20, 2020 from issue wallet: Cleanup and separate BerkeleyDatabase and BerkeleyBatch by achow101
  47. achow101 force-pushed on Jun 22, 2020
  48. ryanofsky approved
  49. ryanofsky commented at 1:18 PM on June 23, 2020: contributor

    Code review ACK 2a03ed863b8202b57392babcf94969220d7168e6 (just last three commits). Nice cleanup. It would be good to mention in PR description this is based on #19334

  50. ryanofsky commented at 1:28 PM on June 23, 2020: contributor

    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

  51. 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.

  52. achow101 force-pushed on Jun 23, 2020
  53. DrahtBot added the label Needs rebase on Jul 1, 2020
  54. achow101 force-pushed on Jul 1, 2020
  55. achow101 force-pushed on Jul 1, 2020
  56. DrahtBot removed the label Needs rebase on Jul 1, 2020
  57. DrahtBot added the label Needs rebase on Jul 5, 2020
  58. achow101 force-pushed on Jul 6, 2020
  59. DrahtBot removed the label Needs rebase on Jul 6, 2020
  60. DrahtBot cross-referenced this on Jul 7, 2020 from issue wallet: Introduce WalletDatabase abstract class by achow101
  61. DrahtBot added the label Needs rebase on Jul 8, 2020
  62. achow101 force-pushed on Jul 9, 2020
  63. DrahtBot removed the label Needs rebase on Jul 9, 2020
  64. achow101 force-pushed on Jul 14, 2020
  65. ryanofsky approved
  66. ryanofsky commented at 8:35 PM on July 21, 2020: contributor

    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)
  67. achow101 force-pushed on Jul 23, 2020
  68. ryanofsky approved
  69. ryanofsky commented at 7:32 PM on July 24, 2020: contributor

    Code review ACK d93adba3f9d3fde4c0a79a62b13e7759fb892466. No changes since last review other than rebase.

  70. DrahtBot cross-referenced this on Jul 29, 2020 from issue Remove wallet.dat path handling from wallet.cpp, rpcwallet.cpp by ryanofsky
  71. Introduce DummyDatabase and use it in the tests 0103d6434e
  72. Remove BDB dummy databases da039d2a91
  73. 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
  74. achow101 force-pushed on Jul 29, 2020
  76. MarcoFalke commented at 3:00 PM on July 30, 2020: member

    crACK 0fcff547d5b47822c13104978fda0c486e596526 🚈

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

crACK 0fcff547d5b47822c13104978fda0c486e596526 🚈
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgGZgv+JeOhpzNalFjNtnK6xDi2rqXcqrtHiNvIa4TGToO6uY5Xh7cIyWrPq6nP
a93uJ8Sagwa+Wspt+MCC1bLmERIvBhfYzr2/GQUbTOlXzR0u5/B2wTNywN23vyNs
27Rjt6cQdE+0XRMNjHPbuQaZw16CdRdMGoW1Tn8fjceRCRHQg0BZaOH9UzWBbDra
42UVvVdmBQiPuYtRrrFOHy0BMJefkPxiYgRoHM6oLordMYH7TnC7RZf+Rn1Kpyk+
/ZqGWl+eRk+Yz5eCizArmzFobcCvzY2VvUsLeg+xq1k1oIbStuCrV2QsLaYRCp4V
Hm180u0sq65gCljrvjnLCKQO/B3tqcR4jsQA5Vy6/PM7yZEHWLKrw4D2H/QvRG/S
aDZrZSO8ZX80MAc6cxLYmZ8cMiWzzet52GIFD5p+QhLhEPQAFhKLBpbQIObA6/L5
X4ozPsQqQj3PiL8ynUTqjDFFpb4yrCOiMBEP1HwBrrAgfMgQtYMaGPnkMC6qHgC0
l6UNOH28
=32me
-----END PGP SIGNATURE-----

    Timestamp of file with hash acb069d85216bbd6e4cfaa47f6c835d6684dd52d7006bbeee8a4fbe3e90dca14 -

    </details>

  77. MarcoFalke merged this on Jul 30, 2020
  78. MarcoFalke closed this on Jul 30, 2020
  79. meshcollider moved this from the "PRs" to the "Done" column in a project
  80. deadalnix referenced this in commit 636082e6d6 on Sep 6, 2021
  81. Fabcien referenced this in commit 5948655c33 on Sep 6, 2021
  82. Fabcien referenced this in commit 2e5de08a80 on Sep 6, 2021
  83. bitcoin locked this on Feb 15, 2022
Contributors
achow101DrahtBotpracticalswiftlaanwjluke-jrryanofskyinstagibbsMarcoFalke
Labels
Wallet
Linked (view graph)
#18971 wallet: Refactor the classes in wallet/db.{cpp/h}#19077 wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets#19334 wallet: Introduce WalletDatabase abstract class
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 12:53 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me