💬 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".

💬 ajtowns commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2138444143)
> I missed that before, but the improved text on the BIP made me realize. It looks like an attacker could jack up the difficulty by mining a few difficulty periods with an ASIC and then stop after the last block in a difficulty period. The network would then be on a difficulty some 4n higher than before, and stuck looking to mine a first block at full difficulty.

I don't think that's much of a concern -- all you'd need to do is invalidateblock the last block of the period, mine a new one with
...

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1619684993)
Thanks, that was helpful, I could reproduce the assertion crash with this input. Ended up putting some debug outputs into the fuzz test to see what txs are created to trigger it, and it matches the scenario described in the commit message via two submitted packages [TxA <- TxB] and [TxB' <- TxC].

>> isn't possible due to V3 ancestor limits.
>
> It's only impossible due to PackageV3Checks, since TxC has no in-mempool ancestors at PreChecks/SingleV3Checks time.

Oh right, that's the part I
...

📝 kevkevinpal opened a pull request: "test: Added test coverage to listsinceblock rpc"
(https://github.com/bitcoin/bitcoin/pull/30195)
This change is meant to add test coverage to this rpc error https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpc/transactions.cpp#L666C53-L666C79

This is done by setting the blocks folder to have 000 permissions before `listsinceblock` is called and then reset afterwards back to 600

---

Doing a quick grep for the error code in our functional tests leads to zero results
`grep -nri "Can't read block from disk" ./test/functional/`

💬 darosior commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1620123640)
It could use the basic testing setup indeed.