💬 remyers commented on pull request "wallet: add coin selection parameter `add_excess_to_recipient_position` for changeless txs with excess that would be added to fees":
(https://github.com/bitcoin/bitcoin/pull/30080#issuecomment-2456675338)
> I think the obvious way to implement this feature is to just detect when the fee paid is greater than the fee needed, and send the excess to the recipient specified, just as we already do when there are change outputs.

Thanks for the feedback. I agree and think for our use we can even do that diff outside of bitcoind. I'll update the PR.

💬 willcl-ark commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2456696504)
> I can test this out on a Ryzen CPU with ZFS on SSD (but with only dual channel DDR4).
>
> * Is there anything else about the config that you can provide so I can try to reproduce?
>
> * What is your ZFS pool setup like?
>
> * Is your SSD also SATA-connected like OP?
>
> * Are you running any Bitcoin Core options that may be relevant?

OK I could not reproduce this on either a native ZFS zpool nor an "ext4 on ZFS" zpool (configurations found here: https://github.co
...

💬 vasild commented on issue "Setting torcontrol overrides proxy address":
(https://github.com/bitcoin/bitcoin/issues/25265#issuecomment-2456720675)
> According to my understanding bitcoin core designed automatically to configure tor when using `torcontrol,`

It automatically configures the location of the Tor proxy, used for outgoing connections to Tor. Don't confuse this with the creation of the Tor hidden service for accepting incoming connections from Tor, that is controlled by `-listenonion=0|1`.

@kroese, it is a bit unclear from the OP, did it connect to torcontrol and retrieve an invalid proxy from there? That is, `127.0.0.1:9050
...

💬 kroese commented on issue "Setting torcontrol overrides proxy address":
(https://github.com/bitcoin/bitcoin/issues/25265#issuecomment-2456762045)
@vasild I cannot remember anymore, it was two years ago that I ran into this behaviour. So feel free to close it.

💬 l0rinc commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2456773318)
Thanks for checking it out @achow101.
I could make the diff minimal, but given that it's not that important, I will leave it as is - until someone starts complaining that `listunspent` is slow :)

👍 vasild approved a pull request: "test: create assert_not_equal util"
(https://github.com/bitcoin/bitcoin/pull/29500#pullrequestreview-2415263506)
Almost ACK 1d722660a6, modulo squash of the two commits.

This is a nice addition.

It is better to squash the two commits into one because IMO it does not make sense to first add the import in one commit and in another commit to use it. That way it becomes difficult to review whether an unnecessary import was added to some file. Also some python linter that tests each commit could be upset about the unused imports in the first commit.

I found it easier to read the diff itself instead of
...

💬 hebasto commented on pull request "Update secp256k1 subtree to v0.6.0":
(https://github.com/bitcoin/bitcoin/pull/31216#issuecomment-2456856188)
> v0.6.0 was just released, main change is that it has the musig module which #29675 needs.

Should we disable the musig module in this PR and enable it only when it is needed?

💬 vasild commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2456862790)
Concept ACK

> A use case that was mentioned to me offline is that some people, before opening a PR to the main repo, like to push their branch and have CI check it.

FWIW I do that but I have a dedicated dummy branch which is associated with a PR to merge it into my copy of `master`. So I never merge that PR and keep `push -f`ing into the dummy branch to test.

✅ ryanofsky closed an issue: "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously"
(https://github.com/bitcoin/bitcoin/issues/31178)

🚀 ryanofsky merged a pull request: "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz"
(https://github.com/bitcoin/bitcoin/pull/31191)

💬 maflcko commented on issue "intermittent issue in wallet_upgradewallet.py: AssertionError: bdb magic does not match bdb btree magic":
(https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2456883152)
Not sure how to fix this. IIUC neither podman (https://github.com/containers/podman/issues/11415#issuecomment-912015581), nor docker (https://docs.docker.com/engine/security/rootless/#known-limitations) support rootless zfs.

💬 ryanofsky commented on pull request "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz":
(https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2456883737)
Went ahead and merged this since it has enough ACKs, and it sounds like the concerns I have about drawbacks of this PR don't seem to be shared by the other reviewers. I think it would be nice if fuzzing could be turned on without turning every else off, and this approach will probably make me a little less likely to start writing fuzz tests, and prefer writing unit tests instead for convenience. But status after this PR isn't worse than the status before #31093, and this approach can always be r
...

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1829166070)
yes, explaining it more would make sense!

but i think it's out of scope for this PR

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1829170503)
right-it can be used for nested structures, but the call site has to do their own iteration and add it up, as this only covers the outermost layer

adding the `std::is_trivial_v` check is an interesting idea but would prevent it from being used there.

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1829176216)
i don't think there's anything to do here; the code is agnostic to the size and implementation of the sso area; the comment gives an example for one C++ library, to illustrate the point
adding more specifics adds more information that could go stale 😄

💬 fanquake commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#discussion_r1829177093)
Sure, dropped.

💬 vasild commented on issue "Distribute darknet node addresses via DNS seeds using AAAA records":
(https://github.com/bitcoin/bitcoin/issues/31062#issuecomment-2456901896)
I agree with the reasoning above. I am still Concept ACK if we can have the clients get all addresses from the response, without some addresses being omitted. I see no downsides if separate DNS seeds are used for the new encoding, leaving the existent ones as they are currently.

Just to clarify - Tor only nodes do not use DNS seeds and the intention here is to keep it that way. The motivation is to help multi-homed nodes that use clearnet and Tor. They are currently only getting clearnet addr
...

💬 fanquake commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2456905576)
@davidgumberg @hodlinator Thanks for taking a look. I've adjusted the subsystem change, and made some other small updates. I think we can do this for now, and follow up further with #30210.

🤔 hebasto reviewed a pull request: "build: increase minimum supported Windows to 10.0"
(https://github.com/bitcoin/bitcoin/pull/31172#pullrequestreview-2415383222)
Appproach ACK 0440c406eedbc16fa1ce6c77a802b7ea60a79057.

💬 maflcko commented on pull request "ci: Split out native fuzz jobs for macOS and windows":
(https://github.com/bitcoin/bitcoin/pull/31073#issuecomment-2456949207)
This may be re-considered after a rebase. However, I wonder if the bloat can be reduced, assuming GHA has a matrix feature, so that the non-fuzz and fuzz-build can share most of the GHA config and steps?