💬 murchandamus commented on pull request "Fix waste calculation in SelectionResult":
(https://github.com/bitcoin/bitcoin/pull/28366#issuecomment-2137911720)
> Happy to reack, but you need to fix clang-tidy first

Fixed the issue with `clang-tidy`, sorry about that

💬 chrisguida commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1619240097)
Can we clarify this message a bit to emphasize that this rescheduling does not preclude periodic flushes to disk?

It's slightly confusing to see this message as it appears that disk flushes are being deferred indefinitely.

See: https://github.com/bitcoinknots/bitcoin/issues/70#issuecomment-2136078696

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1619252262)
It is duplicating part of that, given `CConnman::m_max_automatic_outbound` is private.

I agree it is adding more complexity, but not accounting for it in the minimum means that could be in a situation where no single outbound connection is accounted for, so the user is basically only connected to manuals.

I acknowledge that's fairly rare, but the two reasons why we are currently not facing this issue is, first, because the minimum is fairly low, so it would need a system with really limite
...

💬 murchandamus commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2137944959)
> Yes, this has also been described [here by AJ](https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2092080764).

I must have read that before it was edited and missed the edit.

> @Sjors suggestion to change `nActualTimespan` was also aimed at this problem but I have a few doubts outlined above. I think @ajtowns suggestion to use the version field is the only complete solution for this problem so far.

Thanks for keeping the overview! :)
I guess if someone were to troll testnet
...

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1619278941)
This could be a simpler alternative, that accounts for the automatic within the minimum

```diff
// Automatic outbounds are already accounted for in nUserMaxConnections, but we want to make sure they count towards the minimum later on
int auto_outbounds = std::min(MAX_OUTBOUND_FULL_RELAY_CONNECTIONS + MAX_BLOCK_RELAY_ONLY_CONNECTIONS + MAX_FEELER_CONNECTIONS, nUserMaxConnections);
- // Reserve enough FDs to account for the bare minimum, plus any manual connections, plus the bound interfa
...

💬 brunoerg commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1619292782)
done

💬 brunoerg commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#issuecomment-2137992066)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1618155192.

💬 luke-jr commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#issuecomment-2138039768)
Not sure the asmap feature cares if your map is based on BGP or something else?

💬 jb55 commented on issue "add option to bumpfee RPC to prevent it adding new inputs":
(https://github.com/bitcoin/bitcoin/issues/20935#issuecomment-2138042959)
another thing I wanted to do today with a single output is to bump the fee + add an output. from what I can tell there are simply no tools that exist that allow me to do this with a psbt.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1619345525)
> is this commit (848c4e5, "PackageV3Checks: Relax assumptions") strictly needed for this PR or already preparation for future work?

If it's not removed the next commit will cause failures.

It hits pretty in the fuzzing `package_eval` fuzzing target. I could add a unit test for it, but it doesn't really test much, except that we don't crash right before removing the soon to be erroneous assertion. Here's an input you can run to verify:
```
EQQAAAgAAAAAAAAAAAAAAAUAAAAAAAAAAAAAAAA6AAAACAA
...

💬 jb55 commented on issue "add option to bumpfee RPC to prevent it adding new inputs":
(https://github.com/bitcoin/bitcoin/issues/20935#issuecomment-2138090368)
oh! this was added:

- https://github.com/bitcoin/bitcoin/pull/25344

maybe one day this could have a UI?

💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2138132147)
Here’s the issue:

> Checking bitcoin-27.0.tar.gz
> Found RC version: version_base=bitcoin, version_rc= os_filter=27.0.tar.gz

It’s not designed to work when the version string begins with bitcoin-
What else would we be using bitcoin/bitcoin contrib/verify-binaries for?

27.0-.tar.gz would be the correct way to search for all binaries ending in this extension with this parser.

Whatever comes before the 1st dash must be the version number in <major>.<minor> or <major>.<minor>.<patch> format.

Ev
...

⚠️ jb55 opened an issue: "Generalized fee bumping"
(https://github.com/bitcoin-core/gui/issues/822)
### Please describe the feature you'd like to see added.

It would be nice if we had a GUI implementation of this new RPC feature:

- https://github.com/bitcoin/bitcoin/pull/25344

Maybe the lowest common denominator here is:

- [ ] #558

This would allows us to edit transactions pre-flight and in-flight?

The main usecase here is that let's say you have a transaction that isn't
going to confirm for a couple hours, days, weeks. Then all of a sudden
you remember that you wanted to ad
...

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1619408564)
I think I understand though; the argument to `push_back` or `push_front` should not be inferred but be fixed to be (a reference to) `T`. Done.

💬 sipa commented on pull request "Low-level cluster linearization code":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2138188901)
I've added support for merging linearizations to this PR (`MergeLinearizations()` function), plus benchmarks and tests.

💬 sipa commented on pull request "refactor: use recommended type hiding on multi_index types":
(https://github.com/bitcoin/bitcoin/pull/30194#discussion_r1619432995)
indices?

💬 theuni commented on pull request "refactor: use recommended type hiding on multi_index types":
(https://github.com/bitcoin/bitcoin/pull/30194#discussion_r1619450775)
😬

🤔 furszy reviewed a pull request: "Fix waste calculation in SelectionResult"
(https://github.com/bitcoin/bitcoin/pull/28366#pullrequestreview-2086451768)
Code ACK bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624

💬 furszy commented on pull request "Fix waste calculation in SelectionResult":
(https://github.com/bitcoin/bitcoin/pull/28366#discussion_r1619461019)
Could set the second parameter, `change_cost` to 0 instead of providing `cost_of_change`. The parameter is only used if there is change and BnB goal is to not generate one.

💬 theuni commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1619482803)
While you're at it, how about replacing these move assignments with `Reseed()` functions?

I find the idea of moving an rng to be unintuitive. And as far as I can tell, every case of

`RandomMixin = foo` and `FastRandomContext = foo` and `InsecureRandomContext = foo` (all in tests) really just means "reseed".