💬 ArmchairCryptologist commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537534014)
> Just to confirm, yours was in debug mode as well? Otherwise it'd be concerning.
My nodes are NOT in debug mode, no. I run four public nodes (24.0.1) with different server hosts, and all four of them were spinning a CPU core at 100% and experiencing slowness with block/transaction handling when I checked them yesterday, two to the point where they started occasionally dropping connectivity to my private nodes. Like I mentioned, one 24.0.1 node which does not accept incoming connections (but
...
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537534014)
> Just to confirm, yours was in debug mode as well? Otherwise it'd be concerning.
My nodes are NOT in debug mode, no. I run four public nodes (24.0.1) with different server hosts, and all four of them were spinning a CPU core at 100% and experiencing slowness with block/transaction handling when I checked them yesterday, two to the point where they started occasionally dropping connectivity to my private nodes. Like I mentioned, one 24.0.1 node which does not accept incoming connections (but
...
💬 furszy commented on issue "provide optional RPC parameter to not scramble sendmany transactions":
(https://github.com/bitcoin/bitcoin/issues/27592#issuecomment-1537539811)
If your project requires control over the tx, could move to something like:
```python
raw_tx = wallet.createrawtransaction(inputs=[], outputs=["add outputs here"])
funded_tx = wallet.fundrawtranasction(raw_tx)
signed_tx = wallet.signrawtransactionwithwallet(funded_tx['hex'])
wallet.sendrawtransaction(signed_tx['hex'])
```
(https://github.com/bitcoin/bitcoin/issues/27592#issuecomment-1537539811)
If your project requires control over the tx, could move to something like:
```python
raw_tx = wallet.createrawtransaction(inputs=[], outputs=["add outputs here"])
funded_tx = wallet.fundrawtranasction(raw_tx)
signed_tx = wallet.signrawtransactionwithwallet(funded_tx['hex'])
wallet.sendrawtransaction(signed_tx['hex'])
```
💬 ajtowns commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537544963)
> PS: These nodes do **not** run in debug mode.
With the issue I described here patched around, I see a different problem that also results in b-msghand using ~100% of a single cpu. It seems to be spent in `make_heap` in `net_processing.cpp` which in turn seems to be due to the internal queues for which txs need to be INVed to which peers growing much too large (I've observed >60,000 entries in each of 6 peers, eg). It seems to be exacerbated by a combination of inbound peers that aren't real
...
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537544963)
> PS: These nodes do **not** run in debug mode.
With the issue I described here patched around, I see a different problem that also results in b-msghand using ~100% of a single cpu. It seems to be spent in `make_heap` in `net_processing.cpp` which in turn seems to be due to the internal queues for which txs need to be INVed to which peers growing much too large (I've observed >60,000 entries in each of 6 peers, eg). It seems to be exacerbated by a combination of inbound peers that aren't real
...
💬 ajtowns commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537545897)
Oh, if anyone wants to mitigate the `make_heap` thing; I think something like:
```sh
$ bitcoin-cli getpeerinfo | jq -j '.[] | (.bytessent_per_msg.inv, " ", .bytesrecv_per_msg.inv, " ", .addr, "\n")' | grep ' null ' | sort -n
```
should list the nodes where you're sending lots of inv data, without receiving anything in return. Doing a temporary "setban" on those to disconnect them will clear their inv queue and should speed things up temporarily.
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537545897)
Oh, if anyone wants to mitigate the `make_heap` thing; I think something like:
```sh
$ bitcoin-cli getpeerinfo | jq -j '.[] | (.bytessent_per_msg.inv, " ", .bytesrecv_per_msg.inv, " ", .addr, "\n")' | grep ' null ' | sort -n
```
should list the nodes where you're sending lots of inv data, without receiving anything in return. Doing a temporary "setban" on those to disconnect them will clear their inv queue and should speed things up temporarily.
💬 dzyphr commented on pull request "Inscriptions option":
(https://github.com/bitcoin/bitcoin/pull/27589#issuecomment-1537551415)
It will be left up to personal choice no matter what, however I think we should take the approach of mitigating anything that really looks like spam which again will be personal choice.
(https://github.com/bitcoin/bitcoin/pull/27589#issuecomment-1537551415)
It will be left up to personal choice no matter what, however I think we should take the approach of mitigating anything that really looks like spam which again will be personal choice.
💬 instagibbs commented on issue "Avoid serving stale fees":
(https://github.com/bitcoin/bitcoin/issues/27555#issuecomment-1537586780)
looks like one of my nodes had a similar issue with a clean shutdown and restart
https://twitter.com/CoreFeeHelper/status/1655286815291588611 after restart, then an hour or so later: https://twitter.com/CoreFeeHelper/status/1655301916593668097
back up to expected min fee
(https://github.com/bitcoin/bitcoin/issues/27555#issuecomment-1537586780)
looks like one of my nodes had a similar issue with a clean shutdown and restart
https://twitter.com/CoreFeeHelper/status/1655286815291588611 after restart, then an hour or so later: https://twitter.com/CoreFeeHelper/status/1655301916593668097
back up to expected min fee
💬 ArmchairCryptologist commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537784340)
Of the four nodes, the one that was restarted on May 6th is now back to spinning 100% of a CPU core (60% average CPU usage on a two-core VM the last hour) while the three that were restarted yesterday are also seeing high CPU usage but not 100% all the time (between 30% and 38% average CPU usage on a two-core VM), so it's building back up relatively quickly. I also verified that the mempool is not fully committed (usage is at 251217552).
There were around 15 peers with bytessent_per_msg.inv o
...
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537784340)
Of the four nodes, the one that was restarted on May 6th is now back to spinning 100% of a CPU core (60% average CPU usage on a two-core VM the last hour) while the three that were restarted yesterday are also seeing high CPU usage but not 100% all the time (between 30% and 38% average CPU usage on a two-core VM), so it's building back up relatively quickly. I also verified that the mempool is not fully committed (usage is at 251217552).
There were around 15 peers with bytessent_per_msg.inv o
...
💬 S3RK commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1537830694)
re code review ACK 7baa0971527ba3c8d87b82c7a2a4eabf6abbb2b7
My previous comment about exposing `blank` flag is still valid, but I don't think it's blocking. I like furszy's suggestion to return the flag only for regtest
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1537830694)
re code review ACK 7baa0971527ba3c8d87b82c7a2a4eabf6abbb2b7
My previous comment about exposing `blank` flag is still valid, but I don't think it's blocking. I like furszy's suggestion to return the flag only for regtest
💬 MarcoFalke commented on pull request "rpc: distinguish between vsize and sigop-adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1187091583)
Is there a setting to affect this value? If yes, does it make sense to mention the setting's key?
(https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1187091583)
Is there a setting to affect this value? If yes, does it make sense to mention the setting's key?
💬 MarcoFalke commented on pull request "rpc: distinguish between vsize and sigop-adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1187089387)
Is there still a reason with recent univalue to use potentially narrowing casts? You could try no cast or a w-narrowing one (with `{}`)?
(https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1187089387)
Is there still a reason with recent univalue to use potentially narrowing casts? You could try no cast or a w-narrowing one (with `{}`)?
💬 localbool commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537886785)
> Just to confirm, yours was in debug mode as well? Otherwise it'd be concerning.
What is debug mode? A binary compiled with `--enable-debug` option or `debug=1` saved in bitcoin.conf?
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537886785)
> Just to confirm, yours was in debug mode as well? Otherwise it'd be concerning.
What is debug mode? A binary compiled with `--enable-debug` option or `debug=1` saved in bitcoin.conf?
💬 willcl-ark commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537887983)
> What is debug mode? A binary compiled with `--enable-debug` option or `debug=1` saved in bitcoin.conf?
>
>
Compiled with `--enable-debug`
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537887983)
> What is debug mode? A binary compiled with `--enable-debug` option or `debug=1` saved in bitcoin.conf?
>
>
Compiled with `--enable-debug`
⚠️ MarcoFalke opened an issue: "test: RPC coverage check doesn't work?"
(https://github.com/bitcoin/bitcoin/issues/27593)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Currently CI reports that all RPCs are covered: https://cirrus-ci.com/task/6244871116161024?logs=ci#L4674
### Expected behaviour
However, some RPCs are clearly not covered because they are never called. For example:
```
$ git grep abortrescan ./test/ | grep -v '. Please call `abortrescan` before ' | wc -l
0
```
### Steps to reproduce
?
### Relevant log output
_No response_
...
(https://github.com/bitcoin/bitcoin/issues/27593)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Currently CI reports that all RPCs are covered: https://cirrus-ci.com/task/6244871116161024?logs=ci#L4674
### Expected behaviour
However, some RPCs are clearly not covered because they are never called. For example:
```
$ git grep abortrescan ./test/ | grep -v '. Please call `abortrescan` before ' | wc -l
0
```
### Steps to reproduce
?
### Relevant log output
_No response_
...
💬 MarcoFalke commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187190754)
nit in 1def580e80a1892e473324016db9a0f2ffec0c27: (For this method and others like `UndoReadFromDisk` ...) It is UB to pass a nullptr pindex, so it would be good, while touching the function signature in this pull request to switch the pointer to a reference.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187190754)
nit in 1def580e80a1892e473324016db9a0f2ffec0c27: (For this method and others like `UndoReadFromDisk` ...) It is UB to pass a nullptr pindex, so it would be good, while touching the function signature in this pull request to switch the pointer to a reference.
💬 MarcoFalke commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187174928)
nit in ea2a049b42f73499eaa1b69212af3514026440fd: Any reason to accept a nullptr here, which would be UB?
I think it would be better to accept a `const CBlockIndex& block`.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187174928)
nit in ea2a049b42f73499eaa1b69212af3514026440fd: Any reason to accept a nullptr here, which would be UB?
I think it would be better to accept a `const CBlockIndex& block`.
👍 MarcoFalke approved a pull request: "refactor, kernel: Decouple ArgsManager from blockstorage"
(https://github.com/bitcoin/bitcoin/pull/27125#pullrequestreview-1416363907)
nice ACK 3b34ac7465919b968795063995f6610a73aa2d29 🗣
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: nice ACK 3b34ac7465919b96
...
(https://github.com/bitcoin/bitcoin/pull/27125#pullrequestreview-1416363907)
nice ACK 3b34ac7465919b968795063995f6610a73aa2d29 🗣
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: nice ACK 3b34ac7465919b96
...
💬 MarcoFalke commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187180745)
Also, it would be good to include the diff in `src/zmq/zmqpublishnotifier.cpp` from the next commit. This makes review easier, because reviewers can see that the function-to-lambda is a simple 1-1 replacement that compiles and passes all tests on its own.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187180745)
Also, it would be good to include the diff in `src/zmq/zmqpublishnotifier.cpp` from the next commit. This makes review easier, because reviewers can see that the function-to-lambda is a simple 1-1 replacement that compiles and passes all tests on its own.
💬 MarcoFalke commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187211843)
in 0400fb7b55415867d9ff0aa88898920c60b4b2df: The function parameter consensusParams is now unused and should be removed?
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187211843)
in 0400fb7b55415867d9ff0aa88898920c60b4b2df: The function parameter consensusParams is now unused and should be removed?
💬 MarcoFalke commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187263200)
Thanks, thread can be resolved
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187263200)
Thanks, thread can be resolved
💬 MarcoFalke commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187263434)
Thanks, thread can be resolved
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1187263434)
Thanks, thread can be resolved