diff options
47 files changed, 691 insertions, 142 deletions
diff --git a/configure.ac b/configure.ac index 45def0908b..da71ee93c5 100644 --- a/configure.ac +++ b/configure.ac @@ -837,9 +837,6 @@ if test "$use_lcov_branch" != "no"; then AC_SUBST(LCOV_OPTS, "$LCOV_OPTS --rc lcov_branch_coverage=1") fi -dnl Check for __int128 -AC_CHECK_TYPES([__int128]) - dnl Check for endianness AC_C_BIGENDIAN diff --git a/doc/README.md b/doc/README.md index 4845f00ade..f737fd8a86 100644 --- a/doc/README.md +++ b/doc/README.md @@ -78,6 +78,7 @@ The Bitcoin repo's [root README](/README.md) contains relevant information on th - [I2P Support](i2p.md) - [Init Scripts (systemd/upstart/openrc)](init.md) - [Managing Wallets](managing-wallets.md) +- [Multisig Tutorial](multisig-tutorial.md) - [PSBT support](psbt.md) - [Reduce Memory](reduce-memory.md) - [Reduce Traffic](reduce-traffic.md) diff --git a/doc/multisig-tutorial.md b/doc/multisig-tutorial.md new file mode 100644 index 0000000000..0793040418 --- /dev/null +++ b/doc/multisig-tutorial.md @@ -0,0 +1,241 @@ +# 1. Multisig Tutorial + +Currently, it is possible to create a multisig wallet using Bitcoin Core only. + +Although there is already a brief explanation about the multisig in the [Descriptors documentation](https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md#multisig), this tutorial proposes to use the signet (instead of regtest), bringing the reader closer to a real environment and explaining some functions in more detail. + +This tutorial uses [jq](https://github.com/stedolan/jq) JSON processor to process the results from RPC and stores the relevant values in bash variables. This makes the tutorial reproducible and easier to follow step by step. + +Before starting this tutorial, start the bitcoin node on the signet network. + +```bash +./src/bitcoind -signet -daemon +``` + +This tutorial also uses the default WPKH derivation path to get the xpubs and does not conform to [BIP 45](https://github.com/bitcoin/bips/blob/master/bip-0045.mediawiki) or [BIP 87](https://github.com/bitcoin/bips/blob/master/bip-0087.mediawiki). + +At the time of writing, there is no way to extract a specific path from wallets in Bitcoin Core. For this, an external signer/xpub can be used. + +[PR #22341](https://github.com/bitcoin/bitcoin/pull/22341), which is still under development, introduces a new wallet RPC `getxpub`. It takes a BIP32 path as an argument and returns the xpub, along with the master key fingerprint. + +## 1.1 Basic Multisig Workflow + +### 1.1 Create the Descriptor Wallets + +For a 2-of-3 multisig, create 3 descriptor wallets. It is important that they are of the descriptor type in order to retrieve the wallet descriptors. These wallets contain HD seed and private keys, which will be used to sign the PSBTs and derive the xpub. + +These three wallets should not be used directly for privacy reasons (public key reuse). They should only be used to sign transactions for the (watch-only) multisig wallet. + +```bash +for ((n=1;n<=3;n++)) +do + ./src/bitcoin-cli -signet -named createwallet wallet_name="participant_${n}" descriptors=true +done +``` + +Extract the xpub of each wallet. To do this, the `listdescriptors` RPC is used. By default, Bitcoin Core single-sig wallets are created using path `m/44'/1'/0'` for PKH, `m/84'/1'/0'` for WPKH, `m/49'/1'/0'` for P2WPKH-nested-in-P2SH and `m/86'/1'/0'` for P2TR based accounts. Each of them uses the chain 0 for external addresses and chain 1 for internal ones, as shown in the example below. + +``` +wpkh([1004658e/84'/1'/0']tpubDCBEcmVKbfC9KfdydyLbJ2gfNL88grZu1XcWSW9ytTM6fitvaRmVyr8Ddf7SjZ2ZfMx9RicjYAXhuh3fmLiVLPodPEqnQQURUfrBKiiVZc8/0/*)#g8l47ngv + +wpkh([1004658e/84'/1'/0']tpubDCBEcmVKbfC9KfdydyLbJ2gfNL88grZu1XcWSW9ytTM6fitvaRmVyr8Ddf7SjZ2ZfMx9RicjYAXhuh3fmLiVLPodPEqnQQURUfrBKiiVZc8/1/*)#en65rxc5 +``` + +The suffix (after #) is the checksum. Descriptors can optionally be suffixed with a checksum to protect against typos or copy-paste errors. +All RPCs in Bitcoin Core will include the checksum in their output. + +```bash +declare -A xpubs + +for ((n=1;n<=3;n++)) +do + xpubs["internal_xpub_${n}"]=$(./src/bitcoin-cli -signet -rpcwallet="participant_${n}" listdescriptors | jq '.descriptors | [.[] | select(.desc | startswith("wpkh") and contains("/1/*"))][0] | .desc' | grep -Po '(?<=\().*(?=\))') + + xpubs["external_xpub_${n}"]=$(./src/bitcoin-cli -signet -rpcwallet="participant_${n}" listdescriptors | jq '.descriptors | [.[] | select(.desc | startswith("wpkh") and contains("/0/*") )][0] | .desc' | grep -Po '(?<=\().*(?=\))') +done +``` + +`jq` is used to extract the xpub from the `wpkh` descriptor. + +The following command can be used to verify if the xpub was generated correctly. + +```bash +for x in "${!xpubs[@]}"; do printf "[%s]=%s\n" "$x" "${xpubs[$x]}" ; done +``` + +As previously mentioned, this step extracts the `m/84'/1'/0'` account instead of the path defined in [BIP 45](https://github.com/bitcoin/bips/blob/master/bip-0045.mediawiki) or [BIP 87](https://github.com/bitcoin/bips/blob/master/bip-0087.mediawiki), since there is no way to extract a specific path in Bitcoin Core at the time of writing. + +### 1.2 Define the Multisig Descriptors + +Define the external and internal multisig descriptors, add the checksum and then, join both in a JSON array. + +```bash +external_desc="wsh(sortedmulti(2,${xpubs["external_xpub_1"]},${xpubs["external_xpub_2"]},${xpubs["external_xpub_3"]}))" +internal_desc="wsh(sortedmulti(2,${xpubs["internal_xpub_1"]},${xpubs["internal_xpub_2"]},${xpubs["internal_xpub_3"]}))" + +external_desc_sum=$(./src/bitcoin-cli -signet getdescriptorinfo $external_desc | jq '.descriptor') +internal_desc_sum=$(./src/bitcoin-cli -signet getdescriptorinfo $internal_desc | jq '.descriptor') + +multisig_ext_desc="{\"desc\": $external_desc_sum, \"active\": true, \"internal\": false, \"timestamp\": \"now\"}" +multisig_int_desc="{\"desc\": $internal_desc_sum, \"active\": true, \"internal\": true, \"timestamp\": \"now\"}" + +multisig_desc="[$multisig_ext_desc, $multisig_int_desc]" +``` + +`external_desc` and `internal_desc` specify the output type (`wsh`, in this case) and the xpubs involved. They also use BIP 67 (`sortedmulti`), so the wallet can be recreated without worrying about the order of xpubs. Conceptually, descriptors describe a list of scriptPubKey (along with information for spending from it) [[source](https://github.com/bitcoin/bitcoin/issues/21199#issuecomment-780772418)]. + +Note that at least two descriptors are usually used, one for internal derivation paths and external ones. There are discussions about eliminating this redundancy, as can been seen in the issue [#17190](https://github.com/bitcoin/bitcoin/issues/17190). + +After creating the descriptors, it is necessary to add the checksum, which is required by the `importdescriptors` RPC. + +The checksum for a descriptor without one can be computed using the `getdescriptorinfo` RPC. The response has the `descriptor` field, which is the descriptor with the checksum added. + +There are other fields that can be added to the descriptors: + +* `active`: Sets the descriptor to be the active one for the corresponding output type (`wsh`, in this case). +* `internal`: Indicates whether matching outputs should be treated as something other than incoming payments (e.g. change). +* `timestamp`: Sets the time from which to start rescanning the blockchain for the descriptor, in UNIX epoch time. + +Documentation for these and other parameters can be found by typing `./src/bitcoin-cli help importdescriptors`. + +`multisig_desc` concatenates external and internal descriptors in a JSON array and then it will be used to create the multisig wallet. + +### 1.3 Create the Multisig Wallet + +To create the multisig wallet, first create an empty one (no keys, HD seed and private keys disabled). + +Then import the descriptors created in the previous step using the `importdescriptors` RPC. + +After that, `getwalletinfo` can be used to check if the wallet was created successfully. + +```bash +./src/bitcoin-cli -signet -named createwallet wallet_name="multisig_wallet_01" disable_private_keys=true blank=true descriptors=true + +./src/bitcoin-cli -signet -rpcwallet="multisig_wallet_01" importdescriptors "$multisig_desc" + +./src/bitcoin-cli -signet -rpcwallet="multisig_wallet_01" getwalletinfo +``` + +Once the wallets have already been created and this tutorial needs to be repeated or resumed, it is not necessary to recreate them, just load them with the command below: + +```bash +for ((n=1;n<=3;n++)); do ./src/bitcoin-cli -signet loadwallet "participant_${n}"; done +``` + +### 1.4 Fund the wallet + +The wallet can receive signet coins by generating a new address and passing it as parameters to `getcoins.py` script. + +This script will print a captcha in dot-matrix to the terminal, using unicode Braille characters. After solving the captcha, the coins will be sent directly to the address or wallet (according to the parameters). + +The url used by the script can also be accessed directly. At time of writing, the url is [`https://signetfaucet.com`](https://signetfaucet.com). + +Coins received by the wallet must have at least 1 confirmation before they can be spent. It is necessary to wait for a new block to be mined before continuing. + +```bash +receiving_address=$(./src/bitcoin-cli -signet -rpcwallet="multisig_wallet_01" getnewaddress) + +./contrib/signet/getcoins.py -c ./src/bitcoin-cli -a $receiving_address +``` + +To copy the receiving address onto the clipboard, use the following command. This can be useful when getting coins via the signet faucet mentioned above. + +```bash +echo -n "$receiving_address" | xclip -sel clip +``` + +The `getbalances` RPC may be used to check the balance. Coins with `trusted` status can be spent. + +```bash +./src/bitcoin-cli -signet -rpcwallet="multisig_wallet_01" getbalances +``` + +### 1.5 Create a PSBT + +Unlike singlesig wallets, multisig wallets cannot create and sign transactions directly because they require the signatures of the co-signers. Instead they create a Partially Signed Bitcoin Transaction (PSBT). + +PSBT is a data format that allows wallets and other tools to exchange information about a Bitcoin transaction and the signatures necessary to complete it. [[source](https://bitcoinops.org/en/topics/psbt/)] + +The current PSBT version (v0) is defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki). + +For simplicity, the destination address is taken from the `participant_1` wallet in the code above, but it can be any valid bitcoin address. + +The `walletcreatefundedpsbt` RPC is used to create and fund a transaction in the PSBT format. It is the first step in creating the PSBT. + +```bash +balance=$(./src/bitcoin-cli -signet -rpcwallet="multisig_wallet_01" getbalance) + +amount=$(echo "$balance * 0.8" | bc -l | sed -e 's/^\./0./' -e 's/^-\./-0./') + +destination_addr=$(./src/bitcoin-cli -signet -rpcwallet="participant_1" getnewaddress) + +funded_psbt=$(./src/bitcoin-cli -signet -named -rpcwallet="multisig_wallet_01" walletcreatefundedpsbt outputs="{\"$destination_addr\": $amount}" | jq -r '.psbt') +``` + +There is also the `createpsbt` RPC, which serves the same purpose, but it has no access to the wallet or to the UTXO set. It is functionally the same as `createrawtransaction` and just drops the raw transaction into an otherwise blank PSBT. [[source](https://bitcointalk.org/index.php?topic=5131043.msg50573609#msg50573609)] In most cases, `walletcreatefundedpsbt` solves the problem. + +The `send` RPC can also return a PSBT if more signatures are needed to sign the transaction. + +### 1.6 Decode or Analyze the PSBT + +Optionally, the PSBT can be decoded to a JSON format using `decodepsbt` RPC. + +The `analyzepsbt` RPC analyzes and provides information about the current status of a PSBT and its inputs, e.g. missing signatures. + +```bash +./src/bitcoin-cli -signet decodepsbt $funded_psbt + +./src/bitcoin-cli -signet analyzepsbt $funded_psbt +``` + +### 1.7 Update the PSBT + +In the code above, two PSBTs are created. One signed by `participant_1` wallet and other, by the `participant_2` wallet. + +The `walletprocesspsbt` is used by the wallet to sign a PSBT. + +```bash +psbt_1=$(./src/bitcoin-cli -signet -rpcwallet="participant_1" walletprocesspsbt $funded_psbt | jq '.psbt') + +psbt_2=$(./src/bitcoin-cli -signet -rpcwallet="participant_2" walletprocesspsbt $funded_psbt | jq '.psbt') +``` + +### 1.8 Combine the PSBT + +The PSBT, if signed separately by the co-signers, must be combined into one transaction before being finalized. This is done by `combinepsbt` RPC. + +```bash +combined_psbt=$(./src/bitcoin-cli -signet combinepsbt "[$psbt_1, $psbt_2]") +``` + +There is an RPC called `joinpsbts`, but it has a different purpose than `combinepsbt`. `joinpsbts` joins the inputs from multiple distinct PSBTs into one PSBT. + +In the example above, the PSBTs are the same, but signed by different participants. If the user tries to merge them using `joinpsbts`, the error `Input txid:pos exists in multiple PSBTs` is returned. To be able to merge different PSBTs into one, they must have different inputs and outputs. + +### 1.9 Finalize and Broadcast the PSBT + +The `finalizepsbt` RPC is used to produce a network serialized transaction which can be broadcast with `sendrawtransaction`. + +It checks that all inputs have complete scriptSigs and scriptWitnesses and, if so, encodes them into network serialized transactions. + +```bash +finalized_psbt_hex=$(./src/bitcoin-cli -signet finalizepsbt $combined_psbt | jq -r '.hex') + +./src/bitcoin-cli -signet sendrawtransaction $finalized_psbt_hex +``` + +### 1.10 Alternative Workflow (PSBT sequential signatures) + +Instead of each wallet signing the original PSBT and combining them later, the wallets can also sign the PSBTs sequentially. This is less scalable than the previously presented parallel workflow, but it works. + +After that, the rest of the process is the same: the PSBT is finalized and transmitted to the network. + +```bash +psbt_1=$(./src/bitcoin-cli -signet -rpcwallet="participant_1" walletprocesspsbt $funded_psbt | jq -r '.psbt') + +psbt_2=$(./src/bitcoin-cli -signet -rpcwallet="participant_2" walletprocesspsbt $psbt_1 | jq -r '.psbt') + +finalized_psbt_hex=$(./src/bitcoin-cli -signet finalizepsbt $psbt_2 | jq -r '.hex') + +./src/bitcoin-cli -signet sendrawtransaction $finalized_psbt_hex +```
\ No newline at end of file diff --git a/doc/zmq.md b/doc/zmq.md index 0521fe08d8..b832e71734 100644 --- a/doc/zmq.md +++ b/doc/zmq.md @@ -91,9 +91,11 @@ For instance: Each PUB notification has a topic and body, where the header corresponds to the notification type. For instance, for the notification `-zmqpubhashtx` the topic is `hashtx` (no null -terminator) and the body is the transaction hash (32 -bytes) for all but `sequence` topic. For `sequence`, the body -is structured as the following based on the type of message: +terminator). These options can also be provided in bitcoin.conf. + +The topics are: + +`sequence`: the body is structured as the following based on the type of message: <32-byte hash>C : Blockhash connected <32-byte hash>D : Blockhash disconnected @@ -102,7 +104,24 @@ is structured as the following based on the type of message: Where the 8-byte uints correspond to the mempool sequence number. -These options can also be provided in bitcoin.conf. +`rawtx`: Notifies about all transactions, both when they are added to mempool or when a new block arrives. This means a transaction could be published multiple times. First, when it enters the mempool and then again in each block that includes it. The messages are ZMQ multipart messages with three parts. The first part is the topic (`rawtx`), the second part is the serialized transaction, and the last part is a sequence number (representing the message count to detect lost messages). + + | rawtx | <serialized transaction> | <uint32 sequence number in Little Endian> + +`hashtx`: Notifies about all transactions, both when they are added to mempool or when a new block arrives. This means a transaction could be published multiple times. First, when it enters the mempool and then again in each block that includes it. The messages are ZMQ multipart messages with three parts. The first part is the topic (`hashtx`), the second part is the 32-byte transaction hash, and the last part is a sequence number (representing the message count to detect lost messages). + + | hashtx | <32-byte transaction hash in Little Endian> | <uint32 sequence number in Little Endian> + + +`rawblock`: Notifies when the chain tip is updated. Messages are ZMQ multipart messages with three parts. The first part is the topic (`rawblock`), the second part is the serialized block, and the last part is a sequence number (representing the message count to detect lost messages). + + | rawblock | <serialized block> | <uint32 sequence number in Little Endian> + +`hashblock`: Notifies when the chain tip is updated. Messages are ZMQ multipart messages with three parts. The first part is the topic (`hashblock`), the second part is the 32-byte block hash, and the last part is a sequence number (representing the message count to detect lost messages). + + | hashblock | <32-byte block hash in Little Endian> | <uint32 sequence number in Little Endian> + +**_NOTE:_** Note that the 32-byte hashes are in Little Endian and not in the Big Endian format that the RPC interface and block explorers use to display transaction and block hashes. ZeroMQ endpoint specifiers for TCP (and others) are documented in the [ZeroMQ API](http://api.zeromq.org/4-0:_start). diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index 35d5b0004a..6c0e6f7c6f 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -394,8 +394,7 @@ bitcoin_qt_apk: FORCE cp $(dir $(lastword $(CC)))../sysroot/usr/lib/$(host_alias)/libc++_shared.so $(APK_LIB_DIR) tar xf $(QT_BASE_PATH) -C qt/android/src/ $(QT_BASE_TLD)src/android/jar/src --strip-components=5 tar xf $(QT_BASE_PATH) -C qt/android/src/ $(QT_BASE_TLD)src/android/java/src --strip-components=5 - tar xf $(QT_BASE_PATH) -C qt/android/res/ $(QT_BASE_TLD)src/android/java/res --strip-components=5 - cp qt/bitcoin-qt $(APK_LIB_DIR)/libbitcoin-qt.so + cp qt/bitcoin-qt $(APK_LIB_DIR)/libbitcoin-qt_$(ANDROID_ARCH).so cd qt/android && gradle wrapper --gradle-version=6.6.1 cd qt/android && ./gradlew build diff --git a/src/Makefile.qttest.include b/src/Makefile.qttest.include index 797e1f9a97..a0900f2691 100644 --- a/src/Makefile.qttest.include +++ b/src/Makefile.qttest.include @@ -52,7 +52,7 @@ if ENABLE_ZMQ qt_test_test_bitcoin_qt_LDADD += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS) endif qt_test_test_bitcoin_qt_LDADD += $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_UTIL) $(LIBBITCOIN_CONSENSUS) $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) $(LIBLEVELDB) \ - $(LIBLEVELDB_SSE42) $(LIBMEMENV) $(BOOST_LIBS) $(QT_DBUS_LIBS) $(QT_TEST_LIBS) $(QT_LIBS) \ + $(LIBLEVELDB_SSE42) $(LIBMEMENV) $(BOOST_LIBS) $(QT_LIBS) $(QT_DBUS_LIBS) $(QT_TEST_LIBS) \ $(QR_LIBS) $(BDB_LIBS) $(MINIUPNPC_LIBS) $(NATPMP_LIBS) $(LIBSECP256K1) \ $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(SQLITE_LIBS) qt_test_test_bitcoin_qt_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(QT_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 06d195aaaf..326409c078 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -69,6 +69,7 @@ BITCOIN_TESTS =\ test/allocator_tests.cpp \ test/amount_tests.cpp \ test/arith_uint256_tests.cpp \ + test/banman_tests.cpp \ test/base32_tests.cpp \ test/base58_tests.cpp \ test/base64_tests.cpp \ diff --git a/src/addrman.cpp b/src/addrman.cpp index 8a0433c40d..4d785043b8 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -614,7 +614,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ return fInsert; } -void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nTime) +bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nTime) { AssertLockHeld(cs); @@ -625,8 +625,7 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT AddrInfo* pinfo = Find(addr, &nId); // if not found, bail out - if (!pinfo) - return; + if (!pinfo) return false; AddrInfo& info = *pinfo; @@ -638,13 +637,11 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT // currently-connected peers. // if it is already in the tried set, don't do anything else - if (info.fInTried) - return; + if (info.fInTried) return false; // if it is not in new, something bad happened - if (!Assume(info.nRefCount > 0)) { - return; - } + if (!Assume(info.nRefCount > 0)) return false; + // which tried bucket to move the entry to int tried_bucket = info.GetTriedBucket(nKey, m_asmap); @@ -661,11 +658,13 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT colliding_entry != mapInfo.end() ? colliding_entry->second.ToString() : "", addr.ToString(), m_tried_collisions.size()); + return false; } else { // move nId to the tried tables MakeTried(info, nId); LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n", addr.ToString(), addr.GetMappedAS(m_asmap), tried_bucket, tried_bucket_pos); + return true; } } @@ -1049,12 +1048,13 @@ bool AddrManImpl::Add(const std::vector<CAddress>& vAddr, const CNetAddr& source return ret; } -void AddrManImpl::Good(const CService& addr, int64_t nTime) +bool AddrManImpl::Good(const CService& addr, int64_t nTime) { LOCK(cs); Check(); - Good_(addr, /* test_before_evict */ true, nTime); + auto ret = Good_(addr, /*test_before_evict=*/true, nTime); Check(); + return ret; } void AddrManImpl::Attempt(const CService& addr, bool fCountFailure, int64_t nTime) @@ -1157,9 +1157,9 @@ bool AddrMan::Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, in return m_impl->Add(vAddr, source, nTimePenalty); } -void AddrMan::Good(const CService& addr, int64_t nTime) +bool AddrMan::Good(const CService& addr, int64_t nTime) { - m_impl->Good(addr, nTime); + return m_impl->Good(addr, nTime); } void AddrMan::Attempt(const CService& addr, bool fCountFailure, int64_t nTime) diff --git a/src/addrman.h b/src/addrman.h index 455d84ca71..e9bc372380 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -81,8 +81,14 @@ public: * @return true if at least one address is successfully added. */ bool Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty = 0); - //! Mark an entry as accessible, possibly moving it from "new" to "tried". - void Good(const CService& addr, int64_t nTime = GetAdjustedTime()); + /** + * Mark an address record as accessible and attempt to move it to addrman's tried table. + * + * @param[in] addr Address record to attempt to move to tried table. + * @param[in] nTime The time that we were last connected to this peer. + * @return true if the address is successfully moved from the new table to the tried table. + */ + bool Good(const CService& addr, int64_t nTime = GetAdjustedTime()); //! Mark an entry as connection attempted to. void Attempt(const CService& addr, bool fCountFailure, int64_t nTime = GetAdjustedTime()); diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 10a65871c1..bd7caf473b 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -115,7 +115,7 @@ public: bool Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(!cs); - void Good(const CService& addr, int64_t nTime) + bool Good(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(!cs); void Attempt(const CService& addr, bool fCountFailure, int64_t nTime) @@ -248,7 +248,7 @@ private: * @see AddrMan::Add() for parameters. */ bool AddSingle(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); - void Good_(const CService& addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); + bool Good_(const CService& addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); bool Add_(const std::vector<CAddress> &vAddr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/crypto/muhash.h b/src/crypto/muhash.h index c023a8b9d3..0c050cd32b 100644 --- a/src/crypto/muhash.h +++ b/src/crypto/muhash.h @@ -24,7 +24,7 @@ private: public: static constexpr size_t BYTE_SIZE = 384; -#ifdef HAVE___INT128 +#ifdef __SIZEOF_INT128__ typedef unsigned __int128 double_limb_t; typedef uint64_t limb_t; static constexpr int LIMBS = 48; diff --git a/src/index/base.cpp b/src/index/base.cpp index fc6dd77a72..8525dcbfa0 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -91,11 +91,12 @@ bool BaseIndex::Init() const CBlockIndex* block = active_chain.Tip(); prune_violation = true; // check backwards from the tip if we have all block data until we reach the indexes bestblock - while (block_to_test && block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) { + while (block_to_test && block && (block->nStatus & BLOCK_HAVE_DATA)) { if (block_to_test == block) { prune_violation = false; break; } + assert(block->pprev); block = block->pprev; } } diff --git a/src/init/common.cpp b/src/init/common.cpp index 8f9e0ebc87..38c60366e3 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -64,7 +64,7 @@ void AddLoggingArgs(ArgsManager& argsman) argsman.AddArg("-debug=<category>", "Output debugging information (default: -nodebug, supplying <category> is optional). " "If <category> is not supplied or if <category> = 1, output all debugging information. <category> can be: " + LogInstance().LogCategoriesString() + ". This option can be specified multiple times to output multiple categories.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-debugexclude=<category>", strprintf("Exclude debugging information for a category. Can be used in conjunction with -debug=1 to output debug logs for all categories except the specified category. This option can be specified multiple times to exclude multiple categories."), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-debugexclude=<category>", "Exclude debugging information for a category. Can be used in conjunction with -debug=1 to output debug logs for all categories except the specified category. This option can be specified multiple times to exclude multiple categories.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logips", strprintf("Include IP addresses in debug output (default: %u)", DEFAULT_LOGIPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logtimestamps", strprintf("Prepend debug output with timestamp (default: %u)", DEFAULT_LOGTIMESTAMPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); #ifdef HAVE_THREAD_LOCAL diff --git a/src/net.cpp b/src/net.cpp index cbf2ff3fa2..0f49b8ad5a 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1225,7 +1225,6 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ switch (conn_type) { case ConnectionType::INBOUND: case ConnectionType::MANUAL: - case ConnectionType::FEELER: return false; case ConnectionType::OUTBOUND_FULL_RELAY: max_connections = m_max_outbound_full_relay; @@ -1236,6 +1235,9 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ // no limit for ADDR_FETCH because -seednode has no limit either case ConnectionType::ADDR_FETCH: break; + // no limit for FEELER connections since they're short-lived + case ConnectionType::FEELER: + break; } // no default case, so the compiler can warn about missing cases // Count existing connections @@ -884,8 +884,8 @@ public: * Attempts to open a connection. Currently only used from tests. * * @param[in] address Address of node to try connecting to - * @param[in] conn_type ConnectionType::OUTBOUND or ConnectionType::BLOCK_RELAY - * or ConnectionType::ADDR_FETCH + * @param[in] conn_type ConnectionType::OUTBOUND, ConnectionType::BLOCK_RELAY, + * ConnectionType::ADDR_FETCH or ConnectionType::FEELER * @return bool Returns false if there are no available * slots for this connection: * - conn_type not a supported ConnectionType diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b5d56fc4ff..2a30afdb5b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1104,7 +1104,7 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, int64_t nTime) CService addr_you = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ? addr : CService(); uint64_t your_services{addr.nServices}; - const bool tx_relay = !m_ignore_incoming_txs && pnode.m_tx_relay != nullptr; + const bool tx_relay = !m_ignore_incoming_txs && pnode.m_tx_relay != nullptr && !pnode.IsFeelerConn(); m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, my_services, nTime, your_services, addr_you, // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime) my_services, CService(), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime) diff --git a/src/net_types.cpp b/src/net_types.cpp index c8f57fe6c6..e4101a9876 100644 --- a/src/net_types.cpp +++ b/src/net_types.cpp @@ -4,12 +4,16 @@ #include <net_types.h> +#include <logging.h> #include <netaddress.h> #include <netbase.h> #include <univalue.h> +static const char* BANMAN_JSON_VERSION_KEY{"version"}; + CBanEntry::CBanEntry(const UniValue& json) - : nVersion(json["version"].get_int()), nCreateTime(json["ban_created"].get_int64()), + : nVersion(json[BANMAN_JSON_VERSION_KEY].get_int()), + nCreateTime(json["ban_created"].get_int64()), nBanUntil(json["banned_until"].get_int64()) { } @@ -17,7 +21,7 @@ CBanEntry::CBanEntry(const UniValue& json) UniValue CBanEntry::ToJson() const { UniValue json(UniValue::VOBJ); - json.pushKV("version", nVersion); + json.pushKV(BANMAN_JSON_VERSION_KEY, nVersion); json.pushKV("ban_created", nCreateTime); json.pushKV("banned_until", nBanUntil); return json; @@ -54,11 +58,16 @@ UniValue BanMapToJson(const banmap_t& bans) void BanMapFromJson(const UniValue& bans_json, banmap_t& bans) { for (const auto& ban_entry_json : bans_json.getValues()) { + const int version{ban_entry_json[BANMAN_JSON_VERSION_KEY].get_int()}; + if (version != CBanEntry::CURRENT_VERSION) { + LogPrintf("Dropping entry with unknown version (%s) from ban list\n", version); + continue; + } CSubNet subnet; const auto& subnet_str = ban_entry_json[BANMAN_JSON_ADDR_KEY].get_str(); if (!LookupSubNet(subnet_str, subnet)) { - throw std::runtime_error( - strprintf("Cannot parse banned address or subnet: %s", subnet_str)); + LogPrintf("Dropping entry with unparseable address or subnet (%s) from ban list\n", subnet_str); + continue; } bans.insert_or_assign(subnet, CBanEntry{ban_entry_json}); } diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index 9ad65d15d2..0be2bdb3ef 100644 --- a/src/node/psbt.cpp +++ b/src/node/psbt.cpp @@ -105,7 +105,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) } ); if (!MoneyRange(out_amt)) { - result.SetInvalid(strprintf("PSBT is not valid. Output amount invalid")); + result.SetInvalid("PSBT is not valid. Output amount invalid"); return result; } diff --git a/src/qt/android/.gitignore b/src/qt/android/.gitignore index 74cf42f934..c090a2e98e 100644 --- a/src/qt/android/.gitignore +++ b/src/qt/android/.gitignore @@ -1,9 +1,7 @@ /.gradle /build -/gradle/wrapper +/gradle /gradlew* /libs -/res/layout -/res/values* /src/org/kde /src/org/qtproject diff --git a/src/qt/android/AndroidManifest.xml b/src/qt/android/AndroidManifest.xml index abb88fe89d..41615294e0 100644 --- a/src/qt/android/AndroidManifest.xml +++ b/src/qt/android/AndroidManifest.xml @@ -32,7 +32,8 @@ <meta-data android:name="android.app.background_running" android:value="true"/> <meta-data android:name="android.app.auto_screen_scale_factor" android:value="true"/> <meta-data android:name="android.app.extract_android_style" android:value="default"/> - </activity> + <meta-data android:name="android.app.load_local_libs_resource_id" android:resource="@array/load_local_libs"/> + </activity> </application> </manifest> diff --git a/src/qt/android/res/values/libs.xml b/src/qt/android/res/values/libs.xml new file mode 100644 index 0000000000..0f20df4eb0 --- /dev/null +++ b/src/qt/android/res/values/libs.xml @@ -0,0 +1,17 @@ +<?xml version="1.0" encoding="utf-8"?> +<resources> + <array name="load_local_libs"> + <item> + arm64-v8a;libbitcoin-qt_arm64-v8a.so + </item> + <item> + armeabi-v7a;libbitcoin-qt_armeabi-v7a.so + </item> + <item> + x86_64;libbitcoin-qt_x86_64.so + </item> + <item> + x86;libbitcoin-qt_x86.so + </item> + </array> +</resources> diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 922aac531f..d22111ce44 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -65,6 +65,8 @@ Q_IMPORT_PLUGIN(QWindowsVistaStylePlugin); #elif defined(QT_QPA_PLATFORM_COCOA) Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin); Q_IMPORT_PLUGIN(QMacStylePlugin); +#elif defined(QT_QPA_PLATFORM_ANDROID) +Q_IMPORT_PLUGIN(QAndroidPlatformIntegrationPlugin) #endif #endif diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index e7a3d724bb..f6ab26d1d2 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -34,6 +34,8 @@ Q_IMPORT_PLUGIN(QXcbIntegrationPlugin); Q_IMPORT_PLUGIN(QWindowsIntegrationPlugin); #elif defined(QT_QPA_PLATFORM_COCOA) Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin); +#elif defined(QT_QPA_PLATFORM_ANDROID) +Q_IMPORT_PLUGIN(QAndroidPlatformIntegrationPlugin) #endif #endif diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 7b37635db0..050d9dd980 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1134,7 +1134,7 @@ CoinStatsHashType ParseHashType(const std::string& hash_type_input) } else if (hash_type_input == "none") { return CoinStatsHashType::NONE; } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s is not a valid hash_type", hash_type_input)); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("'%s' is not a valid hash_type", hash_type_input)); } } @@ -2213,7 +2213,7 @@ static RPCHelpMan getblockstats() for (const std::string& stat : stats) { const UniValue& value = ret_all[stat]; if (value.isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid selected statistic %s", stat)); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid selected statistic '%s'", stat)); } ret.pushKV(stat, value); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 66e0df906c..6e67832b31 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -184,7 +184,7 @@ static bool getScriptFromDescriptor(const std::string& descriptor, CScript& scri FlatSigningProvider provider; std::vector<CScript> scripts; if (!desc->Expand(0, key_provider, scripts, provider)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys")); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot derive script without private keys"); } // Combo descriptors can have 2 or 4 scripts, so we can't just check scripts.size() == 1 diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 2761c69167..7705051df9 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -286,13 +286,13 @@ static RPCHelpMan deriveaddresses() FlatSigningProvider provider; std::vector<CScript> scripts; if (!desc->Expand(i, key_provider, scripts, provider)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys")); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot derive script without private keys"); } for (const CScript &script : scripts) { CTxDestination dest; if (!ExtractDestination(script, dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Descriptor does not have a corresponding address")); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Descriptor does not have a corresponding address"); } addresses.push_back(EncodeDestination(dest)); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 8fddb2829b..557b1296a6 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -328,7 +328,7 @@ static RPCHelpMan addconnection() "\nOpen an outbound connection to a specified node. This RPC is for testing only.\n", { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address and port to attempt connecting to."}, - {"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\" or \"addr-fetch\")."}, + {"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\", \"addr-fetch\" or \"feeler\")."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -356,6 +356,8 @@ static RPCHelpMan addconnection() conn_type = ConnectionType::BLOCK_RELAY; } else if (conn_type_in == "addr-fetch") { conn_type = ConnectionType::ADDR_FETCH; + } else if (conn_type_in == "feeler") { + conn_type = ConnectionType::FEELER; } else { throw JSONRPCError(RPC_INVALID_PARAMETER, self.ToString()); } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 4a5cd0a4be..57e3da0351 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -210,7 +210,7 @@ CPubKey AddrToPubKey(const FillableSigningProvider& keystore, const std::string& } CKeyID key = GetKeyForDestination(keystore, dest); if (key.IsNull()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("%s does not refer to a key", addr_in)); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' does not refer to a key", addr_in)); } CPubKey vchPubKey; if (!keystore.GetPubKey(key, vchPubKey)) { diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 31f30d0379..b700c3ae22 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -268,32 +268,32 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions) BOOST_AUTO_TEST_CASE(addrman_tried_collisions) { - AddrManTest addrman; + auto addrman = std::make_unique<AddrMan>(std::vector<bool>(), /*deterministic=*/true, /*consistency_check_ratio=*/100); CNetAddr source = ResolveIP("252.2.2.2"); uint32_t num_addrs{0}; - BOOST_CHECK_EQUAL(addrman.size(), num_addrs); + BOOST_CHECK_EQUAL(addrman->size(), num_addrs); - while (num_addrs < 64) { // Magic number! 250.1.1.1 - 250.1.1.64 do not collide with deterministic key = 1 + while (num_addrs < 35) { // Magic number! 250.1.1.1 - 250.1.1.35 do not collide in tried with deterministic key = 1 CService addr = ResolveService("250.1.1." + ToString(++num_addrs)); - BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); - addrman.Good(CAddress(addr, NODE_NONE)); + BOOST_CHECK(addrman->Add({CAddress(addr, NODE_NONE)}, source)); + + // Test: Add to tried without collision + BOOST_CHECK(addrman->Good(CAddress(addr, NODE_NONE))); - // Test: No collision in tried table yet. - BOOST_CHECK_EQUAL(addrman.size(), num_addrs); } - // Test: tried table collision! + // Test: Unable to add to tried table due to collision! CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs)); - uint32_t collisions{1}; - BOOST_CHECK(!addrman.Add({CAddress(addr1, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); + BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source)); + BOOST_CHECK(!addrman->Good(CAddress(addr1, NODE_NONE))); + // Test: Add the next address to tried without collision CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs)); - BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); + BOOST_CHECK(addrman->Add({CAddress(addr2, NODE_NONE)}, source)); + BOOST_CHECK(addrman->Good(CAddress(addr2, NODE_NONE))); } BOOST_AUTO_TEST_CASE(addrman_find) diff --git a/src/test/banman_tests.cpp b/src/test/banman_tests.cpp new file mode 100644 index 0000000000..27ce9ad638 --- /dev/null +++ b/src/test/banman_tests.cpp @@ -0,0 +1,43 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <banman.h> +#include <chainparams.h> +#include <netbase.h> +#include <streams.h> +#include <test/util/logging.h> +#include <test/util/setup_common.h> +#include <util/readwritefile.h> + + +#include <boost/test/unit_test.hpp> + +BOOST_FIXTURE_TEST_SUITE(banman_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(file) +{ + SetMockTime(777s); + const fs::path banlist_path{m_args.GetDataDirBase() / "banlist_test"}; + { + const std::string entries_write{ + "{ \"banned_nets\": [" + " { \"version\": 1, \"ban_created\": 0, \"banned_until\": 778, \"address\": \"aaaaaaaaa\" }," + " { \"version\": 2, \"ban_created\": 0, \"banned_until\": 778, \"address\": \"bbbbbbbbb\" }," + " { \"version\": 1, \"ban_created\": 0, \"banned_until\": 778, \"address\": \"1.0.0.0/8\" }" + "] }", + }; + assert(WriteBinaryFile(banlist_path + ".json", entries_write)); + { + // The invalid entries will be dropped, but the valid one remains + ASSERT_DEBUG_LOG("Dropping entry with unparseable address or subnet (aaaaaaaaa) from ban list"); + ASSERT_DEBUG_LOG("Dropping entry with unknown version (2) from ban list"); + BanMan banman{banlist_path, /*client_interface=*/nullptr, /*default_ban_time=*/0}; + banmap_t entries_read; + banman.GetBanned(entries_read); + assert(entries_read.size() == 1); + } + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 94a71859e9..1763cd8af3 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -79,11 +79,9 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE } CNode& p2p_node = *ConsumeNodeAsUniquePtr(fuzzed_data_provider).release(); - const bool successfully_connected{fuzzed_data_provider.ConsumeBool()}; - p2p_node.fSuccessfullyConnected = successfully_connected; connman.AddTestNode(p2p_node); g_setup->m_node.peerman->InitializeNode(&p2p_node); - FillNode(fuzzed_data_provider, p2p_node, /*init_version=*/successfully_connected); + FillNode(fuzzed_data_provider, connman, *g_setup->m_node.peerman, p2p_node); const auto mock_time = ConsumeTime(fuzzed_data_provider); SetMockTime(mock_time); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 21a959315e..e1c11e1afd 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -46,11 +46,8 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages) peers.push_back(ConsumeNodeAsUniquePtr(fuzzed_data_provider, i).release()); CNode& p2p_node = *peers.back(); - const bool successfully_connected{fuzzed_data_provider.ConsumeBool()}; - p2p_node.fSuccessfullyConnected = successfully_connected; - p2p_node.fPauseSend = false; g_setup->m_node.peerman->InitializeNode(&p2p_node); - FillNode(fuzzed_data_provider, p2p_node, /*init_version=*/successfully_connected); + FillNode(fuzzed_data_provider, connman, *g_setup->m_node.peerman, p2p_node); connman.AddTestNode(p2p_node); } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index ae5f7a379e..843b29b911 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -3,6 +3,8 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <consensus/amount.h> +#include <net_processing.h> +#include <netmessagemaker.h> #include <pubkey.h> #include <test/fuzz/util.h> #include <test/util/script.h> @@ -200,22 +202,57 @@ bool FuzzedSock::IsConnected(std::string& errmsg) const return false; } -void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, PeerManager& peerman, CNode& node) noexcept { + const bool successfully_connected{fuzzed_data_provider.ConsumeBool()}; const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); const int32_t version = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()); const bool filter_txs = fuzzed_data_provider.ConsumeBool(); - node.nServices = remote_services; - node.m_permissionFlags = permission_flags; - if (init_version) { - node.nVersion = version; - node.SetCommonVersion(std::min(version, PROTOCOL_VERSION)); + const CNetMsgMaker mm{0}; + + CSerializedNetMsg msg_version{ + mm.Make(NetMsgType::VERSION, + version, // + Using<CustomUintFormatter<8>>(remote_services), // + int64_t{}, // dummy time + int64_t{}, // ignored service bits + CService{}, // dummy + int64_t{}, // ignored service bits + CService{}, // ignored + uint64_t{1}, // dummy nonce + std::string{}, // dummy subver + int32_t{}, // dummy starting_height + filter_txs), + }; + + (void)connman.ReceiveMsgFrom(node, msg_version); + node.fPauseSend = false; + connman.ProcessMessagesOnce(node); + { + LOCK(node.cs_sendProcessing); + peerman.SendMessages(&node); } + if (node.fDisconnect) return; + assert(node.nVersion == version); + assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); + assert(node.nServices == remote_services); if (node.m_tx_relay != nullptr) { LOCK(node.m_tx_relay->cs_filter); - node.m_tx_relay->fRelayTxes = filter_txs; + assert(node.m_tx_relay->fRelayTxes == filter_txs); + } + node.m_permissionFlags = permission_flags; + if (successfully_connected) { + CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)}; + (void)connman.ReceiveMsgFrom(node, msg_verack); + node.fPauseSend = false; + connman.ProcessMessagesOnce(node); + { + LOCK(node.cs_sendProcessing); + peerman.SendMessages(&node); + } + assert(node.fSuccessfullyConnected == true); } } diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 40aaeac63f..7937315822 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -36,6 +36,8 @@ #include <string> #include <vector> +class PeerManager; + template <typename... Callables> size_t CallOneOf(FuzzedDataProvider& fuzzed_data_provider, Callables... callables) { @@ -257,7 +259,7 @@ inline CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcep template <bool ReturnUniquePtr = false> auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<NodeId>& node_id_in = std::nullopt) noexcept { - const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegral<NodeId>()); + const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegralInRange<NodeId>(0, std::numeric_limits<NodeId>::max())); const ServiceFlags local_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); const SOCKET socket = INVALID_SOCKET; const CAddress address = ConsumeAddress(fuzzed_data_provider); @@ -275,7 +277,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N } inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); } -void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept; +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, PeerManager& peerman, CNode& node) noexcept; class FuzzedFileProvider { diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index a1f70e7e70..e6bd903b92 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -232,6 +232,9 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) *chainman.ActiveChainstate().m_from_snapshot_blockhash, *chainman.SnapshotBlockhash()); + // Ensure that the genesis block was not marked assumed-valid. + BOOST_CHECK(!chainman.ActiveChain().Genesis()->IsAssumedValid()); + const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params()); const CBlockIndex* tip = chainman.ActiveTip(); @@ -309,4 +312,81 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) loaded_snapshot_blockhash); } +//! Test LoadBlockIndex behavior when multiple chainstates are in use. +//! +//! - First, verfiy that setBlockIndexCandidates is as expected when using a single, +//! fully-validating chainstate. +//! +//! - Then mark a region of the chain BLOCK_ASSUMED_VALID and introduce a second chainstate +//! that will tolerate assumed-valid blocks. Run LoadBlockIndex() and ensure that the first +//! chainstate only contains fully validated blocks and the other chainstate contains all blocks, +//! even those assumed-valid. +//! +BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) +{ + ChainstateManager& chainman = *Assert(m_node.chainman); + CTxMemPool& mempool = *m_node.mempool; + CChainState& cs1 = chainman.ActiveChainstate(); + + int num_indexes{0}; + int num_assumed_valid{0}; + const int expected_assumed_valid{20}; + const int last_assumed_valid_idx{40}; + const int assumed_valid_start_idx = last_assumed_valid_idx - expected_assumed_valid; + + CBlockIndex* validated_tip{nullptr}; + CBlockIndex* assumed_tip{chainman.ActiveChain().Tip()}; + + auto reload_all_block_indexes = [&]() { + for (CChainState* cs : chainman.GetAll()) { + LOCK(::cs_main); + cs->UnloadBlockIndex(); + BOOST_CHECK(cs->setBlockIndexCandidates.empty()); + } + + WITH_LOCK(::cs_main, chainman.LoadBlockIndex()); + }; + + // Ensure that without any assumed-valid BlockIndex entries, all entries are considered + // tip candidates. + reload_all_block_indexes(); + BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), cs1.m_chain.Height() + 1); + + // Mark some region of the chain assumed-valid. + for (int i = 0; i <= cs1.m_chain.Height(); ++i) { + auto index = cs1.m_chain[i]; + + if (i < last_assumed_valid_idx && i >= assumed_valid_start_idx) { + index->nStatus = BlockStatus::BLOCK_VALID_TREE | BlockStatus::BLOCK_ASSUMED_VALID; + } + + ++num_indexes; + if (index->IsAssumedValid()) ++num_assumed_valid; + + // Note the last fully-validated block as the expected validated tip. + if (i == (assumed_valid_start_idx - 1)) { + validated_tip = index; + BOOST_CHECK(!index->IsAssumedValid()); + } + } + + BOOST_CHECK_EQUAL(expected_assumed_valid, num_assumed_valid); + + CChainState& cs2 = WITH_LOCK(::cs_main, + return chainman.InitializeChainstate(&mempool, GetRandHash())); + + reload_all_block_indexes(); + + // The fully validated chain only has candidates up to the start of the assumed-valid + // blocks. + BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(validated_tip), 1); + BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(assumed_tip), 0); + BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), assumed_valid_start_idx); + + // The assumed-valid tolerant chain has all blocks as candidates. + BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.count(validated_tip), 1); + BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.count(assumed_tip), 1); + BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.size(), num_indexes); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 430f1963ea..f4768d5bb6 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -92,7 +92,7 @@ std::vector<unsigned char> ParseHex(const char* psz) signed char c = HexDigit(*psz++); if (c == (signed char)-1) break; - unsigned char n = (c << 4); + auto n{uint8_t(c << 4)}; c = HexDigit(*psz++); if (c == (signed char)-1) break; @@ -141,8 +141,7 @@ std::string EncodeBase64(Span<const unsigned char> input) std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid) { - static const int decode64_table[256] = - { + static const int8_t decode64_table[256]{ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 62, -1, -1, -1, 63, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -1, -1, @@ -164,7 +163,7 @@ std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid) while (*p != 0) { int x = decode64_table[(unsigned char)*p]; if (x == -1) break; - val.push_back(x); + val.push_back(uint8_t(x)); ++p; } @@ -220,8 +219,7 @@ std::string EncodeBase32(const std::string& str, bool pad) std::vector<unsigned char> DecodeBase32(const char* p, bool* pf_invalid) { - static const int decode32_table[256] = - { + static const int8_t decode32_table[256]{ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 26, 27, 28, 29, 30, 31, -1, -1, -1, -1, @@ -243,7 +241,7 @@ std::vector<unsigned char> DecodeBase32(const char* p, bool* pf_invalid) while (*p != 0) { int x = decode32_table[(unsigned char)*p]; if (x == -1) break; - val.push_back(x); + val.push_back(uint8_t(x)); ++p; } @@ -491,14 +489,14 @@ bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out) std::string ToLower(const std::string& str) { std::string r; - for (auto ch : str) r += ToLower((unsigned char)ch); + for (auto ch : str) r += ToLower(ch); return r; } std::string ToUpper(const std::string& str) { std::string r; - for (auto ch : str) r += ToUpper((unsigned char)ch); + for (auto ch : str) r += ToUpper(ch); return r; } diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 08a5465de1..5813e8d212 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -226,7 +226,7 @@ bool TimingResistantEqual(const T& a, const T& b) if (b.size() == 0) return a.size() == 0; size_t accumulator = a.size() ^ b.size(); for (size_t i = 0; i < a.size(); i++) - accumulator |= a[i] ^ b[i%b.size()]; + accumulator |= size_t(a[i] ^ b[i%b.size()]); return accumulator == 0; } diff --git a/src/validation.cpp b/src/validation.cpp index d4203de56a..1aac71fb0f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -55,6 +55,7 @@ #include <validationinterface.h> #include <warnings.h> +#include <algorithm> #include <numeric> #include <optional> #include <string> @@ -3694,7 +3695,7 @@ CBlockIndex * BlockManager::InsertBlockIndex(const uint256& hash) bool BlockManager::LoadBlockIndex( const Consensus::Params& consensus_params, - std::set<CBlockIndex*, CBlockIndexWorkComparator>& block_index_candidates) + ChainstateManager& chainman) { if (!m_block_tree_db->LoadBlockIndexGuts(consensus_params, [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); })) { return false; @@ -3709,17 +3710,41 @@ bool BlockManager::LoadBlockIndex( vSortedByHeight.push_back(std::make_pair(pindex->nHeight, pindex)); } sort(vSortedByHeight.begin(), vSortedByHeight.end()); + + // Find start of assumed-valid region. + int first_assumed_valid_height = std::numeric_limits<int>::max(); + + for (const auto& [height, block] : vSortedByHeight) { + if (block->IsAssumedValid()) { + auto chainstates = chainman.GetAll(); + + // If we encounter an assumed-valid block index entry, ensure that we have + // one chainstate that tolerates assumed-valid entries and another that does + // not (i.e. the background validation chainstate), since assumed-valid + // entries should always be pending validation by a fully-validated chainstate. + auto any_chain = [&](auto fnc) { return std::any_of(chainstates.cbegin(), chainstates.cend(), fnc); }; + assert(any_chain([](auto chainstate) { return chainstate->reliesOnAssumedValid(); })); + assert(any_chain([](auto chainstate) { return !chainstate->reliesOnAssumedValid(); })); + + first_assumed_valid_height = height; + break; + } + } + for (const std::pair<int, CBlockIndex*>& item : vSortedByHeight) { if (ShutdownRequested()) return false; CBlockIndex* pindex = item.second; pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + GetBlockProof(*pindex); pindex->nTimeMax = (pindex->pprev ? std::max(pindex->pprev->nTimeMax, pindex->nTime) : pindex->nTime); - // We can link the chain of blocks for which we've received transactions at some point. + + // We can link the chain of blocks for which we've received transactions at some point, or + // blocks that are assumed-valid on the basis of snapshot load (see + // PopulateAndValidateSnapshot()). // Pruned nodes may have deleted the block. if (pindex->nTx > 0) { if (pindex->pprev) { - if (pindex->pprev->HaveTxsDownloaded()) { + if (pindex->pprev->nChainTx > 0) { pindex->nChainTx = pindex->pprev->nChainTx + pindex->nTx; } else { pindex->nChainTx = 0; @@ -3736,7 +3761,36 @@ bool BlockManager::LoadBlockIndex( if (pindex->IsAssumedValid() || (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr))) { - block_index_candidates.insert(pindex); + + // Fill each chainstate's block candidate set. Only add assumed-valid + // blocks to the tip candidate set if the chainstate is allowed to rely on + // assumed-valid blocks. + // + // If all setBlockIndexCandidates contained the assumed-valid blocks, the + // background chainstate's ActivateBestChain() call would add assumed-valid + // blocks to the chain (based on how FindMostWorkChain() works). Obviously + // we don't want this since the purpose of the background validation chain + // is to validate assued-valid blocks. + // + // Note: This is considering all blocks whose height is greater or equal to + // the first assumed-valid block to be assumed-valid blocks, and excluding + // them from the background chainstate's setBlockIndexCandidates set. This + // does mean that some blocks which are not technically assumed-valid + // (later blocks on a fork beginning before the first assumed-valid block) + // might not get added to the the background chainstate, but this is ok, + // because they will still be attached to the active chainstate if they + // actually contain more work. + // + // Instad of this height-based approach, an earlier attempt was made at + // detecting "holistically" whether the block index under consideration + // relied on an assumed-valid ancestor, but this proved to be too slow to + // be practical. + for (CChainState* chainstate : chainman.GetAll()) { + if (chainstate->reliesOnAssumedValid() || + pindex->nHeight < first_assumed_valid_height) { + chainstate->setBlockIndexCandidates.insert(pindex); + } + } } if (pindex->nStatus & BLOCK_FAILED_MASK && (!pindexBestInvalid || pindex->nChainWork > pindexBestInvalid->nChainWork)) pindexBestInvalid = pindex; @@ -3760,11 +3814,9 @@ void BlockManager::Unload() { m_block_index.clear(); } -bool BlockManager::LoadBlockIndexDB(std::set<CBlockIndex*, CBlockIndexWorkComparator>& setBlockIndexCandidates) +bool BlockManager::LoadBlockIndexDB(ChainstateManager& chainman) { - if (!LoadBlockIndex( - ::Params().GetConsensus(), - setBlockIndexCandidates)) { + if (!LoadBlockIndex(::Params().GetConsensus(), chainman)) { return false; } @@ -4110,7 +4162,7 @@ bool ChainstateManager::LoadBlockIndex() // Load block index from databases bool needs_init = fReindex; if (!fReindex) { - bool ret = m_blockman.LoadBlockIndexDB(ActiveChainstate().setBlockIndexCandidates); + bool ret = m_blockman.LoadBlockIndexDB(*this); if (!ret) return false; needs_init = m_blockman.m_block_index.empty(); } @@ -4840,6 +4892,17 @@ bool ChainstateManager::ActivateSnapshot( return true; } +static void FlushSnapshotToDisk(CCoinsViewCache& coins_cache, bool snapshot_loaded) +{ + LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE( + strprintf("%s (%.2f MB)", + snapshot_loaded ? "saving snapshot chainstate" : "flushing coins cache", + coins_cache.DynamicMemoryUsage() / (1000 * 1000)), + BCLog::LogFlags::ALL); + + coins_cache.Flush(); +} + bool ChainstateManager::PopulateAndValidateSnapshot( CChainState& snapshot_chainstate, CAutoFile& coins_file, @@ -4877,7 +4940,6 @@ bool ChainstateManager::PopulateAndValidateSnapshot( uint64_t coins_left = metadata.m_coins_count; LogPrintf("[snapshot] loading coins from snapshot %s\n", base_blockhash.ToString()); - int64_t flush_now{0}; int64_t coins_processed{0}; while (coins_left > 0) { @@ -4921,19 +4983,14 @@ bool ChainstateManager::PopulateAndValidateSnapshot( const auto snapshot_cache_state = WITH_LOCK(::cs_main, return snapshot_chainstate.GetCoinsCacheSizeState()); - if (snapshot_cache_state >= - CoinsCacheSizeState::CRITICAL) { - LogPrintf("[snapshot] flushing coins cache (%.2f MB)... ", /* Continued */ - coins_cache.DynamicMemoryUsage() / (1000 * 1000)); - flush_now = GetTimeMillis(); - + if (snapshot_cache_state >= CoinsCacheSizeState::CRITICAL) { // This is a hack - we don't know what the actual best block is, but that // doesn't matter for the purposes of flushing the cache here. We'll set this // to its correct value (`base_blockhash`) below after the coins are loaded. coins_cache.SetBestBlock(GetRandHash()); - coins_cache.Flush(); - LogPrintf("done (%.2fms)\n", GetTimeMillis() - flush_now); + // No need to acquire cs_main since this chainstate isn't being used yet. + FlushSnapshotToDisk(coins_cache, /*snapshot_loaded=*/false); } } } @@ -4963,9 +5020,8 @@ bool ChainstateManager::PopulateAndValidateSnapshot( coins_cache.DynamicMemoryUsage() / (1000 * 1000), base_blockhash.ToString()); - LogPrintf("[snapshot] flushing snapshot chainstate to disk\n"); // No need to acquire cs_main since this chainstate isn't being used yet. - coins_cache.Flush(); // TODO: if #17487 is merged, add erase=false here for better performance. + FlushSnapshotToDisk(coins_cache, /*snapshot_loaded=*/true); assert(coins_cache.GetBestBlock() == base_blockhash); @@ -4995,7 +5051,14 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // Fake various pieces of CBlockIndex state: CBlockIndex* index = nullptr; - for (int i = 0; i <= snapshot_chainstate.m_chain.Height(); ++i) { + + // Don't make any modifications to the genesis block. + // This is especially important because we don't want to erroneously + // apply BLOCK_ASSUMED_VALID to genesis, which would happen if we didn't skip + // it here (since it apparently isn't BLOCK_VALID_SCRIPTS). + constexpr int AFTER_GENESIS_START{1}; + + for (int i = AFTER_GENESIS_START; i <= snapshot_chainstate.m_chain.Height(); ++i) { index = snapshot_chainstate.m_chain[i]; // Fake nTx so that LoadBlockIndex() loads assumed-valid CBlockIndex @@ -5004,7 +5067,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( index->nTx = 1; } // Fake nChainTx so that GuessVerificationProgress reports accurately - index->nChainTx = index->pprev ? index->pprev->nChainTx + index->nTx : 1; + index->nChainTx = index->pprev->nChainTx + index->nTx; // Mark unvalidated block index entries beneath the snapshot base block as assumed-valid. if (!index->IsValid(BLOCK_VALID_SCRIPTS)) { @@ -5015,7 +5078,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // Fake BLOCK_OPT_WITNESS so that CChainState::NeedsRedownload() // won't ask to rewind the entire assumed-valid chain on startup. - if (index->pprev && DeploymentActiveAt(*index, ::Params().GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { + if (DeploymentActiveAt(*index, ::Params().GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { index->nStatus |= BLOCK_OPT_WITNESS; } diff --git a/src/validation.h b/src/validation.h index 7457ca5239..534df9bc87 100644 --- a/src/validation.h +++ b/src/validation.h @@ -433,20 +433,16 @@ public: std::unique_ptr<CBlockTreeDB> m_block_tree_db GUARDED_BY(::cs_main); - bool LoadBlockIndexDB(std::set<CBlockIndex*, CBlockIndexWorkComparator>& setBlockIndexCandidates) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool LoadBlockIndexDB(ChainstateManager& chainman) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** * Load the blocktree off disk and into memory. Populate certain metadata * per index entry (nStatus, nChainWork, nTimeMax, etc.) as well as peripheral * collections like setDirtyBlockIndex. - * - * @param[out] block_index_candidates Fill this set with any valid blocks for - * which we've downloaded all transactions. */ bool LoadBlockIndex( const Consensus::Params& consensus_params, - std::set<CBlockIndex*, CBlockIndexWorkComparator>& block_index_candidates) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); + ChainstateManager& chainman) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Clear all data members. */ void Unload() EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -626,6 +622,10 @@ public: */ const std::optional<uint256> m_from_snapshot_blockhash; + //! Return true if this chainstate relies on blocks that are assumed-valid. In + //! practice this means it was created based on a UTXO snapshot. + bool reliesOnAssumedValid() { return m_from_snapshot_blockhash.has_value(); } + /** * The set of all CBlockIndex entries with either BLOCK_VALID_TRANSACTIONS (for * itself and all ancestors) *or* BLOCK_ASSUMED_VALID (if using background diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index ddf6a8b829..25874c601c 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -390,8 +390,7 @@ void SelectionResult::ComputeAndSetWaste(CAmount change_cost) CAmount SelectionResult::GetWaste() const { - Assume(m_waste != std::nullopt); - return *m_waste; + return *Assert(m_waste); } CAmount SelectionResult::GetSelectedValue() const @@ -425,8 +424,8 @@ std::vector<CInputCoin> SelectionResult::GetShuffledInputVector() const bool SelectionResult::operator<(SelectionResult other) const { - Assume(m_waste != std::nullopt); - Assume(other.m_waste != std::nullopt); + Assert(m_waste.has_value()); + Assert(other.m_waste.has_value()); // As this operator is only used in std::min_element, we want the result that has more inputs when waste are equal. return *m_waste < *other.m_waste || (*m_waste == *other.m_waste && m_selected_inputs.size() > other.m_selected_inputs.size()); } diff --git a/test/functional/feature_blockfilterindex_prune.py b/test/functional/feature_blockfilterindex_prune.py index 39eb700b4f..83a50c504e 100755 --- a/test/functional/feature_blockfilterindex_prune.py +++ b/test/functional/feature_blockfilterindex_prune.py @@ -24,9 +24,7 @@ class FeatureBlockfilterindexPruneTest(BitcoinTestFramework): self.log.info("check if we can access a blockfilter when pruning is enabled but no blocks are actually pruned") self.sync_index(height=200) assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getbestblockhash())['filter']), 0) - # Mine two batches of blocks to avoid hitting NODE_NETWORK_LIMITED_MIN_BLOCKS disconnection - self.generate(self.nodes[0], 250) - self.generate(self.nodes[0], 250) + self.generate(self.nodes[0], 500) self.sync_index(height=700) self.log.info("prune some blocks") @@ -39,16 +37,29 @@ class FeatureBlockfilterindexPruneTest(BitcoinTestFramework): self.log.info("check if we can access the blockfilter of a pruned block") assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getblockhash(2))['filter']), 0) + # mine and sync index up to a height that will later be the pruneheight + self.generate(self.nodes[0], 298) + self.sync_index(height=998) + self.log.info("start node without blockfilterindex") self.restart_node(0, extra_args=["-fastprune", "-prune=1"]) self.log.info("make sure accessing the blockfilters throws an error") assert_raises_rpc_error(-1, "Index is not enabled for filtertype basic", self.nodes[0].getblockfilter, self.nodes[0].getblockhash(2)) - self.generate(self.nodes[0], 1000) + self.generate(self.nodes[0], 502) + + self.log.info("prune exactly up to the blockfilterindexes best block while blockfilters are disabled") + pruneheight_2 = self.nodes[0].pruneblockchain(1000) + assert_equal(pruneheight_2, 998) + self.restart_node(0, extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1"]) + self.log.info("make sure that we can continue with the partially synced index after having pruned up to the index height") + self.sync_index(height=1500) self.log.info("prune below the blockfilterindexes best block while blockfilters are disabled") - pruneheight_new = self.nodes[0].pruneblockchain(1000) - assert_greater_than(pruneheight_new, pruneheight) + self.restart_node(0, extra_args=["-fastprune", "-prune=1"]) + self.generate(self.nodes[0], 1000) + pruneheight_3 = self.nodes[0].pruneblockchain(2000) + assert_greater_than(pruneheight_3, pruneheight_2) self.stop_node(0) self.log.info("make sure we get an init error when starting the node again with block filters") diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py index b4e0df8a11..24f79dda67 100755 --- a/test/functional/feature_maxuploadtarget.py +++ b/test/functional/feature_maxuploadtarget.py @@ -36,7 +36,7 @@ class MaxUploadTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [[ - "-maxuploadtarget=800", + "-maxuploadtarget=800M", "-acceptnonstdtxn=1", ]] self.supports_cli = False diff --git a/test/functional/p2p_add_connections.py b/test/functional/p2p_add_connections.py index b86502dc85..d919ed7bd3 100755 --- a/test/functional/p2p_add_connections.py +++ b/test/functional/p2p_add_connections.py @@ -6,8 +6,18 @@ from test_framework.p2p import P2PInterface from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import check_node_connections - +from test_framework.util import ( + assert_equal, + check_node_connections, +) + +class P2PFeelerReceiver(P2PInterface): + def on_version(self, message): + # The bitcoind node closes feeler connections as soon as a version + # message is received from the test framework. Don't send any responses + # to the node's version message since the connection will already be + # closed. + pass class P2PAddConnections(BitcoinTestFramework): def set_test_params(self): @@ -85,6 +95,16 @@ class P2PAddConnections(BitcoinTestFramework): check_node_connections(node=self.nodes[1], num_in=5, num_out=10) + self.log.info("Add 1 feeler connection to node 0") + feeler_conn = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=6, connection_type="feeler") + + # Feeler connection is closed + assert not feeler_conn.is_connected + + # Verify version message received + assert_equal(feeler_conn.message_count["version"], 1) + # Feeler connections do not request tx relay + assert_equal(feeler_conn.last_message["version"].relay, 0) if __name__ == '__main__': P2PAddConnections().main() diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 8f7d5114fa..4dd4899f74 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -329,7 +329,7 @@ class BlockchainTest(BitcoinTestFramework): assert 'muhash' not in r # Unknown hash_type raises an error - assert_raises_rpc_error(-8, "foohash is not a valid hash_type", node.gettxoutsetinfo, "foohash") + assert_raises_rpc_error(-8, "'foo hash' is not a valid hash_type", node.gettxoutsetinfo, "foo hash") def _test_getblockheader(self): self.log.info("Test getblockheader") diff --git a/test/functional/rpc_getblockstats.py b/test/functional/rpc_getblockstats.py index 8c08d2ced5..1ea1ee5659 100755 --- a/test/functional/rpc_getblockstats.py +++ b/test/functional/rpc_getblockstats.py @@ -142,17 +142,17 @@ class GetblockstatsTest(BitcoinTestFramework): inv_sel_stat = 'asdfghjkl' inv_stats = [ [inv_sel_stat], - ['minfee' , inv_sel_stat], + ['minfee', inv_sel_stat], [inv_sel_stat, 'minfee'], ['minfee', inv_sel_stat, 'maxfee'], ] for inv_stat in inv_stats: - assert_raises_rpc_error(-8, 'Invalid selected statistic %s' % inv_sel_stat, + assert_raises_rpc_error(-8, f"Invalid selected statistic '{inv_sel_stat}'", self.nodes[0].getblockstats, hash_or_height=1, stats=inv_stat) # Make sure we aren't always returning inv_sel_stat as the culprit stat - assert_raises_rpc_error(-8, 'Invalid selected statistic aaa%s' % inv_sel_stat, - self.nodes[0].getblockstats, hash_or_height=1, stats=['minfee' , 'aaa%s' % inv_sel_stat]) + assert_raises_rpc_error(-8, f"Invalid selected statistic 'aaa{inv_sel_stat}'", + self.nodes[0].getblockstats, hash_or_height=1, stats=['minfee', f'aaa{inv_sel_stat}']) # Mainchain's genesis block shouldn't be found on regtest assert_raises_rpc_error(-5, 'Block not found', self.nodes[0].getblockstats, hash_or_height='000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f') diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index b3279666b2..ca6e341be8 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -578,7 +578,7 @@ class TestNode(): def add_outbound_p2p_connection(self, p2p_conn, *, p2p_idx, connection_type="outbound-full-relay", **kwargs): """Add an outbound p2p connection from node. Must be an - "outbound-full-relay", "block-relay-only" or "addr-fetch" connection. + "outbound-full-relay", "block-relay-only", "addr-fetch" or "feeler" connection. This method adds the p2p connection to the self.p2ps list and returns the connection to the caller. @@ -590,11 +590,16 @@ class TestNode(): p2p_conn.peer_accept_connection(connect_cb=addconnection_callback, connect_id=p2p_idx + 1, net=self.chain, timeout_factor=self.timeout_factor, **kwargs)() - p2p_conn.wait_for_connect() - self.p2ps.append(p2p_conn) + if connection_type == "feeler": + # feeler connections are closed as soon as the node receives a `version` message + p2p_conn.wait_until(lambda: p2p_conn.message_count["version"] == 1, check_connected=False) + p2p_conn.wait_until(lambda: not p2p_conn.is_connected, check_connected=False) + else: + p2p_conn.wait_for_connect() + self.p2ps.append(p2p_conn) - p2p_conn.wait_for_verack() - p2p_conn.sync_with_ping() + p2p_conn.wait_for_verack() + p2p_conn.sync_with_ping() return p2p_conn diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index bb7a1a5f45..c41e09b5ef 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -76,8 +76,6 @@ implicit-integer-sign-change:test/skiplist_tests.cpp implicit-integer-sign-change:test/streams_tests.cpp implicit-integer-sign-change:test/transaction_tests.cpp implicit-integer-sign-change:txmempool.cpp -implicit-integer-sign-change:util/strencodings.cpp -implicit-integer-sign-change:util/strencodings.h implicit-integer-sign-change:validation.cpp implicit-integer-sign-change:zmq/zmqpublishnotifier.cpp implicit-signed-integer-truncation,implicit-integer-sign-change:chain.h |