Merge #6922: backport: bitcoin#24226, bitcoin#26116, bitcoin#26243, bitcoin#26443, bitcoin#26517, bitcoin#26714, bitcoin#26738, bitcoin#27058, bitcoin#27220, bitcoin#27461

916aafb Merge bitcoin#27461: verify-commits: error and exit cleanly when git is too old. (Andrew Chow)
920d790 Merge bitcoin#27220: doc: update broken str util reference links on developer-notes (fanquake)
2c8b994 Merge bitcoin#27058: contrib: Improve verify-commits.py to work with maintainers leaving (glozow)
13fd397 Merge bitcoin#26714: test: add coverage for unparsable `-maxuploadtarget` (merge-script)
84c07bf Merge bitcoin#26738: test: add coverage for unknown wallet flag in `setwalletflag` (MarcoFalke)
08707f8 Merge bitcoin#26517: test: Changed small_txpuzzle_randfee to return the virtual size instead of the transaction hex for feerate calculation. (MarcoFalke)
12d52b2 Merge bitcoin#24226: rpc: warn that nodes ignore requests for old stale blocks (MarcoFalke)
94b3905 Merge bitcoin#26443: doc: mention BIP86 in doc/bips.md (fanquake)
dfc97a9 Merge bitcoin#26243: test: Remove unused fCheckpointsEnabled from miner_tests (fanquake)
5759d18 Merge bitcoin#26116: rpc: Allow importmulti watchonly imports with locked wallet (Andrew Chow)

Pull request description:

  ## What was done?
  Trivial backports from Bitcoin Core v25

  ## How Has This Been Tested?
  Run & unit functional tests

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 916aafb
  kwvg:
    utACK 916aafb

Tree-SHA512: c728963e5bd2c86c9f4ebfdfe22fcf277701dced1bf1edf38b64738a4ceda3850f7c709912fc64e6b820885845b27f5741ad536b5b86ccb651009663029ff1ab

Author: PastaPastaPasta

contrib/verify-commits/README.md

@@ -27,6 +27,10 @@ Note that the above isn't a good UI/UX yet, and needs significant improvements
 to make it more convenient and reduce the chance of errors; pull-reqs
 improving this process would be much appreciated.
 
+Unless `--clean-merge 0` is specified, `verify-commits.py` will attempt to verify that
+each merge commit applies cleanly (with some exceptions). This requires using at least
+git v2.38.0.
+
 Configuration files
 -------------------
 

contrib/verify-commits/gpg.sh

@@ -5,12 +5,9 @@
 
 export LC_ALL=C
 INPUT=$(cat /dev/stdin)
-VALID=false
-REVSIG=false
-IFS='
-'
 if [ "$BITCOIN_VERIFY_COMMITS_ALLOW_SHA1" = 1 ]; then
-	GPG_RES="$(printf '%s\n' "$INPUT" | gpg --trust-model always "$@" 2>/dev/null)"
+    printf '%s\n' "$INPUT" | gpg --trust-model always "$@" 2>/dev/null
+    exit $?
 else
 	# Note how we've disabled SHA1 with the --weak-digest option, disabling
 	# signatures - including selfsigs - that use SHA1. While you might think that
@@ -20,46 +17,19 @@ else
 	# an attacker could construct a pull-req that results in a commit object that
 	# they've created a collision for. Not the most likely attack, but preventing
 	# it is pretty easy so we do so as a "belt-and-suspenders" measure.
-	GPG_RES=""
 	for LINE in $(gpg --version); do
 		case "$LINE" in
 			"gpg (GnuPG) 1.4.1"*|"gpg (GnuPG) 2.0."*)
 				echo "Please upgrade to at least gpg 2.1.10 to check for weak signatures" > /dev/stderr
-				GPG_RES="$(printf '%s\n' "$INPUT" | gpg --trust-model always "$@" 2>/dev/null)"
+                                printf '%s\n' "$INPUT" | gpg --trust-model always "$@" 2>/dev/null
+                                exit $?
 				;;
 			# We assume if you're running 2.1+, you're probably running 2.1.10+
 			# gpg will fail otherwise
 			# We assume if you're running 1.X, it is either 1.4.1X or 1.4.20+
 			# gpg will fail otherwise
 		esac
 	done
-	[ "$GPG_RES" = "" ] && GPG_RES="$(printf '%s\n' "$INPUT" | gpg --trust-model always --weak-digest sha1 "$@" 2>/dev/null)"
-fi
-for LINE in $GPG_RES; do
-	case "$LINE" in
-	"[GNUPG:] VALIDSIG "*)
-		while read KEY; do
-			[ "${LINE#?GNUPG:? VALIDSIG * * * * * * * * * }" = "$KEY" ] && VALID=true
-		done < ./contrib/verify-commits/trusted-keys
-		;;
-	"[GNUPG:] REVKEYSIG "*)
-		[ "$BITCOIN_VERIFY_COMMITS_ALLOW_REVSIG" != 1 ] && exit 1
-		REVSIG=true
-		GOODREVSIG="[GNUPG:] GOODSIG ${LINE#* * *}"
-		;;
-	"[GNUPG:] EXPKEYSIG "*)
-		[ "$BITCOIN_VERIFY_COMMITS_ALLOW_REVSIG" != 1 ] && exit 1
-		REVSIG=true
-		GOODREVSIG="[GNUPG:] GOODSIG ${LINE#* * *}"
-		;;
-	esac
-done
-if ! $VALID; then
-	exit 1
-fi
-if $VALID && $REVSIG; then
-	printf '%s\n' "$INPUT" | gpg --trust-model always "$@" 2>/dev/null | grep "^\[GNUPG:\] \(NEWSIG\|SIG_ID\|VALIDSIG\)"
-	echo "$GOODREVSIG"
-else
-	printf '%s\n' "$INPUT" | gpg --trust-model always "$@" 2>/dev/null
+        printf '%s\n' "$INPUT" | gpg --trust-model always --weak-digest sha1 "$@" 2>/dev/null
+        exit $?
 fi

contrib/verify-commits/verify-commits.py

@@ -92,8 +92,10 @@ def main():
         unclean_merge_allowed = f.read().splitlines()
     with open(dirname + "/allow-incorrect-sha512-commits", "r", encoding="utf8") as f:
         incorrect_sha512_allowed = f.read().splitlines()
+    with open(dirname + "/trusted-keys", "r", encoding="utf8") as f:
+        trusted_keys = f.read().splitlines()
 
-    # Set commit and branch and set variables
+    # Set commit and variables
     current_commit = args.commit
     if ' ' in current_commit:
         print("Commit must not contain spaces", file=sys.stderr)
@@ -102,7 +104,6 @@ def main():
     no_sha1 = True
     prev_commit = ""
     initial_commit = current_commit
-    branch = subprocess.check_output([GIT, 'show', '-s', '--format=%H', initial_commit]).decode('utf8').splitlines()[0]
 
     # Iterate through commits
     while True:
@@ -113,17 +114,41 @@ def main():
         if current_commit == verified_root:
             print('There is a valid path from "{}" to {} where all commits are signed!'.format(initial_commit, verified_root))
             sys.exit(0)
-        if current_commit == verified_sha512_root:
-            if verify_tree:
+        else:
+            # Make sure this commit isn't older than trusted roots
+            check_root_older_res = subprocess.run([GIT, "merge-base", "--is-ancestor", verified_root, current_commit])
+            if check_root_older_res.returncode != 0:
+                print(f"\"{current_commit}\" predates the trusted root, stopping!")
+                sys.exit(0)
+
+        if verify_tree:
+            if current_commit == verified_sha512_root:
                 print("All Tree-SHA512s matched up to {}".format(verified_sha512_root), file=sys.stderr)
-            verify_tree = False
-            no_sha1 = False
+                verify_tree = False
+                no_sha1 = False
+            else:
+                # Skip the tree check if we are older than the trusted root
+                check_root_older_res = subprocess.run([GIT, "merge-base", "--is-ancestor", verified_sha512_root, current_commit])
+                if check_root_older_res.returncode != 0:
+                    print(f"\"{current_commit}\" predates the trusted SHA512 root, disabling tree verification.")
+                    verify_tree = False
+                    no_sha1 = False
+
 
         os.environ['BITCOIN_VERIFY_COMMITS_ALLOW_SHA1'] = "0" if no_sha1 else "1"
-        os.environ['BITCOIN_VERIFY_COMMITS_ALLOW_REVSIG'] = "1" if current_commit in revsig_allowed else "0"
+        allow_revsig = current_commit in revsig_allowed
 
         # Check that the commit (and parents) was signed with a trusted key
-        if subprocess.call([GIT, '-c', 'gpg.program={}/gpg.sh'.format(dirname), 'verify-commit', current_commit], stdout=subprocess.DEVNULL):
+        valid_sig = False
+        verify_res = subprocess.run([GIT, '-c', 'gpg.program={}/gpg.sh'.format(dirname), 'verify-commit', "--raw", current_commit], capture_output=True)
+        for line in verify_res.stderr.decode().splitlines():
+            if line.startswith("[GNUPG:] VALIDSIG "):
+                key = line.split(" ")[-1]
+                valid_sig = key in trusted_keys
+            elif (line.startswith("[GNUPG:] REVKEYSIG ") or line.startswith("[GNUPG:] EXPKEYSIG ")) and not allow_revsig:
+                valid_sig = False
+                break
+        if not valid_sig:
             if prev_commit != "":
                 print("No parent of {} was signed with a trusted key!".format(prev_commit), file=sys.stderr)
                 print("Parents are:", file=sys.stderr)
@@ -153,15 +178,24 @@ def main():
         allow_unclean = current_commit in unclean_merge_allowed
         if len(parents) == 2 and check_merge and not allow_unclean:
             current_tree = subprocess.check_output([GIT, 'show', '--format=%T', current_commit]).decode('utf8').splitlines()[0]
-            subprocess.call([GIT, 'checkout', '--force', '--quiet', parents[0]])
-            subprocess.call([GIT, 'merge', '--no-ff', '--quiet', '--no-gpg-sign', parents[1]], stdout=subprocess.DEVNULL)
-            recreated_tree = subprocess.check_output([GIT, 'show', '--format=format:%T', 'HEAD']).decode('utf8').splitlines()[0]
+
+            # This merge-tree functionality requires git >= 2.38. The
+            # --write-tree option was added in order to opt-in to the new
+            # behavior. Older versions of git will not recognize the option and
+            # will instead exit with code 128.
+            try:
+                recreated_tree = subprocess.check_output([GIT, "merge-tree", "--write-tree", parents[0], parents[1]]).decode('utf8').splitlines()[0]
+            except subprocess.CalledProcessError as e:
+                if e.returncode == 128:
+                    print("git v2.38+ is required for this functionality.", file=sys.stderr)
+                    sys.exit(1)
+                else:
+                    raise e
+
             if current_tree != recreated_tree:
                 print("Merge commit {} is not clean".format(current_commit), file=sys.stderr)
-                subprocess.call([GIT, 'diff', current_commit])
-                subprocess.call([GIT, 'checkout', '--force', '--quiet', branch])
+                subprocess.call([GIT, 'diff', recreated_tree, current_tree])
                 sys.exit(1)
-            subprocess.call([GIT, 'checkout', '--force', '--quiet', branch])
 
         prev_commit = current_commit
         current_commit = parents[0]

doc/bips.md

@@ -29,6 +29,7 @@ Versions and PRs are relevant to Bitcoin's core if not mentioned other.
   and it is disabled by default at build time since **v0.19.0** ([PR #15584](https://github.com/bitcoin/bitcoin/pull/15584)).
   It has been removed as of **v0.20.0** ([PR 17165](https://github.com/bitcoin/bitcoin/pull/17165)).
 * [`BIP 84`](https://github.com/bitcoin/bips/blob/master/bip-0084.mediawiki): The experimental descriptor wallets introduced in **v0.21.0** by default use the Hierarchical Deterministic Wallet derivation proposed by BIP 84. ([PR #16528](https://github.com/bitcoin/bitcoin/pull/16528))
+* [`BIP 86`](https://github.com/bitcoin/bips/blob/master/bip-0086.mediawiki): Descriptor wallets by default use the Hierarchical Deterministic Wallet derivation proposed by BIP 86 since **v23.0** ([PR #22364](https://github.com/bitcoin/bitcoin/pull/22364)).
 * [`BIP 90`](https://github.com/bitcoin/bips/blob/master/bip-0090.mediawiki): Trigger mechanism for activation of BIPs 34, 65, and 66 has been simplified to block height checks since **v0.14.0** ([PR #8391](https://github.com/bitcoin/bitcoin/pull/8391)).
 * [`BIP 111`](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki): `NODE_BLOOM` service bit added, and enforced for all peer versions as of **v0.13.0** ([PR #6579](https://github.com/bitcoin/bitcoin/pull/6579) and [PR #6641](https://github.com/bitcoin/bitcoin/pull/6641)).
 * [`BIP 112`](https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki): The CHECKSEQUENCEVERIFY opcode has been implemented since **v0.12.1** ([PR #7524](https://github.com/bitcoin/bitcoin/pull/7524)), and has been *buried* since **v0.19.0** ([PR #16060](https://github.com/bitcoin/bitcoin/pull/16060)).

doc/developer-notes.md

@@ -866,12 +866,12 @@ Strings and formatting
     buffer overflows, and surprises with `\0` characters. Also, some C string manipulations
     tend to act differently depending on platform, or even the user locale.
 
-- Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing.
+- Use `ToIntegral` from [`strencodings.h`](/src/util/strencodings.h) for number parsing. In legacy code you might also find `ParseInt*` family of functions, `ParseDouble` or `LocaleIndependentAtoi`.
 
   - *Rationale*: These functions do overflow checking and avoid pesky locale issues.
 
 - Avoid using locale dependent functions if possible. You can use the provided
-  [`lint-locale-dependence.sh`](/test/lint/lint-locale-dependence.sh)
+  [`lint-locale-dependence.py`](/test/lint/lint-locale-dependence.py)
   to check for accidental use of locale dependent functions.
 
   - *Rationale*: Unnecessary locale dependence can cause bugs that are very tricky to isolate and fix.

src/net_processing.cpp

@@ -1935,6 +1935,8 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl
 
     LOCK(cs_main);
     // Mark block as in-flight unless it already is (for this peer).
+    // If the peer does not send us a block, vBlocksInFlight remains non-empty,
+    // causing us to timeout and disconnect.
     // If a block was already in-flight for a different peer, its BLOCKTXN
     // response will be dropped.
     if (!BlockRequested(peer_id, block_index)) return "Already requested from this peer";

src/rpc/blockchain.cpp

@@ -483,7 +483,9 @@ static RPCHelpMan getblockfrompeer()
         "getblockfrompeer",
         "Attempt to fetch block from a given peer.\n\n"
         "We must have the header for this block, e.g. using submitheader.\n"
-        "Subsequent calls for the same block and a new peer will cause the response from the previous peer to be ignored.\n\n"
+        "Subsequent calls for the same block and a new peer will cause the response from the previous peer to be ignored.\n"
+        "Peers generally ignore requests for a stale block that they never fully verified, or one that is more than a month old.\n"
+        "When a peer does not respond with a block, we will disconnect.\n\n"
         "Returns an empty JSON object if the request was successfully scheduled.",
         {
             {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash to try to fetch"},

src/test/miner_tests.cpp

@@ -565,8 +565,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
     CScript scriptPubKey = CScript() << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f") << OP_CHECKSIG;
     std::unique_ptr<CBlockTemplate> pblocktemplate, pemptyblocktemplate;
 
-    fCheckpointsEnabled = false;
-
     // Simple block creation, nothing special yet:
     BOOST_CHECK(pemptyblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey));
 
@@ -643,8 +641,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
     m_node.mempool->clear();
 
     TestPrioritisedMining(chainparams, scriptPubKey, txFirst);
-
-    fCheckpointsEnabled = true;
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/wallet/rpc/backup.cpp

@@ -1582,7 +1582,18 @@ RPCHelpMan importmulti()
     UniValue response(UniValue::VARR);
     {
         LOCK(pwallet->cs_wallet);
-        EnsureWalletIsUnlocked(*pwallet);
+
+        // Check all requests are watchonly
+        bool is_watchonly{true};
+        for (size_t i = 0; i < requests.size(); ++i) {
+            const UniValue& request = requests[i];
+            if (!request.exists("watchonly") || !request["watchonly"].get_bool()) {
+                is_watchonly = false;
+                break;
+            }
+        }
+        // Wallet does not need to be unlocked if all requests are watchonly
+        if (!is_watchonly) EnsureWalletIsUnlocked(wallet);
 
         // Verify all timestamps are present before importing any keys.
         CHECK_NONFATAL(pwallet->chain().findBlock(pwallet->GetLastBlockHash(), FoundBlock().time(nLowestTimestamp).mtpTime(now)));

test/functional/feature_fee_estimation.py

@@ -61,7 +61,7 @@ def small_txpuzzle_randfee(
     unconflist.append({"txid": txid, "vout": 0, "value": total_in - amount - fee})
     unconflist.append({"txid": txid, "vout": 1, "value": amount})
 
-    return (tx.serialize().hex(), fee)
+    return (tx.get_vsize(), fee)
 
 
 def check_raw_estimates(node, fees_seen):
@@ -148,7 +148,7 @@ def transact_and_mine(self, numblocks, mining_node):
             random.shuffle(self.confutxo)
             for _ in range(random.randrange(100 - 50, 100 + 50)):
                 from_index = random.randint(1, 2)
-                (txhex, fee) = small_txpuzzle_randfee(
+                (tx_bytes, fee) = small_txpuzzle_randfee(
                     self.wallet,
                     self.nodes[from_index],
                     self.confutxo,
@@ -157,7 +157,7 @@ def transact_and_mine(self, numblocks, mining_node):
                     min_fee,
                     min_fee,
                 )
-                tx_kbytes = (len(txhex) // 2) / 1000.0
+                tx_kbytes = tx_bytes / 1000.0
                 self.fees_per_kb.append(float(fee) / tx_kbytes)
             self.sync_mempools(wait=0.1)
             mined = mining_node.getblock(self.generate(mining_node, 1)[0], True)["tx"]