Skip to content

Commit 77a2f5d

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#23334: fuzz: Descriptor wallet
1111516 ci: Build fuzz with libsqlite3-dev (MarcoFalke) fa7c6ef fuzz: Add wallet fuzz test (MarcoFalke) fa59d2c refactor: Use local args instead of global gArgs in CWallet::Create (MarcoFalke) fadb446 build: Inline FUZZ_SUITE_LDFLAGS_COMMON (MarcoFalke) Pull request description: Initial sketch to fuzz descriptor wallets. Can be improved in the future. ACKs for top commit: mjdietzx: Code review ACK 1111516 Tree-SHA512: b1d2f24504d1ed5f3c6a031210f04c27c13d4e15576c4acbf50ded37ac45f7b7a5c7553e91d81d4a06e9ea73b3d745a552218d3ef3b2932fa5325a8331b0d3fd
2 parents 24abd83 + 1111516 commit 77a2f5d

File tree

6 files changed

+185
-10
lines changed

6 files changed

+185
-10
lines changed

ci/test/00_setup_env_native_fuzz.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8
88

99
export DOCKER_NAME_TAG="ubuntu:20.04"
1010
export CONTAINER_NAME=ci_native_fuzz
11-
export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev"
11+
export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev libsqlite3-dev"
1212
export NO_DEPENDS=1
1313
export RUN_UNIT_TESTS=false
1414
export RUN_FUNCTIONAL_TESTS=false

ci/test/00_setup_env_native_fuzz_with_valgrind.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8
88

99
export DOCKER_NAME_TAG="ubuntu:20.04"
1010
export CONTAINER_NAME=ci_native_fuzz_valgrind
11-
export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev valgrind"
11+
export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev libsqlite3-dev valgrind"
1212
export NO_DEPENDS=1
1313
export RUN_UNIT_TESTS=false
1414
export RUN_FUNCTIONAL_TESTS=false

configure.ac

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1244,7 +1244,6 @@ if test "x$enable_fuzz" = "xyes"; then
12441244
bitcoin_enable_qt=no
12451245
bitcoin_enable_qt_test=no
12461246
bitcoin_enable_qt_dbus=no
1247-
enable_wallet=no
12481247
use_bench=no
12491248
use_external_signer=no
12501249
use_upnp=no

src/Makefile.test.include

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ if USE_BDB
170170
BITCOIN_TESTS += wallet/test/db_tests.cpp
171171
endif
172172

173+
if USE_SQLITE
174+
FUZZ_WALLET_SRC = \
175+
wallet/test/fuzz/notifications.cpp
176+
endif # USE_SQLITE
173177

174178
BITCOIN_TEST_SUITE += \
175179
wallet/test/util.cpp \
@@ -178,7 +182,7 @@ BITCOIN_TEST_SUITE += \
178182
wallet/test/wallet_test_fixture.h \
179183
wallet/test/init_test_fixture.cpp \
180184
wallet/test/init_test_fixture.h
181-
endif
185+
endif # ENABLE_WALLET
182186

183187
test_test_bitcoin_SOURCES = $(BITCOIN_TEST_SUITE) $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES)
184188
test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(TESTDEFS) $(EVENT_CFLAGS)
@@ -199,14 +203,13 @@ test_test_bitcoin_LDADD += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS)
199203
FUZZ_SUITE_LD_COMMON += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS)
200204
endif
201205

202-
FUZZ_SUITE_LDFLAGS_COMMON = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS)
203-
204206
if ENABLE_FUZZ_BINARY
205207
test_fuzz_fuzz_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
206208
test_fuzz_fuzz_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
207209
test_fuzz_fuzz_LDADD = $(FUZZ_SUITE_LD_COMMON)
208-
test_fuzz_fuzz_LDFLAGS = $(FUZZ_SUITE_LDFLAGS_COMMON) $(RUNTIME_LDFLAGS)
210+
test_fuzz_fuzz_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS) $(RUNTIME_LDFLAGS)
209211
test_fuzz_fuzz_SOURCES = \
212+
$(FUZZ_WALLET_SRC) \
210213
test/fuzz/addition_overflow.cpp \
211214
test/fuzz/addrman.cpp \
212215
test/fuzz/asmap.cpp \
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
// Copyright (c) 2021 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <test/fuzz/FuzzedDataProvider.h>
6+
#include <test/fuzz/fuzz.h>
7+
#include <test/fuzz/util.h>
8+
#include <test/util/setup_common.h>
9+
#include <util/translation.h>
10+
#include <wallet/context.h>
11+
#include <wallet/receive.h>
12+
#include <wallet/wallet.h>
13+
#include <wallet/walletdb.h>
14+
#include <wallet/walletutil.h>
15+
16+
#include <cassert>
17+
#include <cstdint>
18+
#include <string>
19+
#include <vector>
20+
21+
namespace {
22+
const TestingSetup* g_setup;
23+
24+
void initialize_setup()
25+
{
26+
static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
27+
g_setup = testing_setup.get();
28+
}
29+
30+
/**
31+
* Wraps a descriptor wallet for fuzzing. The constructor writes the sqlite db
32+
* to disk, the destructor deletes it.
33+
*/
34+
struct FuzzedWallet {
35+
ArgsManager args;
36+
WalletContext context;
37+
std::shared_ptr<CWallet> wallet;
38+
FuzzedWallet(const std::string& name)
39+
{
40+
context.args = &args;
41+
context.chain = g_setup->m_node.chain.get();
42+
43+
DatabaseOptions options;
44+
options.require_create = true;
45+
options.create_flags = WALLET_FLAG_DESCRIPTORS;
46+
const std::optional<bool> load_on_start;
47+
gArgs.ForceSetArg("-keypool", "0"); // Avoid timeout in TopUp()
48+
49+
DatabaseStatus status;
50+
bilingual_str error;
51+
std::vector<bilingual_str> warnings;
52+
wallet = CreateWallet(context, name, load_on_start, options, status, error, warnings);
53+
assert(wallet);
54+
assert(error.empty());
55+
assert(warnings.empty());
56+
assert(wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
57+
}
58+
~FuzzedWallet()
59+
{
60+
const auto name{wallet->GetName()};
61+
std::vector<bilingual_str> warnings;
62+
std::optional<bool> load_on_start;
63+
assert(RemoveWallet(context, wallet, load_on_start, warnings));
64+
assert(warnings.empty());
65+
UnloadWallet(std::move(wallet));
66+
fs::remove_all(GetWalletDir() / name);
67+
}
68+
CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider)
69+
{
70+
auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)};
71+
if (type == OutputType::BECH32M) {
72+
type = OutputType::BECH32; // TODO: Setup taproot descriptor and remove this line
73+
}
74+
CTxDestination dest;
75+
bilingual_str error;
76+
if (fuzzed_data_provider.ConsumeBool()) {
77+
assert(wallet->GetNewDestination(type, "", dest, error));
78+
} else {
79+
assert(wallet->GetNewChangeDestination(type, dest, error));
80+
}
81+
assert(error.empty());
82+
return GetScriptForDestination(dest);
83+
}
84+
};
85+
86+
FUZZ_TARGET_INIT(wallet_notifications, initialize_setup)
87+
{
88+
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
89+
// The total amount, to be distributed to the wallets a and b in txs
90+
// without fee. Thus, the balance of the wallets should always equal the
91+
// total amount.
92+
const auto total_amount{ConsumeMoney(fuzzed_data_provider)};
93+
FuzzedWallet a{"fuzzed_wallet_a"};
94+
FuzzedWallet b{"fuzzed_wallet_b"};
95+
96+
// Keep track of all coins in this test.
97+
// Each tuple in the chain represents the coins and the block created with
98+
// those coins. Once the block is mined, the next tuple will have an empty
99+
// block and the freshly mined coins.
100+
using Coins = std::set<std::tuple<CAmount, COutPoint>>;
101+
std::vector<std::tuple<Coins, CBlock>> chain;
102+
{
103+
// Add the inital entry
104+
chain.emplace_back();
105+
auto& [coins, block]{chain.back()};
106+
coins.emplace(total_amount, COutPoint{uint256::ONE, 1});
107+
}
108+
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 200)
109+
{
110+
CallOneOf(
111+
fuzzed_data_provider,
112+
[&] {
113+
auto& [coins_orig, block]{chain.back()};
114+
// Copy the coins for this block and consume all of them
115+
Coins coins = coins_orig;
116+
while (!coins.empty()) {
117+
// Create a new tx
118+
CMutableTransaction tx{};
119+
// Add some coins as inputs to it
120+
auto num_inputs{fuzzed_data_provider.ConsumeIntegralInRange<int>(1, coins.size())};
121+
CAmount in{0};
122+
while (num_inputs-- > 0) {
123+
const auto& [coin_amt, coin_outpoint]{*coins.begin()};
124+
in += coin_amt;
125+
tx.vin.emplace_back(coin_outpoint);
126+
coins.erase(coins.begin());
127+
}
128+
// Create some outputs spending all inputs, without fee
129+
LIMITED_WHILE(in > 0 && fuzzed_data_provider.ConsumeBool(), 100)
130+
{
131+
const auto out_value{ConsumeMoney(fuzzed_data_provider, in)};
132+
in -= out_value;
133+
auto& wallet{fuzzed_data_provider.ConsumeBool() ? a : b};
134+
tx.vout.emplace_back(out_value, wallet.GetScriptPubKey(fuzzed_data_provider));
135+
}
136+
// Spend the remaining input value, if any
137+
auto& wallet{fuzzed_data_provider.ConsumeBool() ? a : b};
138+
tx.vout.emplace_back(in, wallet.GetScriptPubKey(fuzzed_data_provider));
139+
// Add tx to block
140+
block.vtx.emplace_back(MakeTransactionRef(tx));
141+
}
142+
// Mine block
143+
a.wallet->blockConnected(block, chain.size());
144+
b.wallet->blockConnected(block, chain.size());
145+
// Store the coins for the next block
146+
Coins coins_new;
147+
for (const auto& tx : block.vtx) {
148+
uint32_t i{0};
149+
for (const auto& out : tx->vout) {
150+
coins_new.emplace(out.nValue, COutPoint{tx->GetHash(), i++});
151+
}
152+
}
153+
chain.emplace_back(coins_new, CBlock{});
154+
},
155+
[&] {
156+
if (chain.size() <= 1) return; // The first entry can't be removed
157+
auto& [coins, block]{chain.back()};
158+
if (block.vtx.empty()) return; // Can only disconnect if the block was submitted first
159+
// Disconnect block
160+
a.wallet->blockDisconnected(block, chain.size() - 1);
161+
b.wallet->blockDisconnected(block, chain.size() - 1);
162+
chain.pop_back();
163+
});
164+
auto& [coins, first_block]{chain.front()};
165+
if (!first_block.vtx.empty()) {
166+
// Only check balance when at least one block was submitted
167+
const auto bal_a{GetBalance(*a.wallet).m_mine_trusted};
168+
const auto bal_b{GetBalance(*b.wallet).m_mine_trusted};
169+
assert(total_amount == bal_a + bal_b);
170+
}
171+
}
172+
}
173+
} // namespace

src/wallet/wallet.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2732,11 +2732,11 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
27322732
walletInstance->m_default_max_tx_fee = max_fee.value();
27332733
}
27342734

2735-
if (gArgs.IsArgSet("-consolidatefeerate")) {
2736-
if (std::optional<CAmount> consolidate_feerate = ParseMoney(gArgs.GetArg("-consolidatefeerate", ""))) {
2735+
if (args.IsArgSet("-consolidatefeerate")) {
2736+
if (std::optional<CAmount> consolidate_feerate = ParseMoney(args.GetArg("-consolidatefeerate", ""))) {
27372737
walletInstance->m_consolidate_feerate = CFeeRate(*consolidate_feerate);
27382738
} else {
2739-
error = AmountErrMsg("consolidatefeerate", gArgs.GetArg("-consolidatefeerate", ""));
2739+
error = AmountErrMsg("consolidatefeerate", args.GetArg("-consolidatefeerate", ""));
27402740
return nullptr;
27412741
}
27422742
}

0 commit comments

Comments
 (0)