💬 sdaftuar commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1197968591)
The `NodeClock::now()` time can go backwards, right? If so I think that could pose some issues if we're primarily relying on system time in order to determine whether transactions should be available for relay. Can we use a steady clock instead?

💬 sdaftuar commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1553249207)
Concept ACK on getting rid of the recently announced filter. I think we'd need to use a steady clock to avoid random failures for no good reason, is that easy to do?

💬 fanquake commented on issue "One core in CPU usage rate remains at 100% for a long time, causing serious delays in new blocks and forks":
(https://github.com/bitcoin/bitcoin/issues/27623#issuecomment-1553254260)
@huzhenyuan v24.1 is now available: https://github.com/bitcoin/bitcoin/releases/tag/v24.1.

👍 ryanofsky approved a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1432884414)
Code review ACK 861582b4087f1602e7115ee4a70ef7a7263eb36b. Just suggested changes since last review. (Thanks!)

💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1197937840)
In commit "refactor: Split util::insert into its own file" (d492a2e5c88257cb14c417615dba525ce2cd9cfc)

Does this file need to `#include <set>`. Just wondering if IWYU would complain.

💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1197984670)
In commit "kernel: Add fatalError method to notifications" (861582b4087f1602e7115ee4a70ef7a7263eb36b)

I think it would make sense to get rid of this callback now that there is a notifications object. Following seems to work:

```diff
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 41056272f549..3a055c8e0bc7 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -200,10 +200,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const C
...

💬 instagibbs commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1553274722)
At first glance, this doesn't seem to interfere in any meaningful way with #27626

I promise to review this as a follow-up cleanup :pray:

💬 fanquake commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1553277212)
> I promise to review this as a follow-up cleanup 🙏

Ok. This could be drafted for now / rebased on top of #27626?

👍 pinheadmz approved a pull request: "test: fix intermittent issue in `feature_bip68_sequence`"
(https://github.com/bitcoin/bitcoin/pull/27177#pullrequestreview-1432988025)
ACK ee018b25af9477050839826c27a885e7211eed14

Great work on this, nice and clean now ;-)

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK ee018b25af9477050839826c27a885e7211eed14
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmRmTNQACgkQ5+KYS2KJ
yTqJsBAAqV8ZWwsCyWxK8THFr3lc5gQWcgpERH4DHAXWFpMn1WGMBLn0aXs6VTMh
V5qXjM76piqQsQcJyLEli5r4zgW0hDF7YdNtLTwPNPEnMYqvI6uxj1mUMochZEXn
dgk9vhhQzRV4s/sixPNolV6Pt6
...

💬 pinheadmz commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1198003218)
This line surprised me (test fails without it) then I saw [achows comment](https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1164465825) so I guess it does make sense to require the miniwallet's node to be running any time we need to send a tx with it. I wondered if maybe you could use `rescan_utxos` with any already-running node to update utxo confirmation count, but I guess this is probably the cleanest way

💬 pinheadmz commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1198005686)
micro-nit: its called `block_height` in the other function. no need to fix unless you have to retouch

📝 fanquake opened a pull request: "random: drop syscall wrapper usage for getrandom()"
(https://github.com/bitcoin/bitcoin/pull/27699)
This requires a linux kernel of `3.17`+, which seems entirely
reasonable. `3.17` went EOL in 2015, and the last supported `3.x` kernel
(`3.16`) went EOL > 4 years ago, in 2020. For reference, the current
oldest maintained kernel is `4.14` (released 2017, going EOL Jan 2024).

Support for `getrandom()` (and `getentropy()`) was added to
glibc `2.25` https://sourceware.org/legacy-ml/libc-alpha/2017-02/msg00079.html:
> * The getentropy and getrandom functions, and the <sys/random.h> header

...

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198025552)
Seems wrong not to include it, will do so.

💬 brunoerg commented on pull request "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`":
(https://github.com/bitcoin/bitcoin/pull/27678#issuecomment-1553339959)
@dergoegge I just removed `perfer_evict` from `ConsumeNode`, kept `recv_flood_size` and added target for `MarkReceivedMsgsForProcessing`.

`MarkReceivedMsgsForProcessing` uses `m_recv_flood_size` and plays with `vRecvMsg` which may be filled by `ReceiveMsgBytes`.

If it makes sense for a PR scope, I can update the description and we can go ahead this way here. Otherwise, it's ok for me to close it for now and add `recv_flood_size` and `prefer_evict` alongside with more appropriate net target
...

👍 instagibbs approved a pull request: "test: Add test to check tx in the last block can be downloaded"
(https://github.com/bitcoin/bitcoin/pull/27695#pullrequestreview-1433101407)
ACK fa4c16b186a34f4172deda617166813c8cb92c59

broke test intentionally by disabling non-mempool checks in `PeerManagerImpl::FindTxForGetData`, resulted in expected failure

💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553369947)
CI failure is not related.

💬 0xB10C commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1553373102)
> I've been running this branch for 7 days on a VPS and noticed this morning the CPU on b-msghand is back up to 100%. It was about that high running v24 release but dropped to 30% or so when I first switched to this branch and restarted.

@pinheadmz I've been running the patch on multiple nodes for a week now and haven't seen 100% CPU usage in the b-msghand thread again. If you haven't restarted or if it happens again, it would be helpful to see which functions are slow. `perf top -p $(pidof b
...

💬 DanM3rcurius commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1553373128)
Thanks for taking my feedback into consideration.
Have a wonderful weekend ahead!

Sent from Proton Mail for iOS

On Thu, May 18, 2023 at 12:58, MacrabFalke ***@***.***> wrote:

>> Also I didn't quite understand the note and how to specify the absolute path ...
>
> Thanks, I agree that the section about absolute folders is confusing. So I removed it in [#27685](https://github.com/bitcoin/bitcoin/pull/27685)
>
> —
> Reply to this email directly, [view it on GitHub](https://github.com/bitcoin/bitc
...

💬 MarcoFalke commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553376251)
See https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1527772394

💬 MarcoFalke commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1553377467)
Needs rebase