💬 m3dwards commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1819053244)
@kashifs thank you for your review, fixed all the found typos and removed the `$ ` signs.
Yes, for the mainnet test of import mempool you would need to be fully synced. I've added a note about that and an extra call to `-getinfo` to confirm it.
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1819053244)
@kashifs thank you for your review, fixed all the found typos and removed the `$ ` signs.
Yes, for the mainnet test of import mempool you would need to be fully synced. I've added a note about that and an extra call to `-getinfo` to confirm it.
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399223399)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1: can you order chunks by mining score?
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399223399)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1: can you order chunks by mining score?
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399231122)
It would be good to explain the rationale for the miner score somewhere. Why a * b and b * a?
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399231122)
It would be good to explain the rationale for the miner score somewhere. Why a * b and b * a?
📝 hebasto opened a pull request: "build: Fix regression in "ARMv8 CRC32 intrinsics" test"
(https://github.com/bitcoin/bitcoin/pull/28919)
In the master branch, the `aarch64` binaries lack support for CRC32 intrinsics.
The `vmull_p64` is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a `+crypto` for architecture flags.
The regression was introduced in https://github.com/bitcoin/bitcoin/pull/26183 (v25.0).
The `./configure` script log excerpts:
- the master branch @ d752349029ec7a76f1fd440db2ec2e458d0f3c99:
```
checking whether C++ compiler accepts -march=armv8-a+crc
...
(https://github.com/bitcoin/bitcoin/pull/28919)
In the master branch, the `aarch64` binaries lack support for CRC32 intrinsics.
The `vmull_p64` is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a `+crypto` for architecture flags.
The regression was introduced in https://github.com/bitcoin/bitcoin/pull/26183 (v25.0).
The `./configure` script log excerpts:
- the master branch @ d752349029ec7a76f1fd440db2ec2e458d0f3c99:
```
checking whether C++ compiler accepts -march=armv8-a+crc
...
💬 sipa commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1819105431)
@Sjors I believe most if not all of this PR will be rewritten, and split up into several components. The goal here is just to give an idea of the high-level interactions with other changes (wallet behavior, package validation/relay/RBF, ...). I don't think a detailed line-by-line code review at this stage is a good use of your time.
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1819105431)
@Sjors I believe most if not all of this PR will be rewritten, and split up into several components. The goal here is just to give an idea of the high-level interactions with other changes (wallet behavior, package validation/relay/RBF, ...). I don't think a detailed line-by-line code review at this stage is a good use of your time.
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399244164)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1: this should also list the cluster id, and maybe also the chunk number.
Could also add an argument limit the total number of vbytes.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399244164)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1: this should also list the cluster id, and maybe also the chunk number.
Could also add an argument limit the total number of vbytes.
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399246551)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1 : Could this use `CompareMiningScore` or does it have a different goal?
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399246551)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1 : Could this use `CompareMiningScore` or does it have a different goal?
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399253500)
bf467f8286425b692a1736ab6d417d0ba6074658 Maybe make this rule 7. That seems a bit more clear than saying "previously this rule referred to fee rate"
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399253500)
bf467f8286425b692a1736ab6d417d0ba6074658 Maybe make this rule 7. That seems a bit more clear than saying "previously this rule referred to fee rate"
💬 hebasto commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1819129629)
A failure in the "Win64 native, VS 2022" CI job is unrelated. See: https://github.com/bitcoin/bitcoin/pull/28905.
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1819129629)
A failure in the "Win64 native, VS 2022" CI job is unrelated. See: https://github.com/bitcoin/bitcoin/pull/28905.
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399257380)
"a higher feerate or (using clusters) mining score"
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399257380)
"a higher feerate or (using clusters) mining score"
👍 BrandonOdiwuor approved a pull request: "Update Node window title with the chain type"
(https://github.com/bitcoin-core/gui/pull/758#pullrequestreview-1739931365)
tested ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457
(https://github.com/bitcoin-core/gui/pull/758#pullrequestreview-1739931365)
tested ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399277318)
bf467f8286425b692a1736ab6d417d0ba6074658: history can be expanded:
```md
* Cluster mempool introduced, dropping rule 2 and introducing rule 7. As of **v27.0** ([PR 28676](https://github.com/bitcoin/bitcoin/pull/28676)).
```
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399277318)
bf467f8286425b692a1736ab6d417d0ba6074658: history can be expanded:
```md
* Cluster mempool introduced, dropping rule 2 and introducing rule 7. As of **v27.0** ([PR 28676](https://github.com/bitcoin/bitcoin/pull/28676)).
```
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1399277913)
Ok removed the 0 padding in 1b0fa12fa5
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1399277913)
Ok removed the 0 padding in 1b0fa12fa5
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1819164214)
Updated to address luke's comment and added a test to ensure rpccookieperms are being applied
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1819164214)
Updated to address luke's comment and added a test to ensure rpccookieperms are being applied
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399281690)
bf467f8286425b692a1736ab6d417d0ba6074658: `; new chunk feerate %s <= old chunk feerate`
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399281690)
bf467f8286425b692a1736ab6d417d0ba6074658: `; new chunk feerate %s <= old chunk feerate`
💬 hebasto commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399309912)
> Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/.
Mind sharing your reasoning behind this decision? Asking because the alternative looks more generic and will work if any other dependency package will switch to CMake.
---
A side note, not directly related to this PR: the CMake-based build system has no such an issue at all.
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399309912)
> Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/.
Mind sharing your reasoning behind this decision? Asking because the alternative looks more generic and will work if any other dependency package will switch to CMake.
---
A side note, not directly related to this PR: the CMake-based build system has no such an issue at all.
💬 ryanofsky commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399329330)
> > Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/.
>
> Mind sharing your reasoning behind this decision?
The quote you're responding is from fanquake, but I think it makes sense for the depends build to hardcode "lib" here because it is already hardcoding "lib" in `config.site.in`:
https://github.com/bitcoin/bitcoin/blob/d752349029ec7a76f1fd440db2ec2e458d0f3c99/depends/config.site.in#L91
So this change just keeps the build consist
...
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399329330)
> > Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/.
>
> Mind sharing your reasoning behind this decision?
The quote you're responding is from fanquake, but I think it makes sense for the depends build to hardcode "lib" here because it is already hardcoding "lib" in `config.site.in`:
https://github.com/bitcoin/bitcoin/blob/d752349029ec7a76f1fd440db2ec2e458d0f3c99/depends/config.site.in#L91
So this change just keeps the build consist
...
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1819230080)
I was wondering if we can drop RBF rule 5 (in a followup), but I'm guessing not really. My initial thinking was the cluster limit could be used instead. But `CalculateMiningScoreOfReplacementTx` only checks that the _new_ cluster doesn't get too big.
But does the new cluster system make it less of a burden to have large numbers of transactions appear and disappear from the mempool?
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1819230080)
I was wondering if we can drop RBF rule 5 (in a followup), but I'm guessing not really. My initial thinking was the cluster limit could be used instead. But `CalculateMiningScoreOfReplacementTx` only checks that the _new_ cluster doesn't get too big.
But does the new cluster system make it less of a burden to have large numbers of transactions appear and disappear from the mempool?
💬 brunoerg commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1819233630)
force-pushed to fix CI error
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1819233630)
force-pushed to fix CI error
👋 brunoerg's pull request is ready for review: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578)
(https://github.com/bitcoin/bitcoin/pull/28578)
💬 hebasto commented on issue "Sync slow":
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1819241844)
> Please reopen the issue, I will try to give logs with lock conflicts later. Probably related #776
Windows and Fedora, which was mentioned by this issue author, are quite different systems.
The rule of thumb for Bitcoin Core users on Windows is to add the data and block directories into Windows Security exclusions.
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1819241844)
> Please reopen the issue, I will try to give logs with lock conflicts later. Probably related #776
Windows and Fedora, which was mentioned by this issue author, are quite different systems.
The rule of thumb for Bitcoin Core users on Windows is to add the data and block directories into Windows Security exclusions.