Skip to content

[29.x] backports and rc2#32062

Merged
fanquake merged 10 commits intobitcoin:29.xfrom
glozow:2025-03-29-rc2
Mar 18, 2025
Merged

[29.x] backports and rc2#32062
fanquake merged 10 commits intobitcoin:29.xfrom
glozow:2025-03-29-rc2

Conversation

@glozow
Copy link
Member

@glozow glozow commented Mar 13, 2025 •
edited
Loading

@davidgumberg @glozow
contrib: Fix gen-bitcoin-conf.sh.
80c5d57 
In bitcoin#31118, the format of bitcoind's `--help` output changed slightly in
a way that breaks `gen-bitcoin-conf.sh`, modify the script to accomodate
the new format, by starting after the line that says "Options:" and
strip the `-help` option and its description from the output.

Github-Pull: bitcoin#32049
Rebased-From: a24419f
@glozow glozow added this to the 29.0 milestone Mar 13, 2025
@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 13, 2025 •
edited
Loading

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32062.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK Sjors, hebasto, ismaelsadeeq
Stale ACK davidgumberg

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

@davidgumberg
Copy link
Contributor

davidgumberg commented Mar 13, 2025

lgtm ACK 188ec87

@fanquake fanquake mentioned this pull request Mar 14, 2025
Merged
l0rinc and others added 5 commits March 16, 2025 22:07
@l0rinc @maflcko
@marcofleon @mzumsande @glozow
fuzz: Always restrict base conversion input lengths
15ecae3 
They seem to cause timeouts:
> Issue 397734700: bitcoin-core:base58check_encode_decode: Timeout in base58check_encode_decode

The `encoded_string.empty()` check was corrected here to `decoded.empty()` to make sure the `(0, decoded.size() - 1)` range is always valid.

Github-Pull: bitcoin#31917
Rebased-From: bad1433

Co-authored-by: maflcko <[email protected]>
Co-authored-by: marcofleon <[email protected]>
Co-authored-by: Martin Zumsande <[email protected]>
@l0rinc @maflcko
@marcofleon @glozow
fuzz: make sure DecodeBase58(Check) is called with valid values more …
458655b 
…often

In Base58 fuzz the two roundtrips are merged now, the new `decode_input` switches between a completely random input and a valid encoded one, to make sure the decoding passes more often.
The `max_ret_len` can also exceed the original length now and is being validated more thoroughly.

Github-Pull: bitcoin#31917
Rebased-From: d5537c1

Co-authored-by: maflcko <[email protected]>
Co-authored-by: marcofleon <[email protected]>
@mzumsande @glozow
test: fix intermittent failure in p2p_orphan_handling.py
5ebcb59 
If we bump the mocktime before the node has successfully disconnected
the peer, the requests for both parents could be spread over
two GETDATAS, which would make the test fail.

Github-Pull: bitcoin#32063
Rebased-From: 0294205
@achow101 @glozow
build: Remove manpages when making MacOS app
7ff0b02 
Github-Pull: bitcoin#32064
Rebased-From: 80b5e7f
@Sjors @ryanofsky @glozow
build: use make < 3.82 syntax for define directive
4e438d3 
From the GNU make 3.82 release announcement:

* The 'define' make directive now allows a variable assignment operator
  after the variable name, to allow for simple, conditional, or appending
  multi-line variable assignment.

macOS ships with 3.81. This caused the multiprocess config options
to be ignored.

Fixes bitcoin#32068

Github-Pull: bitcoin#32070
Rebased-From: 9157d9e

Co-authored-by: Ryan Ofsky <[email protected]>
This was referenced Mar 17, 2025
Merged
Merged
Merged
Merged
@glozow glozow marked this pull request as ready for review March 17, 2025 02:18
@fanquake
Copy link
Member

fanquake commented Mar 17, 2025

Can also pull in bitcoin-core/gui#858 (if not this rc than next).

@Sjors
Copy link
Member

Sjors commented Mar 17, 2025 •
edited
Loading

ACK 02d030292a93fdf77bf59c8007fea3cebb2bf3fa

I cherry-picked the pull requests, ran gen-bitcoin-conf.sh (built with -DWITH_ZMQ=ON) and compared to this PR (minus the version bump commits).

I do get a slightly difference in share/examples/bitcoin.conf, maybe due to a build option?

-# estimatefee, http, i2p, ipc, leveldb, libevent, mempool,
+# estimatefee, http, i2p, ipc, leveldb, libevent, lock, mempool,

Update: it's because I had to delete build, rather than just remove -DCMAKE_BUILD_TYPE=Debug

@DrahtBot DrahtBot requested a review from davidgumberg March 17, 2025 08:15
@hebasto
Copy link
Member

hebasto commented Mar 17, 2025

Can also pull in bitcoin-core/gui#858 (if not this rc than next).

+1

@glozow
Copy link
Member Author

glozow commented Mar 17, 2025

Can also pull in bitcoin-core/gui#858 (if not this rc than next).

added

@Sjors
Copy link
Member

Sjors commented Mar 17, 2025

ACK 74df31c

Don't forget to add bitcoin-core/gui#858 to the PR description.

hebasto
hebasto approved these changes Mar 17, 2025
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 74df31c, I have reviewed the code and it looks OK.

I've backported PRs and generated contrib/devtools/gen-bitcoin-conf.sh locally and got zero-diff with this PR branch.

ismaelsadeeq
ismaelsadeeq reviewed Mar 17, 2025
View reviewed changes
Copy link
Member

@ismaelsadeeq ismaelsadeeq left a comment

Choose a reason for hiding this comment

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

Code review ACK 74df31c

I've verified that the patch-ids are identical. Only 4e438d3 has a different patch-id from the original commit, and that's due to line number differences.

I've verified that the backported commits is identical to the PR commits.

@fanquake fanquake merged commit d6db871 into bitcoin:29.x Mar 18, 2025
17 of 18 checks passed
@glozow glozow deleted the 2025-03-29-rc2 branch March 18, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hebasto hebasto hebasto approved these changes

@davidgumberg davidgumberg Awaiting requested review from davidgumberg

+1 more reviewer

@ismaelsadeeq ismaelsadeeq ismaelsadeeq left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Backport

Projects

None yet

Milestone

29.0

Development

Successfully merging this pull request may close these issues.