💬 ryanofsky commented on pull request "kernel: Add interrupt class":
(https://github.com/bitcoin/bitcoin/pull/27861#discussion_r1226848259)
In commit "kernel: Add fatalError method to notifications" (d60ab3b5099d3a348f3122b5d3207ac12f4a751c)

This is the other misleading `fatalError` call (see comment above)

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1226992419)
done

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1226992717)
done

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1226993131)
done

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1226993867)
It was need for the old approach, but not anymore I guess

💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1227001555)
thanks 👍

🤔 LarryRuane reviewed a pull request: "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full"
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1475451194)
utACK 2a6a981e4d892c3d7196ddce54471350950affc7
These latest changes look good! I'll test later in the week.

💬 LarryRuane commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1227005332)
(I didn't test that this is okay)
```suggestion
const std::optional<NodeId> node_to_evict = SelectNodeToEvict(std::move(eviction_candidates), /*force=*/fuzzed_data_provider.ConsumeBool());
```

💬 TheCharlatan commented on pull request "kernel: Add interrupt class":
(https://github.com/bitcoin/bitcoin/pull/27861#issuecomment-1587753463)
> The only thing I think really should be addressed is the two BlockManager::Flush calls aborting without returning errors.

I was about to open a separate PR from this branch here: https://github.com/TheCharlatan/bitcoin/tree/kernelReturnOnAbort . I think this should go in a separate PR for ease of review. I'll also provide a list of all the other non-immediate returns that should be fine to do.

📝 hebasto converted_to_draft a pull request: "Use `int32_t` type for most transaction size/weight values"
(https://github.com/bitcoin/bitcoin/pull/23962)
From bitcoin/bitcoin#23957 which has been incorporated into this PR:
> A file-wide suppression is problematic because it will wave through future violations, potentially bugs.
>
> Fix that by using per-statement casts.
>
> This refactor doesn't change behavior because the now explicit casts were previously done implicitly.
>
> Similar to commit 8b5a4de904b414fb3a818732cd0a2c90b91bc275

👍 stickies-v approved a pull request: "rest: bugfix, fix crash error when calling `/deploymentinfo`"
(https://github.com/bitcoin/bitcoin/pull/27853#pullrequestreview-1475484670)
ACK 7d452d826a7056411077b870efc3872bb2fa45e4

📝 mzumsande opened a pull request: "test: fix intermittent failure in p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/27864)
Fixes #27860

The problem was that the `inv` for `tx_b` would sometimes be sent out to the inbound peer after the `notfound`, so that the `notfound` wouldn't be the last message anymore and the test fails.

```
node0 2023-06-12T12:48:24.903204Z [msghand] [net.cpp:2856] [PushMessage] [net] sending notfound (73 bytes) peer=1
node0 2023-06-12T12:48:24.903423Z [msghand] [net.cpp:2856] [PushMessage] [net] sending inv (37 bytes) peer=1
```

Fix this by waiting for the `inv` before contin
...

💬 brunoerg commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1227055025)
I think you forgot this. (in case you have to touch the code again)

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1227061175)
Hm, I did do it, not sure why it got lost...

💬 Xekyo commented on pull request "fuzz: Fix mini_miner_selection running out of coin":
(https://github.com/bitcoin/bitcoin/pull/27806#discussion_r1227064407)
The point of this fuzz test is to test the calculation of bump fees. If all outputs of transactions are spent, there are no bump fees to be calculated. With the std::min(2, available_coins.size()) the (2, 5) should also not crash, though. I’ll test that and return it to that if my understanding is substantiated.

📝 mzumsande converted_to_draft a pull request: "test: fix intermittent failure in p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/27864)
Fixes #27860

The problem was that the `inv` for `tx_b` would sometimes be sent out to the inbound peer after the `notfound`, so that the `notfound` wouldn't be the last message anymore and the test fails.

```
node0 2023-06-12T12:48:24.903204Z [msghand] [net.cpp:2856] [PushMessage] [net] sending notfound (73 bytes) peer=1
node0 2023-06-12T12:48:24.903423Z [msghand] [net.cpp:2856] [PushMessage] [net] sending inv (37 bytes) peer=1
```

Fix this by waiting for the `inv` before contin
...

💬 ryanofsky commented on pull request "wallet: Migrate wallets that are not in a wallet dir":
(https://github.com/bitcoin/bitcoin/pull/26740#discussion_r1227076942)
Sorry, I overlooked that in my suggestion. But I think this function can do the same thing as [`MakeWalletDatabase`](https://github.com/bitcoin/bitcoin/blob/c92fd638860c5b4477851fb3790bc8068f808d3e/src/wallet/wallet.cpp#L2892), [`VerifyWallets`](https://github.com/bitcoin/bitcoin/blob/c92fd638860c5b4477851fb3790bc8068f808d3e/src/wallet/load.cpp#L79), [`RestoreWallet`](https://github.com/bitcoin/bitcoin/blob/c92fd638860c5b4477851fb3790bc8068f808d3e/src/wallet/wallet.cpp#L446), [`MigrateLegacyToDe
...

💬 Xekyo commented on pull request "fuzz: Fix mini_miner_selection running out of coin":
(https://github.com/bitcoin/bitcoin/pull/27806#discussion_r1227081979)
Thanks, done.

💬 ryanofsky commented on pull request "kernel: Add interrupt class":
(https://github.com/bitcoin/bitcoin/pull/27861#issuecomment-1587870978)
> > The only thing I think really should be addressed is the two BlockManager::Flush calls aborting without returning errors.
>
> I was about to open a separate PR from this branch here: https://github.com/TheCharlatan/bitcoin/tree/kernelReturnOnAbort

Great! I also opened #27862 early which overlaps with the branch a little. I think between the branch and PR all the cases where errors are not currently returned are covered.

💬 Xekyo commented on pull request "fuzz: Fix mini_miner_selection running out of coin":
(https://github.com/bitcoin/bitcoin/pull/27806#discussion_r1227089752)
I’ve reset it to (2, 5) after fuzzing ~100k execs and the problematic seed. I’m not sure (1, 5) would be an improvement in this particular fuzz seed, but happy to change it if people are convinced otherwise.