💬 pinheadmz commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2898832559)
Also this is somewhat pathalogical but...
```
--> bccli -regtest setmocktime 1719861124 # July 1, 2024
--> bccli -regtest -getinfo
Chain: regtest
Blocks: 203
Headers: 203
Verification progress: -0.7451%
```
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2898832559)
Also this is somewhat pathalogical but...
```
--> bccli -regtest setmocktime 1719861124 # July 1, 2024
--> bccli -regtest -getinfo
Chain: regtest
Blocks: 203
Headers: 203
Verification progress: -0.7451%
```
👍 ryanofsky approved a pull request: "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage"
(https://github.com/bitcoin/bitcoin/pull/32467#pullrequestreview-2858731450)
Code review ACK fd290730f530a8b76a9607392f49830697cdd7c5, just removing assert since last review. Thanks for considering all the comments and feedback!
(https://github.com/bitcoin/bitcoin/pull/32467#pullrequestreview-2858731450)
Code review ACK fd290730f530a8b76a9607392f49830697cdd7c5, just removing assert since last review. Thanks for considering all the comments and feedback!
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101000637)
Added as comment.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101000637)
Added as comment.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100993968)
I have changed the variable names to `max_cluster_count` and `max_cluster_size` everywhere.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100993968)
I have changed the variable names to `max_cluster_count` and `max_cluster_size` everywhere.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100995536)
`GetTotalTxSize` is a nice color.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100995536)
`GetTotalTxSize` is a nice color.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100996101)
Done, but also removed in a later commit.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100996101)
Done, but also removed in a later commit.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100995144)
Do you mean in the TxGraph::AddTransaction definition in txgraph.h? I have expanded the comments in the fuzz test around this a bit.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100995144)
Do you mean in the TxGraph::AddTransaction definition in txgraph.h? I have expanded the comments in the fuzz test around this a bit.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100993169)
I have expended the commit message of both Trim commits.
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100993169)
I have expended the commit message of both Trim commits.
💬 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.