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

💬 mzumsande commented on issue "rpc_getblockfrompeer.py intermittent failure: assert_equal(pruneheight, 248); not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1568894113)
Opened #27784 which just adds an additional sync, as suggested in https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567870784 .

💬 brunoerg commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1210698732)
In 30c39c1316a7b5c1914654f2a1487309b6e550ac: If we're in IBD, wouldn't we put that peer in the end of the queue with no need (I mean, without doing a rec request in fact)?

💬 Crypt-iQ commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1568965050)
@erikarvstedt it seems like there's a call to `importmulti` that is outstanding while bitcoind is trying to shut down

💬 aureleoules commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#discussion_r1210750673)
Done thanks. I rolled back to jammy.