💬 maflcko commented on issue "valgrind: Conditional jump or move depends on uninitialised value(s)":
(https://github.com/bitcoin/bitcoin/issues/29635#issuecomment-2047861599)
Some options:

* Add a valgrind suppression
* Avoid clang17+ for now, use up to clang16 in CI
* Something else?

💬 maflcko commented on pull request "refactor: Simplify base32/64 padding calculations":
(https://github.com/bitcoin/bitcoin/pull/29847#issuecomment-2047875411)
Thank you for your contribution. While this stylistic change makes sense on its own, it comes at a cost and risk for the project as a whole. The weak motivation for the change does not justify the burden that it places on the project. A burden could be any of the following:

* Time spent on review
* Accidental introduction of bugs
* (Silent) merge conflicts, either in the branch or a backport branch. Those conflicts demand further developer and reviewer time or introduce bugs.

For more in
...

💬 paplorinc commented on pull request "refactor: Simplify base32/64 padding calculations":
(https://github.com/bitcoin/bitcoin/pull/29847#issuecomment-2047973653)
Thanks for the review @fanquake and @maflcko - though it was still in draft form because of the unsigned problems.

I didn't mean the change as a stylistic change.

In the encodings the original `str.reserve` doesn't enforce the final size, but after the change it's obvious that it's an exact calculation, not just an upper bound, i.e. `while (str.size() % 4) str += '=';` vs `str.append(size - str.size(), '=');`.

In the second change (which I've removed for now) the point was to unify the
...

💬 itornaza commented on pull request "test: Add `leaf_version` parameter to `taproot_tree_helper()`":
(https://github.com/bitcoin/bitcoin/pull/29371#issuecomment-2048105785)
> Reasonable minds can differ of course, for future readers of the PR leave a comment or give me a 👎 if you disagree and would like to see the default parameter used in this case. If it is clear that I'm in the minority, I will change it.

Who am I to say, but I philosophically agree with your reasoning on not passing default parameters in critical test code.

👋 Eunovo's pull request is ready for review: "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict"
(https://github.com/bitcoin/bitcoin/pull/29680)

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1560028640)
The error message is:
```
Traceback (most recent call last):
File "D:\a\bitcoin\bitcoin\test\fuzz\test_runner.py", line 406, in <module>
main()
File "D:\a\bitcoin\bitcoin\test\fuzz\test_runner.py", line 110, in main
test_list_all = parse_test_list(
^^^^^^^^^^^^^^^^
File "D:\a\bitcoin\bitcoin\test\fuzz\test_runner.py", line 392, in parse_test_list
test_list_all = subprocess.run(
^^^^^^^^^^^^^^^
File "C:\hostedtoolcache\windows
...

💬 hebasto commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2048425728)
Concept ACK.

💬 paplorinc commented on pull request "refactor: Simplify base32/64 padding calculations":
(https://github.com/bitcoin/bitcoin/pull/29847#issuecomment-2048485814)
@maflcko, you were right, the changes didn't add enough value, I've reverted them.
I've kept the tests, since I think they're valuable and unified the style between base 32 and 64 in the source. If you think that's still too much, I'll revert it.

💬 niftynei commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-2048493284)
Yes but won't have any progress to show til May or so. Thanks

👋 paplorinc's pull request is ready for review: "refactor/test: add a few more base32/64 calculation corner cases"
(https://github.com/bitcoin/bitcoin/pull/29847)

💬 hebasto commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2049000540)
My Guix builds:
```
x86_64
07f97e7479e5a7e1baae091ed74839e9b7be118d0504a8b334c4add8e13c78cc guix-build-a0dc2ebcda9e/output/aarch64-linux-gnu/SHA256SUMS.part
ee4ab82a42c802dfa1e455e6badb6a49e819c77f03fcaf3568656faa5f69dce4 guix-build-a0dc2ebcda9e/output/aarch64-linux-gnu/bitcoin-a0dc2ebcda9e-aarch64-linux-gnu-debug.tar.gz
f405a168ea3aa2b3a230664381babb406b03680404ebb89e1638f07efa872520 guix-build-a0dc2ebcda9e/output/aarch64-linux-gnu/bitcoin-a0dc2ebcda9e-aarch64-linux-gnu.tar.gz
98c203bb
...

💬 laanwj commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#discussion_r1560502458)
i think i'm missing something: if this is a build flag, is a patch really needed? Or is this more of a defense-in-depth approach to prevent accidentally forgetting it.

💬 laanwj commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2049044482)
Concept ACK, will test

💬 laanwj commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1560532624)
maybe "Accept was interrupted"

📝 fanquake converted_to_draft a pull request: "RPC: add new `listmempooltransactions`"
(https://github.com/bitcoin/bitcoin/pull/29016)
## Proposed Update

Add a new RPC endpoint, `listmempooltransactions`. Takes as args a `start_sequence` and `verbose`.

Returns:
- if verbose false, list of txids + their `entry_sequence` where each entry's `entry_sequence` >= the provided `start_sequence.
- if verbose true, raw tx output info including each entry's `entry_sequence`.


Builds on work done in #19572.

## Rationale
The current mempool RPCs are lacking an ability to scan for updates in a more efficient manner. You can
...

💬 maflcko commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1560540110)
I still fail to understand it. Can you explain what it means and why passing the env fixes it? Is there an error message on stderr or stdout as well?

💬 fanquake commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#discussion_r1560542032)
It's so we build the whole world / toolchain / depends etc with this option.

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1560558840)
On Windows, when `subprocess` sets environment variables, it effectively overrides all of them, which makes `PATH` and `COMSPEC` environment variable empty on the master branch.

💬 maflcko commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1560562920)
Is it not the same on Linux?

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1560567478)
Linux does not need the `COMSPEC` environment variable to spawn a new process, right?