Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[25.x] backports #29531

Merged
merged 13 commits into from Mar 22, 2024
Merged

[25.x] backports #29531

merged 13 commits into from Mar 22, 2024

Conversation

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 1, 2024

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
Copy link
Contributor

DrahtBot commented Mar 2, 2024

🚧 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

Copy link
Member

@josibake josibake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK f93be01

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

@luke-jr
Copy link
Member

luke-jr commented Mar 4, 2024

Suggest resetting to luke-jr@fix_reservedest_failure_pr29510-24 (merges into 24.x, 25.x, and 26.x) for cleaner backports

dergoegge and others added 8 commits March 5, 2024 12:17
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. bitcoin#27608).

Github-Pull: bitcoin#29412
Rebased-From: 49257c0
Slight performance improvement by avoiding duplicate work.

Github-Pull: bitcoin#29412
Rebased-From: 1ec6bbe
@achow101
Copy link
Member Author

achow101 commented Mar 8, 2024

Pushed the final changes for 25.2rc2

@fanquake
Copy link
Member

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

Running tests: amount_tests from test/amount_tests.cpp
make[4]: Entering directory '/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src'
/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
make[3]: *** [Makefile:21916: test/amount_tests.cpp.test] Error 1
make[3]: *** Waiting for unfinished jobs....

@achow101
Copy link
Member Author

but the tests are segfaulting in the ASAN job:

Can't reproduce locally to debug.

@fanquake
Copy link
Member

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

@achow101
Copy link
Member Author

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

Doesn't look like this is an us problem.

Also, it does not segfault consistently.

@achow101
Copy link
Member Author

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.

@glozow
Copy link
Member

glozow commented Mar 21, 2024

backport commits lgtm

Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 27cfda1

@DrahtBot DrahtBot requested a review from josibake March 21, 2024 17:54
@fanquake fanquake added this to the 25.2 milestone Mar 22, 2024
@fanquake fanquake merged commit d5bad0d into bitcoin:25.x Mar 22, 2024
12 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants