💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841223757)
In that case, no transaction would be moved from the reconciliation set to `invs_to_send`, given no transaction would be removed:

```cpp
auto it = std::find(fanout_with_ancestors.begin(), fanout_with_ancestors.end(), peer_id);
if (it != fanout_with_ancestors.end()) {
for (const auto wtxid: parents) {
m_txreconciliation->TryRemovingFromSet(peer_id, wtxid);
invs_to_send.push_back(wtxid);
}
} else {
// If the peer is registered for set reconciliation, maybe pi
...

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841243963)
Done

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841245788)
Done

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841245945)
Done

👍 hodlinator approved a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2434652082)
re-ACK fe39acf88ff552bfc4a502c99774375b91824bb1

`git range-diff master ecc5cb9 fe39acf`

- Added more `FailFmtWithError`-tests ([maflcko](https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2423906139)).
- Terser skipping of `%%` ([l0rinc](https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835770569)).
- Comment regarding format string components updated ([me](https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834472950)).

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1841318869)
Thanks for clarifying when modified fees are ignored.

> We're assuming node operators/miners care about dust. If they do not, they can already set their dust relay rate to 0 and avoid all of these mechanisms entirely, ephemeral dust becomes a no-op. (e.g., MARA reportedly doesn't enforce dust limits and runs an accelerator already)
>
> If we end up being wrong and miners are ok putting one dust output in the utxo set but not multiple or whatever, we can revisit?

My assumption would be th
...

💬 achow101 commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2475050409)
ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048

🤔 hodlinator reviewed a pull request: "descriptor: Add proper Clone function to miniscript::Node"
(https://github.com/bitcoin/bitcoin/pull/30866#pullrequestreview-2434704657)
I've experimented with a change on top of this PR that removes `shared_ptr` usage from *src/script/* altogether in order to clarify ownership.

My change is in 80fca845b5f28677207a8fea4a173baaef23036f. It helped uncover another place where the `node` needs to be cloned (at the bottom of the outer loop):

```diff
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index b099fdfc3f..f0294df72a 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -2154,7
...

🚀 achow101 merged a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000)

💬 achow101 commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2475109762)
> My change is in [80fca84](https://github.com/bitcoin/bitcoin/commit/80fca845b5f28677207a8fea4a173baaef23036f). It helped uncover another place where the `node` needs to be cloned (at the bottom of the outer loop):

Good catch, added that.

Do you have a particular test case that triggered there?

👍 ajtowns approved a pull request: "validation: Remove RECENT_CONSENSUS_CHANGE validation result"
(https://github.com/bitcoin/bitcoin/pull/31269#pullrequestreview-2434890591)
ACK e80e4c6ff91e27d7d40f099a2d7942c29085234c

💬 andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1841534621)
Good suggestion. Thanks!

💬 andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1841572082)
The most common use case is running the script without setting BUILDDIR, in which case the default builddir, `/build`, is used. However, if no binaries are found in `/build` we remind the user to set BUILDDIR to specify the correct path.

💬 kristapsk commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2475531811)
Rebased and squashed commits.

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841676251)
Say there is an in-mempool parent announced earlier, so it's not in any of the sets.
`SortPeersByFewestParents` will return the same order as `m_states` (since all `parent_count` are 0),
and `fanout_with_ancestors` will take first two after resize.

Then for two peers (specifically, the first two from `m_states`) this code is executed:
```
for (const auto wtxid: parents) {
m_txreconciliation->TryRemovingFromSet(peer_id, wtxid);
invs
...

💬 0xB10C commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2475583549)
fwiw: https://x.com/rodarmor/status/1856879301578428826

> I did a little hacking on Bitcoin Core and wrote a new RPC command to export JSON describing the entirety of the RPC interface.
> If you're writing a RPC client for Core, using this with a code generator is definitely the way.
> https://github.com/casey/bitcoin/blob/rpc-json/api.json

cc @casey

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841679464)
ee46e21a5c913175415a192661b406b328209800

First I thought why don't `fanout=true` here and handle it below. We can't do it because txid/wtxid difference (although we could have a `txid_or_wtxid` variable above....). But then, you certainly want to at least set `fanout=false` here, otherwise it remains true and added to `invs_to_send` twice (it's not critical but a bug still).

💬 maflcko commented on issue "Unit test failures when using multiple jobs and RANDOM_CTX_SEED":
(https://github.com/bitcoin/bitcoin/issues/30696#issuecomment-2475616303)
This was fixed in ad9c2cceda9cd893c0f754e49f7fca6e417ee95f by using the test name as seed.

At least I can no longer reproduce, even when pinning the time to a constant:


```diff
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 46a6daa88c..185adf3457 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -138,7 +138,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)

const std::stri
...

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841697846)
Probably a taste thing. I think `auto` should be used only when it's obvious what type it is.

Here there are two non-obvious occasions:
- `auto x = bool + bool` resulting in x=2.
- casting 2 to bool for the result.

I won't insist but i think verbose types are better here. Feel free to close.

💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1841700796)
Have you left a comment on my branch? I can't see it for some reason.