🚀 achow101 merged a pull request: "util: improve FindByte() performance"
(https://github.com/bitcoin/bitcoin/pull/19690)
(https://github.com/bitcoin/bitcoin/pull/19690)
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190437947)
Re-added
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190437947)
Re-added
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190437970)
Yeah. Added some docs to the class (also mentioned BIP331) so maybe it's more clear? I used to call it `Packageifier` because it can potentially build a package out of any random list of transactions. But then it's weird because we packageify a `Package`. Open to naming improvements 😅
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190437970)
Yeah. Added some docs to the class (also mentioned BIP331) so maybe it's more clear? I used to call it `Packageifier` because it can potentially build a package out of any random list of transactions. But then it's weird because we packageify a `Package`. Open to naming improvements 😅
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438085)
Elaborated on the comment, hopefully it's better now
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438085)
Elaborated on the comment, hopefully it's better now
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438226)
> I think hashing these bytes would defeat this since it can't generate a valid tx with that txid?
Did this one, thanks 👍
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438226)
> I think hashing these bytes would defeat this since it can't generate a valid tx with that txid?
Did this one, thanks 👍
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438295)
Elaborated
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438295)
Elaborated
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438618)
Changed to always use `package[i]`, and added a comment about this unstable sort
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438618)
Changed to always use `package[i]`, and added a comment about this unstable sort
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438675)
Aha, thanks!
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438675)
Aha, thanks!
📝 ismaelsadeeq opened a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622)
Fixes #27555
The issue arises during initialization where an old `fee_estimates.dat` file is sometimes read during initialization.
Or after an unclean shutdown, the latest fee estimates are not flushed to `fee_estimates.dat`.
If the fee estimates in the old file are old, they can cause transactions to become stuck in the mempool.
This PR ensures that nodes do not use stale estimates from the old file during initialization. If `fee_estimates.dat`
has not been updated for 12 hours or mo
...
(https://github.com/bitcoin/bitcoin/pull/27622)
Fixes #27555
The issue arises during initialization where an old `fee_estimates.dat` file is sometimes read during initialization.
Or after an unclean shutdown, the latest fee estimates are not flushed to `fee_estimates.dat`.
If the fee estimates in the old file are old, they can cause transactions to become stuck in the mempool.
This PR ensures that nodes do not use stale estimates from the old file during initialization. If `fee_estimates.dat`
has not been updated for 12 hours or mo
...
💬 theuni commented on pull request "build: LLVM 16 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1542911418)
I've experimented and implemented with the above here: https://github.com/theuni/bitcoin/commits/21778
@fanquake I think we want https://github.com/theuni/bitcoin/commit/a0427fc986294ef81605b4c44b54c3ec9825bf48 regardless, which fixes the libbitcoinconsensus linking issue. tl;dr: It makes lld linking work like ld64 by skipping a busted test. The `-r` was a red herring as a result of that busted test.
Shall I go ahead and PR that commit separately?
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1542911418)
I've experimented and implemented with the above here: https://github.com/theuni/bitcoin/commits/21778
@fanquake I think we want https://github.com/theuni/bitcoin/commit/a0427fc986294ef81605b4c44b54c3ec9825bf48 regardless, which fixes the libbitcoinconsensus linking issue. tl;dr: It makes lld linking work like ld64 by skipping a busted test. The `-r` was a red herring as a result of that busted test.
Shall I go ahead and PR that commit separately?
💬 ajtowns commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1190497125)
`MAX_PEER_TX_ANNOUNCEMENTS` doesn't really provide a rate limit -- it stops the queue from being more than 5k at any point in time, but the flow through the queue could still be 400tx/s, for instance. Beyond that, `m_recently_announced_invs` only tracks things we announced, not things we received, and we rate limit that (as per the static_assert on `INVENTORY_MAX_RECENT_RELAY`).
Delaying MEMPOOL results for 2 minutes seems annoying. But how about just recording the timestamp of the last MEMP
...
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1190497125)
`MAX_PEER_TX_ANNOUNCEMENTS` doesn't really provide a rate limit -- it stops the queue from being more than 5k at any point in time, but the flow through the queue could still be 400tx/s, for instance. Beyond that, `m_recently_announced_invs` only tracks things we announced, not things we received, and we rate limit that (as per the static_assert on `INVENTORY_MAX_RECENT_RELAY`).
Delaying MEMPOOL results for 2 minutes seems annoying. But how about just recording the timestamp of the last MEMP
...
💬 ajtowns commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1190528963)
The `3500` figure is calculated based on outbound peers, which already have 2.5x the INV rate of inbound peers. An inbound peer would need to be at 65 or 70 txids per INV message to have the same 1-in-a-million chance of a problem, implying a tx rate of 13 or 14 tx/s. To get to a 1-in-100 chance of a problem, you'd need to hit 90-to-95 txids per INV (and sustain that for ~40 INVs, and hit the 1-in-100 chance of sending those 40 INVs in a 2 minute period). Those seem okay to me.
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1190528963)
The `3500` figure is calculated based on outbound peers, which already have 2.5x the INV rate of inbound peers. An inbound peer would need to be at 65 or 70 txids per INV message to have the same 1-in-a-million chance of a problem, implying a tx rate of 13 or 14 tx/s. To get to a 1-in-100 chance of a problem, you'd need to hit 90-to-95 txids per INV (and sustain that for ~40 INVs, and hit the 1-in-100 chance of sending those 40 INVs in a 2 minute period). Those seem okay to me.
💬 ajtowns commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1190533182)
Just seems like more code to do it that way, since that is just dealing with txids and `CompareDepthAndScore` already has access to the mempool info.
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1190533182)
Just seems like more code to do it that way, since that is just dealing with txids and `CompareDepthAndScore` already has access to the mempool info.
💬 ajtowns commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1190574243)
It's more related to `MAX_PEER_TX_ANNOUNCEMENTS=5000` (we don't want to send more than we would be willing to receive) and `INVENTORY_MAX_RECENT_RELAY=3500` (we don't want to advertise txids and then refuse to relay them because we've forgotten that we advertised them).
(Could make sense to limit it to 90 for inbounds and 45 for outbounds as (if sustained for two minutes) those values would hit roughly a 1-in-100 chance of overflowing `INVENTORY_MAX_RECENT_RELAY`)
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1190574243)
It's more related to `MAX_PEER_TX_ANNOUNCEMENTS=5000` (we don't want to send more than we would be willing to receive) and `INVENTORY_MAX_RECENT_RELAY=3500` (we don't want to advertise txids and then refuse to relay them because we've forgotten that we advertised them).
(Could make sense to limit it to 90 for inbounds and 45 for outbounds as (if sustained for two minutes) those values would hit roughly a 1-in-100 chance of overflowing `INVENTORY_MAX_RECENT_RELAY`)
💬 MarcoFalke commented on issue "make check errors on big endian OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-1543500303)
> Everything passed when running ./test/util/test_runner.py --verbose .
That means the issue originally reported is fixed and `make check` fully passes?
> ./test/functional/feature_addrman.py --tracerpc
Can you also try with `./test/functional/feature_addrman.py --tracerpc -l DEBUG` and then `./test/functional/combine_logs.py ./folder-tmp-test/foo/bar`?
(https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-1543500303)
> Everything passed when running ./test/util/test_runner.py --verbose .
That means the issue originally reported is fixed and `make check` fully passes?
> ./test/functional/feature_addrman.py --tracerpc
Can you also try with `./test/functional/feature_addrman.py --tracerpc -l DEBUG` and then `./test/functional/combine_logs.py ./folder-tmp-test/foo/bar`?
⚠️ huzhenyuan opened an 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)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
We are the technical team from AntPool. We found that when the Bitcoin wallet node processes the process message recently, the single-threaded CPU 100% causes block parsing delays, resulting in serious technical failures.
Our version number is the latest version 24.0.1。
Not only antpool, we also asked other mining pools and found this problem. This will lead to increasingly serious orpha
...
(https://github.com/bitcoin/bitcoin/issues/27623)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
We are the technical team from AntPool. We found that when the Bitcoin wallet node processes the process message recently, the single-threaded CPU 100% causes block parsing delays, resulting in serious technical failures.
Our version number is the latest version 24.0.1。
Not only antpool, we also asked other mining pools and found this problem. This will lead to increasingly serious orpha
...
💬 darosior commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1543552214)
I'm only a casual lurker on the net_processing side but i was under the impression we were thinking of the BIP35 as permissioned anyways. If we are already trusting the peer that sends use MEMPOOL message, why not just include everything we've got?
A practical counter-argument to this could be - yeah but at this time [around 20% to 30% of reachable nodes](https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529678174) set `peerbloomfilters` so it's still worth closing an obvious fingerpr
...
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1543552214)
I'm only a casual lurker on the net_processing side but i was under the impression we were thinking of the BIP35 as permissioned anyways. If we are already trusting the peer that sends use MEMPOOL message, why not just include everything we've got?
A practical counter-argument to this could be - yeah but at this time [around 20% to 30% of reachable nodes](https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529678174) set `peerbloomfilters` so it's still worth closing an obvious fingerpr
...
💬 willcl-ark 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-1543579414)
Hello @huzhenyuan, thank you for the report.
There is a possible fix for similar symptoms currently open in https://github.com/bitcoin/bitcoin/pull/27610, but note that this is to alleviate high CPU usage originating from `SendMessages()`, not `ProcessMessages()` as you describe you have found in your case. However, both of these functions are run in the same thread `b-msghand` so they could be related or difficult to discern from one another.
If you are able and willing, you could try che
...
(https://github.com/bitcoin/bitcoin/issues/27623#issuecomment-1543579414)
Hello @huzhenyuan, thank you for the report.
There is a possible fix for similar symptoms currently open in https://github.com/bitcoin/bitcoin/pull/27610, but note that this is to alleviate high CPU usage originating from `SendMessages()`, not `ProcessMessages()` as you describe you have found in your case. However, both of these functions are run in the same thread `b-msghand` so they could be related or difficult to discern from one another.
If you are able and willing, you could try che
...
💬 FelixWeis 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-1543649256)
thanks @willcl-ark, can you edit you comment and point to the relevant mitigation PR/issues.
(https://github.com/bitcoin/bitcoin/issues/27623#issuecomment-1543649256)
thanks @willcl-ark, can you edit you comment and point to the relevant mitigation PR/issues.
💬 fanquake commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#issuecomment-1543653619)
cc @instagibbs @TheBlueMatt
(https://github.com/bitcoin/bitcoin/pull/27622#issuecomment-1543653619)
cc @instagibbs @TheBlueMatt