💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101001371)
I've added a comment to highlight that.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101001371)
I've added a comment to highlight that.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101002887)
Done.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101002887)
Done.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101002139)
Good idea, I went with `OVERSIZED_SINGLETON`.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101002139)
Good idea, I went with `OVERSIZED_SINGLETON`.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101006466)
Instead of this big unintuitive comment, I have replaced it with `/*oversized_tx=*/m_quality == QualityLevel::OVERSIZED_SINGLETON`. With that, I don't think the `Assume` is neccesary?
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101006466)
Instead of this big unintuitive comment, I have replaced it with `/*oversized_tx=*/m_quality == QualityLevel::OVERSIZED_SINGLETON`. With that, I don't think the `Assume` is neccesary?
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101012572)
I have expanded the commit messages.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101012572)
I have expanded the commit messages.
💬 laanwj commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2101043927)
fwiw: `GetACP` is a windows system function implemented in `kernel32.dll`, defined in the `winnls.h` header. There shouldn't be a mingw-specific definition.
`CP_UTF8` is also defined in that header and has the following value:
```c++
#define CP_UTF8 65001
```
`1252` is codepage [windows-1252](https://en.wikipedia.org/wiki/Windows-1252), a legacy encoding.
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2101043927)
fwiw: `GetACP` is a windows system function implemented in `kernel32.dll`, defined in the `winnls.h` header. There shouldn't be a mingw-specific definition.
`CP_UTF8` is also defined in that header and has the following value:
```c++
#define CP_UTF8 65001
```
`1252` is codepage [windows-1252](https://en.wikipedia.org/wiki/Windows-1252), a legacy encoding.
💬 polespinasa commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2899113774)
> when I open regtest after a few days...
Maybe we should force this code to work only in mainnet? It doesn't make sense the time considerations for regtest. Maybe testnet and signet could still use it.
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2899113774)
> when I open regtest after a few days...
Maybe we should force this code to work only in mainnet? It doesn't make sense the time considerations for regtest. Maybe testnet and signet could still use it.
👍 willcl-ark approved a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32292#pullrequestreview-2859072889)
crACK a0d1f69b555fa0c76df1a63a0b127c7816596107
Checked all patches were backported cleanly and appear in the release notes.
Did not build/test.
(https://github.com/bitcoin/bitcoin/pull/32292#pullrequestreview-2859072889)
crACK a0d1f69b555fa0c76df1a63a0b127c7816596107
Checked all patches were backported cleanly and appear in the release notes.
Did not build/test.
🤔 Crypt-iQ reviewed a pull request: "fuzz: doc: add info about `afl-system-config` for macOS"
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2859098136)
ACK 61ea5f3
I noticed that a couple lines above, the link to "selecting the best AFL compiler..." is invalid and has instead moved to https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/fuzzing_in_depth.md#a-selecting-the-best-afl-compiler-for-instrumenting-the-target. I can open a PR to fix the doc link.
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2859098136)
ACK 61ea5f3
I noticed that a couple lines above, the link to "selecting the best AFL compiler..." is invalid and has instead moved to https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/fuzzing_in_depth.md#a-selecting-the-best-afl-compiler-for-instrumenting-the-target. I can open a PR to fix the doc link.
💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2101176481)
Fixed
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2101176481)
Fixed
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101196912)
Does :+1: mean "That is what I meant" or "It's fixed now"?
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101196912)
Does :+1: mean "That is what I meant" or "It's fixed now"?
💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2899294093)
@laanwj @ryanofsky
Thank you for your feedback!
I've restored the `shell` option in the `subprocess` API, and employed it on non-Windows systems.
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2899294093)
@laanwj @ryanofsky
Thank you for your feedback!
I've restored the `shell` option in the `subprocess` API, and employed it on non-Windows systems.
💬 sipa commented on pull request "allocators: Apply manual ASan poisoning to `PoolResource`":
(https://github.com/bitcoin/bitcoin/pull/32581#issuecomment-2899298395)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32581#issuecomment-2899298395)
Concept ACK
💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2899323908)
> Then, given that the fallback seems to be needed by OpenBSD and newlibc (possibly others), it may be better to keep it?
It would be even better to use `ftruncate` on openbsd. But yes, simply assuming `posix_fallocate` availability is going to fail on some platforms.
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2899323908)
> Then, given that the fallback seems to be needed by OpenBSD and newlibc (possibly others), it may be better to keep it?
It would be even better to use `ftruncate` on openbsd. But yes, simply assuming `posix_fallocate` availability is going to fail on some platforms.
⚠️ hMsats reopened an issue: "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system"
(https://github.com/bitcoin/bitcoin/issues/32455)
I always compile the bitcoin software myself (`Ubuntu 24.04.2 LTS`). For `bitcoin-29.0` I compiled it (of course) for the first time with `cmake` (with and without `berkeley-db`).
For both (with and without `berkeley-db`), `bitcoind-29.0` is much slower than `bitcoin-28.0` which gives me problems on my server.
To investigate I wrote a shell script that determines the time difference in seconds between "Saw new header" and "UpdateTip":
```
2025-05-09T05:24:11Z Saw new header hash=000000000000
...
(https://github.com/bitcoin/bitcoin/issues/32455)
I always compile the bitcoin software myself (`Ubuntu 24.04.2 LTS`). For `bitcoin-29.0` I compiled it (of course) for the first time with `cmake` (with and without `berkeley-db`).
For both (with and without `berkeley-db`), `bitcoind-29.0` is much slower than `bitcoin-28.0` which gives me problems on my server.
To investigate I wrote a shell script that determines the time difference in seconds between "Saw new header" and "UpdateTip":
```
2025-05-09T05:24:11Z Saw new header hash=000000000000
...
💬 hMsats commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2899388960)
@sipa After a long and systematic research of my problem, I was probably able to find the cause:
In validate.cpp `else if (!check())` was changed into `else if (auto result = check(); result.has_value())`.
Shouldn't that be `else if (auto result = check(); !result.has_value())`?
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2899388960)
@sipa After a long and systematic research of my problem, I was probably able to find the cause:
In validate.cpp `else if (!check())` was changed into `else if (auto result = check(); result.has_value())`.
Shouldn't that be `else if (auto result = check(); !result.has_value())`?
💬 achow101 commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2899407390)
> In validation.cpp `else if (!check())` was changed into `else if (auto result = check(); result.has_value())`.
> Shouldn't that be `else if (auto result = check(); !result.has_value())`?
Why do you think that's the error?
If this were incorrect, you wouldn't be seeing that validation be slow; it would instead be failing on a bunch of blocks.
This line is
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2899407390)
> In validation.cpp `else if (!check())` was changed into `else if (auto result = check(); result.has_value())`.
> Shouldn't that be `else if (auto result = check(); !result.has_value())`?
Why do you think that's the error?
If this were incorrect, you wouldn't be seeing that validation be slow; it would instead be failing on a bunch of blocks.
This line is
💬 hMsats commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2899420381)
@achow101 somewhere near that commit I think my server becomes slow. I didn't fully understand the code, so sorry. I will investigate further until I do find a problematic commit. Thanks!
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2899420381)
@achow101 somewhere near that commit I think my server becomes slow. I didn't fully understand the code, so sorry. I will investigate further until I do find a problematic commit. Thanks!
💬 hMsats commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2899433923)
For now I think something bad happens between commit:
52fd1511a774d1ff9e747b2ce88aa1fcb778ced8
and
ebe4cac38bf6c510b00b8e080acab079c54016d6
I'll try to narrow it down.
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2899433923)
For now I think something bad happens between commit:
52fd1511a774d1ff9e747b2ce88aa1fcb778ced8
and
ebe4cac38bf6c510b00b8e080acab079c54016d6
I'll try to narrow it down.
💬 ryanofsky commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2899448860)
> I've restored the `shell` option in the `subprocess` API, and employed it on non-Windows systems.
Hmm the shell support implemented in eb160602a50bebcca3f53cdae741e1732738d51a actually seems some broken as it is still splitting and joining instead of preserving the original string passed to Popen() instead of just passing it unaltered to execvp as I was suggesting.
It actually might not work too badly in that commit as long as the original string doesn't contain any tab characters, becau
...
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2899448860)
> I've restored the `shell` option in the `subprocess` API, and employed it on non-Windows systems.
Hmm the shell support implemented in eb160602a50bebcca3f53cdae741e1732738d51a actually seems some broken as it is still splitting and joining instead of preserving the original string passed to Popen() instead of just passing it unaltered to execvp as I was suggesting.
It actually might not work too badly in that commit as long as the original string doesn't contain any tab characters, becau
...