💬 MarcoFalke commented on pull request "[PoC] Structure aware fuzzing with libprotobuf-mutator":
(https://github.com/bitcoin/bitcoin/pull/26975#issuecomment-1568737329)
Did you find that this was useless in a benchmark, or did you just close for fun :smiling_face_with_tear: ?

💬 theuni commented on issue "Adding a minimally-patched Guix repo to the org":
(https://github.com/bitcoin/bitcoin/issues/25098#issuecomment-1568737780)
> > even if accepted that means a (potentially lengthy) delay
>
> Have you tried it? If this goes into upstream within a week or two and all what is left for us to bump the time machine, this doesn't seem like a reason to create the fork, only to remove it again in two weeks.

@fanquake mentioned that we're currently blocked from bumping our time machine for an unrelated reason, so I figured the fork would be necessary either way.

But no, I haven't yet submitted the patch upstream. I'm g
...

👋 TheCharlatan's pull request is ready for review: "kernel: Remove args, chainparams, chainparamsbase from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27576)

💬 mzumsande commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1568771908)
> np for me. @mzumsande do you want to go ahead with your fix? I can just relabel this PR to "Implement getblockfileinfo RPC command" and done.

yes, I'll open a PR with the one-line fix soon!

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1210571545)
> What's the reason for leaving certain mappings out of this list, i.e `#include <boost/operators.hpp>`?

To keep this PR in reviewable state. For example, a bunch of public Boost headers must be included explicitly and add to a linter:
- `boost/multi_index/identity.hpp`
- `boost/multi_index/indexed_by.hpp`
- `boost/multi_index/tag.hpp`
- `boost/tuple/tuple.hpp`

> I'm guessing it's because those same includes are recommended for non-test code, and mapping them to Boost Test here would j
...

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1568778803)
Addressed @fanquake's https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1210341923.

💬 brunoerg commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1210580514)
If the purpose is maturing them, I think we could do:
```suggestion
self.generate(self.nodes[0], COINBASE_MATURITY) # mature coinbase UTXO used later
```

📝 hebasto opened a pull request: "Add public Boost headers explicitly"
(https://github.com/bitcoin/bitcoin/pull/27783)
To check symbols in the code base, run:
```
git grep boost::multi_index::identity
git grep boost::multi_index::indexed_by
git grep boost::multi_index::tag
git grep boost::make_tuple
```

Hoping on the absence of conflicts with top-prio PRs :)

💬 MarcoFalke commented on pull request "Add public Boost headers explicitly":
(https://github.com/bitcoin/bitcoin/pull/27783#discussion_r1210593227)
Why are they in the header when they can be in the cpp file?

💬 MarcoFalke commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1568815816)
Looks like the fuzz test needs an update?

```
82bb7831fa6052620998c7eef47e48ed594248a8 is the first bad commit
```

```
echo 'AQAAAAEAAAA='|base64 --decode > /tmp/a
FUZZ=wallet_notifications ./src/test/fuzz/fuzz /tmp/a
```

```
fuzz: wallet/test/fuzz/notifications.cpp:175: void wallet::(anonymous namespace)::wallet_notifications_fuzz_target(FuzzBufferType): Assertion `total_amount == bal_a + bal_b' failed.
==301525== ERROR: libFuzzer: deadly signal

🤔 stickies-v reviewed a pull request: "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN"
(https://github.com/bitcoin/bitcoin/pull/27777#pullrequestreview-1451472783)
ACK fa9c65a74cf18e9c75cd3472112d5197532ac2f2

I've not used podman and don't fully understand the need for fa123077bc3f39aa0969d883e2d799a054cd4543, so I'm hoping that other people do and can comment on that so I don't need to dive into it. Otherwise, happy to revisit this later.

💬 stickies-v commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210602495)
Okay, I can't comment on that. Since there is no mention of `podman` anywhere else in the codebase/documentation, I think this may make maintaining the CI more opaque, but perhaps that's an okay tradeoff.

💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210608472)
Yeah, I don't expect anyone to use `RESTART_CI_DOCKER_BEFORE_RUN` beside the persistent workers, so it is an undocumented setting. Anyone using it is expected to infer the documentation from the two lines of code.

💬 hebasto commented on pull request "Add public Boost headers explicitly":
(https://github.com/bitcoin/bitcoin/pull/27783#discussion_r1210610907)
```
$ git grep -l boost::multi_index::tag
src/node/miner.h
src/txmempool.h
src/txrequest.cpp
```

The symbol is used in the header. IWYU agree :)

💬 MarcoFalke commented on pull request "Add public Boost headers explicitly":
(https://github.com/bitcoin/bitcoin/pull/27783#discussion_r1210612717)
what is the point in adding it and using it over just `assert` or `Assert` or `BOOST_REQUIRE`?

💬 hebasto commented on pull request "Add public Boost headers explicitly":
(https://github.com/bitcoin/bitcoin/pull/27783#discussion_r1210613769)
I see no point at all. Is it OK to put this change into this PR ?

💬 hebasto commented on pull request "Add public Boost headers explicitly":
(https://github.com/bitcoin/bitcoin/pull/27783#discussion_r1210614349)
Or I can just revert the recent change?

💬 MarcoFalke commented on pull request "Add public Boost headers explicitly":
(https://github.com/bitcoin/bitcoin/pull/27783#discussion_r1210615011)
I meant moving everything to the cpp file, but they are included via txmempool.h either way, so while everything can be moved to the cpp file, it doesn't matter.

💬 erikarvstedt commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1568888056)
A similar bug appears in the [nix-bitcoin](https://github.com/fort-nix/nix-bitcoin/) [test suite](https://github.com/fort-nix/nix-bitcoin/blob/master/test/tests.py).

The test is executed inside a QEMU VM and runs `bitcoind` together with a bunch of client services (like `clightning`, `lnd`, `joinmarket`). It then stops all services, including `bitcoind`.
Sometimes `bitcoind` hangs during shutdown:
1. First, it waits for a few minutes at [httpserver.cpp#L469](https://github.com/bitcoin/bitco
...

📝 mzumsande opened a pull request: "test: fix intermittent error in getblockfrompeer.py"
(https://github.com/bitcoin/bitcoin/pull/27784)
This adds an additional `sync_blocks` call, fixing an intermittent error caused by blocks arriving out of order due to how compact block relay may revert to headers processing when the tip hasn't caught up, and resulting in slightly different pruning behavior.
Making sure that all blocks from the previous tests are synced before generating more blocks makes this impossible.

See https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566354933 and https://github.com/bitcoin/bitcoin/iss
...