test: Add wallet_balance benchmark

MarcoFalke commented at 10:19 pm on April 9, 2019: member

fanquake added the label Tests on Apr 9, 2019

MarcoFalke force-pushed on Apr 10, 2019

MarcoFalke commented at 2:16 pm on April 11, 2019: member

Going to reopen next week

MarcoFalke closed this on Apr 11, 2019

MarcoFalke reopened this on Apr 15, 2019

MarcoFalke force-pushed on Apr 15, 2019

bench: Add wallet_balance benchmarks fa46ac3127

MarcoFalke force-pushed on Apr 15, 2019

MarcoFalke commented at 6:24 pm on April 16, 2019: member

this is ready for review now

laanwj commented at 4:05 pm on April 17, 2019: member

utACK fa46ac3127142358116c8473741708b3bb70ca15

in src/bench/wallet_balance.cpp:18 in fa46ac3127 outdated

13+struct WalletTestingSetup {
14+    std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain();
15+    CWallet m_wallet;
16+
17+    WalletTestingSetup()
18+        : m_wallet{m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()}

ryanofsky commented at 4:18 pm on April 17, 2019:

Could get rid of WalletTestingSetup constructor and just initialize m_wallet directly above.

in src/bench/wallet_balance.cpp:35 in fa46ac3127 outdated

30+    const auto& ADDRESS_WATCHONLY = ADDRESS_BCRT1_UNSPENDABLE;
31+
32+    WalletTestingSetup wallet_t{};
33+    auto& wallet = wallet_t.m_wallet;
34+    {
35+        bool first_run;

ryanofsky commented at 4:21 pm on April 17, 2019:

I think there’s no need for this to be indented, but I guess it’s reasonable to limit the scope the first_run variable.

in src/bench/wallet_balance.cpp:13 in fa46ac3127 outdated

 8+#include <optional.h>
 9+#include <test/util.h>
10+#include <validationinterface.h>
11+#include <wallet/wallet.h>
12+
13+struct WalletTestingSetup {

ryanofsky commented at 4:32 pm on April 17, 2019:

This seems fine, but in the future it might be better not have two different structs called WalletTestingSetup with different implementations. (The other one is https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/wallet_test_fixture.h), because if the other WalletTestingSetup struct is ever used in a benchmark, it might result in weird link errors or even runtime errors.

I think if you just took the handleNotifications method below and moved it to the CWallet class as a public method, this struct would be simpler and no longer need to be declared a CWallet friend, and it could move to a private namespace to avoid conflicting with the other struct.

ryanofsky approved

ryanofsky commented at 4:37 pm on April 17, 2019: member

utACK fa46ac3127142358116c8473741708b3bb70ca15

refactor: Add handleNotifications method to wallet

Further stylistic cleanups in touched files:

* Sort the includes
* Wrap long single-line constructors into multiple lines

fad7c33342

MarcoFalke force-pushed on Apr 17, 2019

MarcoFalke commented at 6:54 pm on April 17, 2019: member

Added a method to the wallet to handle notifications (and removed the friend struct workarounds in tests)

ryanofsky approved

ryanofsky commented at 7:07 pm on April 17, 2019: member

utACK fad7c33342cb51b310a7dd372bfa675df8810367. I might squash or rearrange the commits to avoid adding code in one commit that just gets deleted in the next one. But overall this looks good and the cleanup is nice.

MarcoFalke referenced this in commit dae72998e8 on Apr 17, 2019

MarcoFalke merged this on Apr 17, 2019

MarcoFalke closed this on Apr 17, 2019

MarcoFalke deleted the branch on Apr 17, 2019

deadalnix referenced this in commit bf444f46b2 on May 23, 2020

deadalnix referenced this in commit bef2081ba9 on May 23, 2020

in src/test/util.cpp:25 in fad7c33342

21@@ -22,6 +22,8 @@
22 
23 #include <boost/thread.hpp>
24 
25+const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj";

decryp2kanon commented at 1:09 pm on October 23, 2020:

interesting. just curious, how to create this un-spendable address? @MarcoFalke

kittywhiskers referenced this in commit dfe6266664 on Oct 12, 2021

kittywhiskers referenced this in commit 10f0d8502e on Oct 12, 2021

kittywhiskers referenced this in commit 7f8f9b56b0 on Oct 12, 2021

kittywhiskers referenced this in commit f42b6a5c1e on Oct 12, 2021

kittywhiskers referenced this in commit 8bf278a37f on Oct 12, 2021

kittywhiskers referenced this in commit c7ad657fc1 on Oct 25, 2021

kittywhiskers referenced this in commit 597d5e5387 on Oct 25, 2021

kittywhiskers referenced this in commit 046fcc2060 on Oct 25, 2021

UdjinM6 referenced this in commit 61a69cf09b on Oct 25, 2021

kittywhiskers referenced this in commit 092c145395 on Oct 31, 2021

kittywhiskers referenced this in commit 4062f84b71 on Nov 1, 2021

kittywhiskers referenced this in commit b2dec899e7 on Nov 1, 2021

kittywhiskers referenced this in commit 0f0d2b1805 on Nov 1, 2021

kittywhiskers referenced this in commit 6c9ad91b2f on Nov 1, 2021

kittywhiskers referenced this in commit 0c23c7e756 on Nov 4, 2021

kittywhiskers referenced this in commit 41e2707e69 on Nov 4, 2021

kittywhiskers referenced this in commit 1a47ff7fe5 on Nov 6, 2021

kittywhiskers referenced this in commit 3b067a3883 on Nov 14, 2021

kittywhiskers referenced this in commit 0ac021f903 on Nov 14, 2021

kittywhiskers referenced this in commit 0596058467 on Nov 14, 2021

kittywhiskers referenced this in commit 6cc3648fae on Nov 14, 2021

pravblockc referenced this in commit 3ad196bb32 on Nov 18, 2021

pravblockc referenced this in commit 0431ddc8b3 on Nov 18, 2021

DrahtBot locked this on Feb 15, 2022

test: Add wallet_balance benchmark #15779