Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 laanwj approved a pull request: "doc: Extend developer-notes with file-name-only debugging fix"
(https://github.com/bitcoin/bitcoin/pull/30670#pullrequestreview-2401819547)
ACK 1b0b9b4c7873ff0c6323de0c7f439466eed06049
👍 hebasto approved a pull request: "Fix unsigned integer overflows in interpreter"
(https://github.com/bitcoin/bitcoin/pull/24214#pullrequestreview-2401888182)
ACK bbbbaa0d9ac9ae9c9b8109503aa30213eed543b9, I have reviewed the code and it looks OK.

A decent alternative to https://github.com/bitcoin/bitcoin/pull/29543.
💬 ryanofsky commented on issue "ci: intermittent issue in rpc_misc.py node0 stderr terminate called after throwing an instance of 'kj::ExceptionImpl' [15:12:14.943] what(): mp/proxy.cpp:242: disconnected: write(m_post_fd, &buffer, 1): Broken pipe [15:12:14.943] stack: 657ed083 657ed86e f7ac7d20 f7713156 f77a7e07 ":
(https://github.com/bitcoin/bitcoin/issues/31151#issuecomment-2444255035)
Looking more closely at changes in #31105, I don't see anything that directly relates to this code, so it seems possible this is an older bug or a different bug. Looking up the "Broken pipe" error, it also seems like that can only be triggered if something is closing the `EventLoop::m_wait_fd` file descriptor before the `write(m_post_fd, ...)` call happens. But I don't see a code path where that would happen. May need to reproduce locally to debug. Would be curious to know if this happens more.
👍 TheCharlatan approved a pull request: "ci: Use clang-19 from apt.llvm.org"
(https://github.com/bitcoin/bitcoin/pull/30634#pullrequestreview-2401970706)
ACK fabe90c8242aa45a8b9925347ca6fc11d5852ffd
💬 laanwj commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2444317433)
> I implemented your suggestion by adding the --skip-missing-binaries option to the command.

Yes, thanks!

> I wonder if a better approach here would be to further integrate the manpage generation into the build system; given that the two issues of "which binaries exist" and "where do they exist" are already solved in that case. This should also remove the dep on git.

That would be better. The thing is, the build system isn't allowed to execute the binaries it generated (and even if so t
...
💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1820864917)
I'll let the others weigh in a bit before touching docs more
💬 0xB10C commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1820876152)
Thanks for the ping, sorry for the late response. I agree that a "package hash" on it's own might not be useful for someone interested in "which transactions were replaced by which other transactions". On the other hand, the tracepoints are on purpose only semi-stable as they expose internals. If the internals change, the tracepoints need to change too.

> which (as of this PR) would now report the package hash along with the transaction ids that are in it?

That could be useful, yes. Howev
...
💬 sipa commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1820886255)
You can mimic what we did for BIP61 in the same document (which just lists the history of when it was introduced, and when it was removed).
⚠️ fanquake opened an issue: "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously"
(https://github.com/bitcoin/bitcoin/issues/31178)
According to https://corecheck.dev/benchmarks, the `linearizeoptimallyexample11` benchmark 4x'd it's execution time at some point in the last few days.
![Screenshot 2024-10-29 at 14 15 02](https://github.com/user-attachments/assets/96b20c6c-09af-4233-b643-64df88ed225f)
💬 Christewart commented on pull request "consensus: fix `OP_1NEGATE` handling in `CScriptOp`":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2444403494)
> Hey @Christewart! Do you plan to update for cmake?

Done, i squashed everything into 1 commit as well.
👍 hebasto approved a pull request: "ci: Use clang-19 from apt.llvm.org"
(https://github.com/bitcoin/bitcoin/pull/30634#pullrequestreview-2402063455)
ACK fabe90c8242aa45a8b9925347ca6fc11d5852ffd, I have reviewed the code and it looks OK.
💬 sipa commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2444413270)
More like 2x (from 0.64 to 1.21)? Still bizarre, this code has not been touched the past month for as far as I can see?
💬 fanquake commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2444418197)
I was just looking at total execution time increasing from ~4.3s to ~16.3.
💬 sipa commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2444420462)
Oh nevermind, I was looking at the "outliers" panel, which doesn't give absolute time. Indeed, 4x.
🤔 jonatack reviewed a pull request: "rpc, cli: return "verificationprogress" of 1 when up to date"
(https://github.com/bitcoin/bitcoin/pull/31177#pullrequestreview-2402104620)
Concept ACK, some ideas to encourage review:

- make sure your proposal compiles in your local environment, with passing unit and functional tests, so updating all the callers of `GuessVerificationProgress`. This makes it easier for reviewers to build and test your code and see if any test coverage needs to be updated or possibly added. Some reviewers may also wait until your pull has a green CI as enough proof of work before reviewing it more closely.
- squash your commits

This [article](
...
💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1820926787)
Ah, yeah, right, that was stupid. I'll stick to just renaming `use_pcp` to `enabled`.
💬 mzumsande commented on pull request "test: Don't enforce BIP94 on regtest unless specified by arg":
(https://github.com/bitcoin/bitcoin/pull/31156#discussion_r1820928668)
Just following the process described in https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#release-notes - a change to that process should be discussed in a separate issue / PR - whatever works best for the maintainers I guess, personally I don't care at all.
💬 hebasto commented on pull request "refactor: Clean up messy strformat and bilingual_str usages":
(https://github.com/bitcoin/bitcoin/pull/31072#issuecomment-2444460496)
Concept ACK.
💬 l0rinc commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1820938133)
we still have unbounded increment without checking the end (I though we've fixed this already, maybe it got stuck in the comments...)
💬 mzumsande commented on pull request "test: Don't enforce BIP94 on regtest unless specified by arg":
(https://github.com/bitcoin/bitcoin/pull/31156#discussion_r1820940387)
By the way, the reason I added a release note is that there also was one in the [28.0 release notes](https://bitcoincore.org/en/releases/28.0/) about introducing this.