[25.x] backports

achow101 commented at 10:36 pm on March 1, 2024: member

Backport:

wallet: Avoid updating `ReserveDestination::nIndex` when `GetReservedDestination` fails

Github-Pull: bitcoin/bitcoin#29510
Rebased-From: 367bb7a80cc71130995672c853d4a6e0134721d6

7c08ccf19b

test: make sure keypool sizes do not change on `getrawchangeaddress`/`getnewaddress` failures

Github-Pull: bitcoin/bitcoin#29510
Rebased-From: e073f1dfda7a2a2cb2be9fe2a1d576f122596021

f93be0103f

DrahtBot commented at 10:36 pm on March 1, 2024: contributor

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	glozow
Stale ACK	josibake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Backport on Mar 1, 2024

DrahtBot added the label CI failed on Mar 2, 2024

DrahtBot commented at 0:41 am on March 2, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/22191422369

fanquake requested review from josibake on Mar 4, 2024

josibake commented at 1:25 pm on March 4, 2024: member

ACK https://github.com/bitcoin/bitcoin/pull/29531/commits/f93be0103fe9cb92a376848fc534233b48977918

Verified that the correct commits are referenced in the commit messages here and that the content matches

luke-jr commented at 8:22 pm on March 4, 2024: member

Suggest resetting to https://github.com/luke-jr/bitcoin/commit/fix_reservedest_failure_pr29510-24 (merges into 24.x, 25.x, and 26.x) for cleaner backports

[validation] Isolate merkle root checks

Github-Pull: #29412
Rebased-From: 95bddb930aa72edd40fdff52cf447202995b0dce

4f5baac6ca

[validation] Introduce IsBlockMutated

Github-Pull: #29412
Rebased-From: 66abce1d98115e41f394bc4f4f52341960ddc839

8804c368f5

[validation] Merkle root malleation should be caught by IsBlockMutated

Github-Pull: #29412
Rebased-From: 2d8495e0800f5332748cd50eaad801ff77671bba

098f07dc8d

[net processing] Don't process mutated blocks

We preemptively perform a block mutation check before further processing
a block message (similar to early sanity checks on other messsage
types). The main reasons for this change are as follows:

- `CBlock::GetHash()` is a foot-gun without a prior mutation check, as
  the hash returned only commits to the header but not to the actual
  transactions (`CBlock::vtx`) contained in the block.
- We have observed attacks that abused mutated blocks in the past, which
  could have been prevented by simply not processing mutated blocks
  (e.g. https://github.com/bitcoin/bitcoin/pull/27608).

Github-Pull: #29412
Rebased-From: 49257c0304828a185c273fcb99742c54bbef0c8e

8cc4b24c74

[test] Add regression test for #27608

Github-Pull: #29412
Rebased-From: 5bf4f5ba32da4627f152b54d266df9b2aa930457

de97ecf14f

[validation] Cache merkle root and witness commitment checks

Slight performance improvement by avoiding duplicate work.

Github-Pull: #29412
Rebased-From: 1ec6bbeb8d27d31647d1433ccb87b362f6d81f90

0667441a7b

[test] IsBlockMutated unit tests

Github-Pull: #29412
Rebased-From: d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc

3eaaafa225

p2p: Don't consider blocks mutated if they don't connect to known prev block

Github-Pull: #29524
Rebased-From: a1fbde0ef7cf6c94d4a5181f8ceb327096713160

cf7d3a8cd0

build: Bump to 25.2rc2 8a0c980d6e

doc: Update manpages for 25.2rc2 daba5e2c5b

achow101 commented at 5:23 pm on March 8, 2024: member

Pushed the final changes for 25.2rc2

fanquake commented at 10:20 am on March 12, 2024: member

The win64 CI can be ignored, and the TSAN failure is an upstream issue, but the tests are segfaulting in the ASAN job:

0Running tests: amount_tests from test/amount_tests.cpp
1make[4]: Entering directory '/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src'
2/bin/bash: line 2: 26108 Segmentation fault      (core dumped) test/test_bitcoin --catch_system_errors=no -l test_suite -t "$( cat test/amount_tests.cpp | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1 )" -- DEBUG_LOG_OUT > "$TEST_LOGFILE" 2>&1
3make[3]: *** [Makefile:21916: test/amount_tests.cpp.test] Error 1
4make[3]: *** Waiting for unfinished jobs....

achow101 commented at 10:56 am on March 12, 2024: member

but the tests are segfaulting in the ASAN job:

Can’t reproduce locally to debug.

fanquake commented at 11:22 am on March 12, 2024: member

You can run the ASAN CI job locally, it recreates there.

achow101 commented at 1:12 pm on March 12, 2024: member

 0Program received signal SIGSEGV, Segmentation fault.
 10x0000618e63bd4ce1 in __sanitizer::internal_mmap(void*, unsigned long, int, int, int, unsigned long long) ()
 2(gdb) bt
 3[#0](/bitcoin-bitcoin/0/)  0x0000618e63bd4ce1 in __sanitizer::internal_mmap(void*, unsigned long, int, int, int, unsigned long long) ()
 4[#1](/bitcoin-bitcoin/1/)  0x0000618e63bd65da in __sanitizer::MmapNamed(void*, unsigned long, int, int, char const*) ()
 5[#2](/bitcoin-bitcoin/2/)  0x0000618e63be0398 in __sanitizer::ReservedAddressRange::Init(unsigned long, char const*, unsigned long) ()
 6[#3](/bitcoin-bitcoin/3/)  0x0000618e63b3d07a in __sanitizer::SizeClassAllocator64<__asan::AP64<__sanitizer::LocalAddressSpaceView> >::Init(int, unsigned long) ()
 7[#4](/bitcoin-bitcoin/4/)  0x0000618e63b3a37d in __asan::Allocator::InitLinkerInitialized(__asan::AllocatorOptions const&) ()
 8[#5](/bitcoin-bitcoin/5/)  0x0000618e63bc4f44 in __asan::AsanInitInternal() ()
 9[#6](/bitcoin-bitcoin/6/)  0x00007434223fd5be in _dl_init (main_map=0x7434224322e0, argc=3, argv=0x7ffe15237df8, env=0x7ffe15237e18)
10    at ./elf/dl-init.c:102
11[#7](/bitcoin-bitcoin/7/)  0x00007434224172ca in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
12[#8](/bitcoin-bitcoin/8/)  0x0000000000000003 in ?? ()
13[#9](/bitcoin-bitcoin/9/)  0x00007ffe15239302 in ?? ()
14[#10](/bitcoin-bitcoin/10/) 0x00007ffe15239346 in ?? ()
15[#11](/bitcoin-bitcoin/11/) 0x00007ffe15239349 in ?? ()
16[#12](/bitcoin-bitcoin/12/) 0x0000000000000000 in ?? ()
17(gdb)

Doesn’t look like this is an us problem.

Also, it does not segfault consistently.

achow101 commented at 1:54 pm on March 12, 2024: member

In my testing, it seems that the ASAN task intermittently segfaults on current 25.x. I’m unable to figure out which PR actually fixes the problem. If we’re okay with ignoring CI that fails because of upstream issues, then I’m fine with ignoring this failure. But if someone wants to open a backport PR that fixes it, I’ll be happy to review that.

in doc/release-notes.md:59 in 67af36297a outdated

50@@ -51,6 +51,12 @@ Notable changes
51 ### Wallet
52 
53 - #29176 wallet: Fix use-after-free in WalletBatch::EraseRecords
54+- #29510 wallet: `getrawchangeaddress` and `getnewaddress` failures should not affect keypools for descriptor wallets
55+
56+### P2P and network changes
57+
58+- #29412 p2p: Don't process mutated blocks
59+- #29524 p2p: Don't consider blocks mutated if they don't connect to known prev block

glozow commented at 5:29 pm on March 21, 2024:

Need to update the Credits section

achow101 commented at 5:44 pm on March 21, 2024:

Done

glozow commented at 5:31 pm on March 21, 2024: member

backport commits lgtm

doc: Update release notes for 25.2rc2 27cfda1bae

achow101 force-pushed on Mar 21, 2024

glozow commented at 5:54 pm on March 21, 2024: member

utACK 27cfda1baec63ee4fb0f743576227528104fe495

DrahtBot requested review from josibake on Mar 21, 2024

fanquake added this to the milestone 25.2 on Mar 22, 2024

fanquake merged this on Mar 22, 2024

fanquake closed this on Mar 22, 2024

[25.x] backports #29531

Code Coverage

Reviews