π€ tapcrafter reviewed a pull request: "test: properly check for per-tx sigops limit"
(https://github.com/bitcoin/bitcoin/pull/32533#pullrequestreview-2848800587)
tACK 7bc64a8859c3644fdf2eeff59f72a778ae60ea3f
<details>
<summary>Test protocol</summary>
On master (7710a31f0cb69a04529f39840196826d0b9770ab), changing `MAX_STANDARD_TX_SIGOPS_COST` in `src/policy/policy.h` to a higher value didn't cause the tests to fail:
```shell
$ ./build/test/functional/p2p_invalid_tx.py
2025-05-18T07:49:02.383000Z TestFramework (INFO): PRNG seed is: 4606283318320792897
...
2025-05-18T07:49:04.855000Z TestFramework (INFO): Testing invalid transaction: Too
...
(https://github.com/bitcoin/bitcoin/pull/32533#pullrequestreview-2848800587)
tACK 7bc64a8859c3644fdf2eeff59f72a778ae60ea3f
<details>
<summary>Test protocol</summary>
On master (7710a31f0cb69a04529f39840196826d0b9770ab), changing `MAX_STANDARD_TX_SIGOPS_COST` in `src/policy/policy.h` to a higher value didn't cause the tests to fail:
```shell
$ ./build/test/functional/p2p_invalid_tx.py
2025-05-18T07:49:02.383000Z TestFramework (INFO): PRNG seed is: 4606283318320792897
...
2025-05-18T07:49:04.855000Z TestFramework (INFO): Testing invalid transaction: Too
...
π¬ katesalazar commented on issue "Option to use dark theme for Windows":
(https://github.com/bitcoin-core/gui/issues/378#issuecomment-2888852325)
> Feel free to open a fresh issue about that.
I won't, busy days.
(https://github.com/bitcoin-core/gui/issues/378#issuecomment-2888852325)
> Feel free to open a fresh issue about that.
I won't, busy days.
π¬ romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2888872800)
> Nice.
Thanks :)
> I wonder about just adding a content-type like option so that the api endpoint is the same but you simply say if you want binary or json content returned? Maybe there's other endpoints that it could then be applied to as well.
Good idea - would it be OK to implement it in a separate PR?
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2888872800)
> Nice.
Thanks :)
> I wonder about just adding a content-type like option so that the api endpoint is the same but you simply say if you want binary or json content returned? Maybe there's other endpoints that it could then be applied to as well.
Good idea - would it be OK to implement it in a separate PR?
π¬ tapcrafter commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2094428227)
Do I assume correctly that the reason for this limit not being 4 GiB but only 1 GiB is that the sum of all cache/limit values should be below 4GB to avoid issues on 32-bit systems?
The same question would apply to `MAX_32BIT_MEMPOOL_MB`, what is the rationale for the value of 500MiB?
Not sure if this would fall into the "you chose to shoot yourself in the foot" category, but I wonder: If I turned on all indexes and used the maximum "allowed" values for all of them, would I still be able to r
...
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2094428227)
Do I assume correctly that the reason for this limit not being 4 GiB but only 1 GiB is that the sum of all cache/limit values should be below 4GB to avoid issues on 32-bit systems?
The same question would apply to `MAX_32BIT_MEMPOOL_MB`, what is the rationale for the value of 500MiB?
Not sure if this would fall into the "you chose to shoot yourself in the foot" category, but I wonder: If I turned on all indexes and used the maximum "allowed" values for all of them, would I still be able to r
...
π€ tapcrafter reviewed a pull request: "node: cap `-maxmempool` and `-dbcache` values for 32-bit"
(https://github.com/bitcoin/bitcoin/pull/32530#pullrequestreview-2848812176)
utACK e88056ba323da9127775a41db92835378600d9fc
Will test this later once I've familiarized myself a bit more with the build system to produce 32-bit ARM binaries outside of the guix build.
(https://github.com/bitcoin/bitcoin/pull/32530#pullrequestreview-2848812176)
utACK e88056ba323da9127775a41db92835378600d9fc
Will test this later once I've familiarized myself a bit more with the build system to produce 32-bit ARM binaries outside of the guix build.
π¬ tapcrafter commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2094428985)
I'm a bit surprised there isn't a macro for the 32-bit detection (`sizeof(void*) == 4`). But I'm fairly new to the C++ part of the codebase and I see this is used in other places too, so just a random review thought that can probably be ignored.
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2094428985)
I'm a bit surprised there isn't a macro for the 32-bit detection (`sizeof(void*) == 4`). But I'm fairly new to the C++ part of the codebase and I see this is used in other places too, so just a random review thought that can probably be ignored.
π luke-jr opened a pull request: "Mining: Avoid copying template CBlocks"
(https://github.com/bitcoin/bitcoin/pull/32547)
Refactoring to avoid unnecessary copies/complexity, at least in the mainnet paths.
Also abstracts out a new `TemplateToJSON` function to keep track of variable scoping better.
(https://github.com/bitcoin/bitcoin/pull/32547)
Refactoring to avoid unnecessary copies/complexity, at least in the mainnet paths.
Also abstracts out a new `TemplateToJSON` function to keep track of variable scoping better.
π¬ 1440000bytes commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2888896477)
> It seems if you want people to actually use this much OP RETURN data by relaxing the filter it'd make sense to simultaneously prevent "inscriptions" with this #28408
You can ACK this pull request if you agree with the changes and open a new issue or pull request for #28408
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2888896477)
> It seems if you want people to actually use this much OP RETURN data by relaxing the filter it'd make sense to simultaneously prevent "inscriptions" with this #28408
You can ACK this pull request if you agree with the changes and open a new issue or pull request for #28408
β οΈ l3x3l opened an issue: "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client"
(https://github.com/bitcoin/bitcoin/issues/32548)
### Motivation
I am encountering a persistent "Wallet requires newer version of Bitcoin Core (code -4)" error when trying to open my wallet.dat file on macOS. This issue occurs even when attempting to open the wallet with Bitcoin Core version 0.21.0.1, which the debug log from a later version (29.0) indicated was the last client version to interact with the wallet.
Environment:
* Operating System: macOS (please specify your version if you know it)
* Bitcoin Core Version Initially Used: 29.0
...
(https://github.com/bitcoin/bitcoin/issues/32548)
### Motivation
I am encountering a persistent "Wallet requires newer version of Bitcoin Core (code -4)" error when trying to open my wallet.dat file on macOS. This issue occurs even when attempting to open the wallet with Bitcoin Core version 0.21.0.1, which the debug log from a later version (29.0) indicated was the last client version to interact with the wallet.
Environment:
* Operating System: macOS (please specify your version if you know it)
* Bitcoin Core Version Initially Used: 29.0
...
π¬ blockdyor commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2888907334)
Concept NACK
Nodes shouldnβt relay arbitrary data on the network. Removing the cap on the default datacarriersize makes that easier. Weβve already seen a clear signal that people donβt want this, with a noticeable migration from Bitcoin Core to Bitcoin Knots over the past three weeks. And since [PR #32359](https://github.com/bitcoin/bitcoin/pull/32359#event-17618932800) was closed by @glozow a few days ago, Iβd expect this one to be closed too.
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2888907334)
Concept NACK
Nodes shouldnβt relay arbitrary data on the network. Removing the cap on the default datacarriersize makes that easier. Weβve already seen a clear signal that people donβt want this, with a noticeable migration from Bitcoin Core to Bitcoin Knots over the past three weeks. And since [PR #32359](https://github.com/bitcoin/bitcoin/pull/32359#event-17618932800) was closed by @glozow a few days ago, Iβd expect this one to be closed too.
π¬ Fraser052 commented on issue "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client":
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2888912141)
> ### Motivation
>
> I am encountering a persistent "Wallet requires newer version of Bitcoin Core (code -4)" error when trying to open my wallet.dat file on macOS. This issue occurs even when attempting to open the wallet with Bitcoin Core version 0.21.0.1, which the debug log from a later version (29.0) indicated was the last client version to interact with the wallet. Environment:
>
> * Operating System: macOS 15
> * Bitcoin Core Version Initially Used: 29.0
> * Bitcoin Core Version Tested
...
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2888912141)
> ### Motivation
>
> I am encountering a persistent "Wallet requires newer version of Bitcoin Core (code -4)" error when trying to open my wallet.dat file on macOS. This issue occurs even when attempting to open the wallet with Bitcoin Core version 0.21.0.1, which the debug log from a later version (29.0) indicated was the last client version to interact with the wallet. Environment:
>
> * Operating System: macOS 15
> * Bitcoin Core Version Initially Used: 29.0
> * Bitcoin Core Version Tested
...
π¬ l3x3l commented on issue "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client":
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2888913648)
Hello,
I have opened the following issue in GitHub
On Sun, May 18, 2025 at 4:35 AM Fraser052 ***@***.***> wrote:
> *Fraser052* left a comment (bitcoin/bitcoin#32548)
> <https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2888912141>
>
> Motivation
>
> I am encountering a persistent "Wallet requires newer version of Bitcoin
> Core (code -4)" error when trying to open my wallet.dat file on macOS. This
> issue occurs even when attempting to open the wallet with Bitcoin Co
...
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2888913648)
Hello,
I have opened the following issue in GitHub
On Sun, May 18, 2025 at 4:35 AM Fraser052 ***@***.***> wrote:
> *Fraser052* left a comment (bitcoin/bitcoin#32548)
> <https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2888912141>
>
> Motivation
>
> I am encountering a persistent "Wallet requires newer version of Bitcoin
> Core (code -4)" error when trying to open my wallet.dat file on macOS. This
> issue occurs even when attempting to open the wallet with Bitcoin Co
...
π¬ maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2888967608)
The bt is:
```
Thread 9 (Thread 0x7f0973400640 (LWP 21341) "b-httpworker.5"):
#0 __GI___libc_read (nbytes=1024, buf=0x7f09733fb7f0, fd=30) at ../sysdeps/unix/sysv/linux/read.c:26
#1 __GI___libc_read (fd=30, buf=0x7f09733fb7f0, nbytes=1024) at ../sysdeps/unix/sysv/linux/read.c:24
#2 0x0000564d74731ba3 in subprocess::Popen::execute_process() ()
#3 0x0000564d74732b44 in subprocess::Popen::Popen<subprocess::input, subprocess::output, subprocess::error, subprocess::close_fds>(std::__cxx11::basic
...
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2888967608)
The bt is:
```
Thread 9 (Thread 0x7f0973400640 (LWP 21341) "b-httpworker.5"):
#0 __GI___libc_read (nbytes=1024, buf=0x7f09733fb7f0, fd=30) at ../sysdeps/unix/sysv/linux/read.c:26
#1 __GI___libc_read (fd=30, buf=0x7f09733fb7f0, nbytes=1024) at ../sysdeps/unix/sysv/linux/read.c:24
#2 0x0000564d74731ba3 in subprocess::Popen::execute_process() ()
#3 0x0000564d74732b44 in subprocess::Popen::Popen<subprocess::input, subprocess::output, subprocess::error, subprocess::close_fds>(std::__cxx11::basic
...
π€ tapcrafter reviewed a pull request: "rest: fetch spent transaction outputs by blockhash"
(https://github.com/bitcoin/bitcoin/pull/32540#pullrequestreview-2848945630)
tACK ffe571f461930b7a05a3cf9f7128e843ea9f7e2d
<details>
<summary>Test protocol</summary>
Running ffe571f461930b7a05a3cf9f7128e843ea9f7e2d:
```shell
$ ./build/bin/bitcoind -regtest -rpcallowip=::1 -rpcuser=u -rpcpassword=p -rest -txindex
# Invalid extension:
$ curl -v -g 'u:p@localhost:18443/rest/spentoutputs/4376e2f945afef224981b665778ec45ebe64745e7d768c2937a8e271b69d708a.json'
* Host localhost:18443 was resolved.
...
< HTTP/1.1 404 Not Found
< Content-Type: text/plain
...
(https://github.com/bitcoin/bitcoin/pull/32540#pullrequestreview-2848945630)
tACK ffe571f461930b7a05a3cf9f7128e843ea9f7e2d
<details>
<summary>Test protocol</summary>
Running ffe571f461930b7a05a3cf9f7128e843ea9f7e2d:
```shell
$ ./build/bin/bitcoind -regtest -rpcallowip=::1 -rpcuser=u -rpcpassword=p -rest -txindex
# Invalid extension:
$ curl -v -g 'u:p@localhost:18443/rest/spentoutputs/4376e2f945afef224981b665778ec45ebe64745e7d768c2937a8e271b69d708a.json'
* Host localhost:18443 was resolved.
...
< HTTP/1.1 404 Not Found
< Content-Type: text/plain
...
π¬ tapcrafter commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2094516153)
Is there a way to document the output of this REST method?
From the name alone I would've expected it to return a list of outpoints.
But it seems to return a list of transaction outputs (`CTxOut` or `Coin` depending on the context).
Which absolutely makes sense given the use case.
So perhaps a different name would help make that more clear? Perhaps `rest/spenttxouts`?
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2094516153)
Is there a way to document the output of this REST method?
From the name alone I would've expected it to return a list of outpoints.
But it seems to return a list of transaction outputs (`CTxOut` or `Coin` depending on the context).
Which absolutely makes sense given the use case.
So perhaps a different name would help make that more clear? Perhaps `rest/spenttxouts`?
π¬ Ashkar776 commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2888998449)
Idoknow
On Sun, 18 May, 2025, 5:05 pm tapcrafter, ***@***.***> wrote:
> ***@***.**** commented on this pull request.
>
> tACK ffe571f
> <https://github.com/bitcoin/bitcoin/commit/ffe571f461930b7a05a3cf9f7128e843ea9f7e2d>
> Test protocol
>
> Running ffe571f
> <https://github.com/bitcoin/bitcoin/commit/ffe571f461930b7a05a3cf9f7128e843ea9f7e2d>
> :
>
> $ ./build/bin/bitcoind -regtest -rpcallowip=::1 -rpcuser=u -rpcpassword=p -rest -txindex
> # Invalid extension:
>
> $ curl -v -g
...
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2888998449)
Idoknow
On Sun, 18 May, 2025, 5:05 pm tapcrafter, ***@***.***> wrote:
> ***@***.**** commented on this pull request.
>
> tACK ffe571f
> <https://github.com/bitcoin/bitcoin/commit/ffe571f461930b7a05a3cf9f7128e843ea9f7e2d>
> Test protocol
>
> Running ffe571f
> <https://github.com/bitcoin/bitcoin/commit/ffe571f461930b7a05a3cf9f7128e843ea9f7e2d>
> :
>
> $ ./build/bin/bitcoind -regtest -rpcallowip=::1 -rpcuser=u -rpcpassword=p -rest -txindex
> # Invalid extension:
>
> $ curl -v -g
...
π hebasto opened a pull request: "cmake: Add missed `SSE41_CXXFLAGS`"
(https://github.com/bitcoin/bitcoin/pull/32550)
The missed flags were noticed when building with clang-cl on Windows.
(https://github.com/bitcoin/bitcoin/pull/32550)
The missed flags were noticed when building with clang-cl on Windows.
π¬ yancyribbens commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2889002779)
> Good idea - would it be OK to implement it in a separate PR?
Sure, as you wish. I'm no authority :)
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2889002779)
> Good idea - would it be OK to implement it in a separate PR?
Sure, as you wish. I'm no authority :)
π¬ furszy commented on issue "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client":
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2889007547)
Try sharing your debug.log file when loading v29. It will provide further information to debug your issue.
Also, please avoid pasting LLM-generated text. It adds unnecessary unhelpful noise.
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2889007547)
Try sharing your debug.log file when loading v29. It will provide further information to debug your issue.
Also, please avoid pasting LLM-generated text. It adds unnecessary unhelpful noise.
π¬ pinheadmz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2094541457)
I should have mentioned the initial test commits have been split off into https://github.com/bitcoin/bitcoin/pull/32408 and the test as written here failed CI, so has been modified with `rpcservertimeout=2` and then expects a timeout between 1 and 4 seconds
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2094541457)
I should have mentioned the initial test commits have been split off into https://github.com/bitcoin/bitcoin/pull/32408 and the test as written here failed CI, so has been modified with `rpcservertimeout=2` and then expects a timeout between 1 and 4 seconds