π¬ pinheadmz commented on issue "listunspent, fundrawtransaction, getwalletinfo locks wallet for any other operation":
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2884897418)
> Yes, this is it! Version of bitcoind from this [#26008](https://github.com/bitcoin/bitcoin/pull/26008) pull request works just fine:
Can this issue be closed?
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2884897418)
> Yes, this is it! Version of bitcoind from this [#26008](https://github.com/bitcoin/bitcoin/pull/26008) pull request works just fine:
Can this issue be closed?
π¬ hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091860732)
Git approach is command + subcommand:
```
βΏ git sdfsdf
git: 'sdfsdf' is not a git command. See 'git --help'.
βΏ git remote sdfsdf
error: unknown subcommand: `sdfsdf'
```
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091860732)
Git approach is command + subcommand:
```
βΏ git sdfsdf
git: 'sdfsdf' is not a git command. See 'git --help'.
βΏ git remote sdfsdf
error: unknown subcommand: `sdfsdf'
```
π¬ gituser commented on issue "listunspent, fundrawtransaction, getwalletinfo locks wallet for any other operation":
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2884901552)
@pinheadmz no, this is still an actual bug for me.
e.g. `fundrawtransaction` blocks the wallet for me if it executes for a long time and also blocks all other open wallets for that time.
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2884901552)
@pinheadmz no, this is still an actual bug for me.
e.g. `fundrawtransaction` blocks the wallet for me if it executes for a long time and also blocks all other open wallets for that time.
π¬ furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2091867851)
yes, but without expecting a shutdown.
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2091867851)
yes, but without expecting a shutdown.
π¬ Crypto2 commented on issue "listunspent, fundrawtransaction, getwalletinfo locks wallet for any other operation":
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2884909389)
It is insane after all this time this still hasn't been fixed.
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2884909389)
It is insane after all this time this still hasn't been fixed.
π¬ i-am-yuvi commented on pull request "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/32509#issuecomment-2884915373)
Tested ACK c47f634718d4248fd2a30e51a57944f89da72a64
```
./build/test/functional/test_runner.py feature_framework_startup_failures.py --timeout-factor=0
Temporary test directory at /var/folders/jb/wlbrz0t95vl58wzxjt75wqmh0000gn/T/test_runner_βΏ_π_20250516_012813
Remaining jobs: [feature_framework_startup_failures.py]
1/1 - feature_framework_startup_failures.py passed, Duration: 5 s
TEST | STATUS | DURATION
feature_framework_startup_failures.py | β
...
(https://github.com/bitcoin/bitcoin/pull/32509#issuecomment-2884915373)
Tested ACK c47f634718d4248fd2a30e51a57944f89da72a64
```
./build/test/functional/test_runner.py feature_framework_startup_failures.py --timeout-factor=0
Temporary test directory at /var/folders/jb/wlbrz0t95vl58wzxjt75wqmh0000gn/T/test_runner_βΏ_π_20250516_012813
Remaining jobs: [feature_framework_startup_failures.py]
1/1 - feature_framework_startup_failures.py passed, Duration: 5 s
TEST | STATUS | DURATION
feature_framework_startup_failures.py | β
...
π¬ furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2884916792)
> Is it worth it to verify that, previous to this PR, the node with the legacy wallet in its `settings.json` was failing to restart (using `assert_start_raises_init_error`)?
That's what is being tested. You can run the test on master and it will fail due to that behavior.
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2884916792)
> Is it worth it to verify that, previous to this PR, the node with the legacy wallet in its `settings.json` was failing to restart (using `assert_start_raises_init_error`)?
That's what is being tested. You can run the test on master and it will fail due to that behavior.
π¬ achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2091873231)
Will do if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2091873231)
Will do if I need to retouch.
π¬ achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2091878196)
Not sure what you would be verified? After invalidateblock and before the crash, we do verify that the coinbase transaction is marked abandoned. This check is to verify that starting after the crash, the wallet has the correct before-invalidate state.
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2091878196)
Not sure what you would be verified? After invalidateblock and before the crash, we do verify that the coinbase transaction is marked abandoned. This check is to verify that starting after the crash, the wallet has the correct before-invalidate state.
π¬ achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2091878948)
Will do if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2091878948)
Will do if I need to retouch.
π maflcko opened a pull request: "Remove legacy `Parse(U)Int*`"
(https://github.com/bitcoin/bitcoin/pull/32520)
The legacy int parsing is problematic, because it accepts the `+` sign for unsigned integers. In all cases this is either:
* Useless, because the `+` sign was already rejected.
* Erroneous, when third party parsers reject it.
* Confusing, because the `+` sign is neither documented, nor can it be assumed to be present.
Fix all issues by removing the legacy int parsing.
(https://github.com/bitcoin/bitcoin/pull/32520)
The legacy int parsing is problematic, because it accepts the `+` sign for unsigned integers. In all cases this is either:
* Useless, because the `+` sign was already rejected.
* Erroneous, when third party parsers reject it.
* Confusing, because the `+` sign is neither documented, nor can it be assumed to be present.
Fix all issues by removing the legacy int parsing.
π¬ achow101 commented on issue "listunspent, fundrawtransaction, getwalletinfo locks wallet for any other operation":
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2884937158)
> I've suggested before just having a second map of only unspents,
Then you should be interested in reviewing #27865 and its prerequisite #27286
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2884937158)
> I've suggested before just having a second map of only unspents,
Then you should be interested in reviewing #27865 and its prerequisite #27286
π¬ w0xlt commented on pull request "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`":
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2884949100)
`std::string_view` is known (and usable) at compile time as a view over some static data (it can be constexpr), so it can be concatenated to RPC docs. [1]
`std::string` no, itβs always a runtime, owning, heapβallocated string, so it wonΒ΄t work to produce the RPC docs.
[1] https://stackoverflow.com/a/77358740
[2] https://en.cppreference.com/w/cpp/string/basic_string_view
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2884949100)
`std::string_view` is known (and usable) at compile time as a view over some static data (it can be constexpr), so it can be concatenated to RPC docs. [1]
`std::string` no, itβs always a runtime, owning, heapβallocated string, so it wonΒ΄t work to produce the RPC docs.
[1] https://stackoverflow.com/a/77358740
[2] https://en.cppreference.com/w/cpp/string/basic_string_view
π€ furszy reviewed a pull request: "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors"
(https://github.com/bitcoin/bitcoin/pull/32475#pullrequestreview-2844875076)
Code review ACK f17945b347f6a46dee3b56f86a557eaccec1bc72
(https://github.com/bitcoin/bitcoin/pull/32475#pullrequestreview-2844875076)
Code review ACK f17945b347f6a46dee3b56f86a557eaccec1bc72
π¬ hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091886628)
https://github.com/bitcoin/bitcoin/blob/7af6e1089ea264e870b26ac83e81e7aa374acbe1/src/bitcoin.cpp#L76-L83
Note that some RPCs take Base64 parameters (finalizepsbt, verifymessage), and Base64 is commonly padded with `=`.
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091886628)
https://github.com/bitcoin/bitcoin/blob/7af6e1089ea264e870b26ac83e81e7aa374acbe1/src/bitcoin.cpp#L76-L83
Note that some RPCs take Base64 parameters (finalizepsbt, verifymessage), and Base64 is commonly padded with `=`.
π¬ amg1127 commented on issue ""rpcallowip=" configuration directive doesn't accept RFC4193 addresses":
(https://github.com/bitcoin/bitcoin/issues/32433#issuecomment-2884958889)
Thank you for evaluating my bug report.
I had never heard of CJDNS before. On the day I was creating this bug report, I initially thought that the root cause of the issue was an issue with the IPv6 address parser, and got surprised when I found that `bitcoind` accepted a RFC3849 address.
In my opinion, both the error message and the parameter documentation presented by `bitcoind --help` command line should state that CJDNS addresses ([`[fc00::/8]` block](https://github.com/cjdelisle/cjdns/blob
...
(https://github.com/bitcoin/bitcoin/issues/32433#issuecomment-2884958889)
Thank you for evaluating my bug report.
I had never heard of CJDNS before. On the day I was creating this bug report, I initially thought that the root cause of the issue was an issue with the IPv6 address parser, and got surprised when I found that `bitcoind` accepted a RFC3849 address.
In my opinion, both the error message and the parameter documentation presented by `bitcoind --help` command line should state that CJDNS addresses ([`[fc00::/8]` block](https://github.com/cjdelisle/cjdns/blob
...
π¬ darosior commented on pull request "init: drop `-upnp`":
(https://github.com/bitcoin/bitcoin/pull/32500#issuecomment-2884959221)
> if this reintroduces the `settings.json` error, I can also see waiting one more release
An unknown `upnp` entry in `settings.json` would just be ignored? A `upnp` entry in the config file would cause a startup failure. Concept ACK on this basis.
(https://github.com/bitcoin/bitcoin/pull/32500#issuecomment-2884959221)
> if this reintroduces the `settings.json` error, I can also see waiting one more release
An unknown `upnp` entry in `settings.json` would just be ignored? A `upnp` entry in the config file would cause a startup failure. Concept ACK on this basis.
π¬ davidgumberg commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#discussion_r2091916322)
Question: Where do these version numbers come from?
(https://github.com/bitcoin/bitcoin/pull/32499#discussion_r2091916322)
Question: Where do these version numbers come from?
π darosior approved a pull request: "init: drop `-upnp`"
(https://github.com/bitcoin/bitcoin/pull/32500#pullrequestreview-2844910044)
> An unknown upnp entry in settings.json would just be ignored?
Confirmed.
tACK 301993ebf7f8ec23050e91377e0fd05823bb372a
(https://github.com/bitcoin/bitcoin/pull/32500#pullrequestreview-2844910044)
> An unknown upnp entry in settings.json would just be ignored?
Confirmed.
tACK 301993ebf7f8ec23050e91377e0fd05823bb372a
π¬ davidgumberg commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#discussion_r2091919391)
Answer: There is a table on this page https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170#version-macros
(https://github.com/bitcoin/bitcoin/pull/32499#discussion_r2091919391)
Answer: There is a table on this page https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170#version-macros