📝 fanquake locked a pull request: "i deleted a commented line in invalid_signer.py file"
(https://github.com/bitcoin/bitcoin/pull/31914)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31914)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 dergoegge commented on issue "build: x86 ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2671770232)
> Does -O0 even work with -DUSE_ASM_X86_64=1, absent asan?
Looks like it.
> You can also set -O1 instead of -O0, but I guess you don't want that?
Just re-read your message, isn't `-O2` our default? i.e. I don't think I was using `-O0` when I first saw this.
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2671770232)
> Does -O0 even work with -DUSE_ASM_X86_64=1, absent asan?
Looks like it.
> You can also set -O1 instead of -O0, but I guess you don't want that?
Just re-read your message, isn't `-O2` our default? i.e. I don't think I was using `-O0` when I first saw this.
💬 rkrux commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1963746974)
This is an expected error from the underlying RPC call. Although using `coin` here in the assertion won't fail the functional test but I don't think this makes any meaningful difference.
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1963746974)
This is an expected error from the underlying RPC call. Although using `coin` here in the assertion won't fail the functional test but I don't think this makes any meaningful difference.
📝 stephenmiracle4 opened a pull request: "i deleted a commented line in invalid_signer.py file"
(https://github.com/bitcoin/bitcoin/pull/31915)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31915)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ fanquake closed a pull request: "i deleted a commented line in invalid_signer.py file"
(https://github.com/bitcoin/bitcoin/pull/31915)
(https://github.com/bitcoin/bitcoin/pull/31915)
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31915)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31915)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ fanquake closed a pull request: "ci: switch MSAN to use prebuilt Clang binaries"
(https://github.com/bitcoin/bitcoin/pull/31850)
(https://github.com/bitcoin/bitcoin/pull/31850)
💬 fanquake commented on pull request "ci: switch MSAN to use prebuilt Clang binaries":
(https://github.com/bitcoin/bitcoin/pull/31850#issuecomment-2671801172)
Yea, I don't think so. We can circle back around to this soon.
(https://github.com/bitcoin/bitcoin/pull/31850#issuecomment-2671801172)
Yea, I don't think so. We can circle back around to this soon.
💬 l0rinc commented on pull request "test: cover base[32|58|64] with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1963776373)
> issues.oss-fuzz.com/issues/397734700](https://issues.oss-fuzz.com/issues/397734700:
`Access is denied to this issue`
> Looking into this. Was there any reason to change max_ret_len from 100 to 1000?
The scenarios when `maxlength` doesn't restrict `random_string` seems like the more useful case. Given that the actual value was capped at `1000` already, I have aligned it to that.
Also, @brunoerg extended 100 to 300 in https://github.com/bitcoin/bitcoin/pull/31577/files#diff-c4025879a0b
...
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1963776373)
> issues.oss-fuzz.com/issues/397734700](https://issues.oss-fuzz.com/issues/397734700:
`Access is denied to this issue`
> Looking into this. Was there any reason to change max_ret_len from 100 to 1000?
The scenarios when `maxlength` doesn't restrict `random_string` seems like the more useful case. Given that the actual value was capped at `1000` already, I have aligned it to that.
Also, @brunoerg extended 100 to 300 in https://github.com/bitcoin/bitcoin/pull/31577/files#diff-c4025879a0b
...
✅ fanquake closed a pull request: "rpc: add address_type field in getaddressinfo"
(https://github.com/bitcoin/bitcoin/pull/30727)
(https://github.com/bitcoin/bitcoin/pull/30727)
💬 fanquake commented on pull request "rpc: add address_type field in getaddressinfo":
(https://github.com/bitcoin/bitcoin/pull/30727#issuecomment-2671823879)
Closing for now, due to no/lack of followup. Please leave a comment, if you want this reopened.
(https://github.com/bitcoin/bitcoin/pull/30727#issuecomment-2671823879)
Closing for now, due to no/lack of followup. Please leave a comment, if you want this reopened.
💬 darosior commented on pull request "qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data":
(https://github.com/bitcoin/bitcoin/pull/31910#discussion_r1963788069)
Nevertheless this is not obvious and Niklas' suggestion of having it in the initialization of the fuzz target makes sense so i just did that too.
(https://github.com/bitcoin/bitcoin/pull/31910#discussion_r1963788069)
Nevertheless this is not obvious and Niklas' suggestion of having it in the initialization of the fuzz target makes sense so i just did that too.
✅ fanquake closed a pull request: "test: cover edge case for lockunspent rpc"
(https://github.com/bitcoin/bitcoin/pull/31209)
(https://github.com/bitcoin/bitcoin/pull/31209)
💬 fanquake commented on pull request "test: cover edge case for lockunspent rpc":
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2671825254)
Closing for now. Doesn't seem to be agreement on the change.
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2671825254)
Closing for now. Doesn't seem to be agreement on the change.
🤔 vasild reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2629536362)
Approach ACK 185c6a416df679ddc7b38750aa4d181c0b043d5b
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2629536362)
Approach ACK 185c6a416df679ddc7b38750aa4d181c0b043d5b
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963378155)
```suggestion
//! Return path to argv0.
fs::path GetExePath(std::string_view argv0);
```
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963378155)
```suggestion
//! Return path to argv0.
fs::path GetExePath(std::string_view argv0);
```
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963540791)
Some leftovers, I guess from previous incarnation of this which did not use `R"(...)"`:
```suggestion
-m, --multiprocess Run multiprocess binaries bitcoin-node, bitcoin-gui.
-M, --monolithic Run monolithic binaries bitcoind, bitcoin-qt. (Default behavior)
```
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963540791)
Some leftovers, I guess from previous incarnation of this which did not use `R"(...)"`:
```suggestion
-m, --multiprocess Run multiprocess binaries bitcoin-node, bitcoin-gui.
-M, --monolithic Run monolithic binaries bitcoind, bitcoin-qt. (Default behavior)
```
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963537775)
`tx` is missing and `test` is part of the internal commands, so:
```suggestion
{gui,daemon,rpc,wallet,tx,help}
```
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963537775)
`tx` is missing and `test` is part of the internal commands, so:
```suggestion
{gui,daemon,rpc,wallet,tx,help}
```
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963523131)
nit: unused include
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963523131)
nit: unused include
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963558283)
This prints "bitcoin gui". One might get the impression that the command is "bitcoin gui", whereas it is just "gui". For comparison:
```
$ git help
usage: git ... <command> [<args>]
...
clone Clone a repository into a new directory
init Create an empty Git repository or reinitialize an existing one
...
```
So maybe:
```suggestion
gui [ARGS] Start GUI, equivalent to running 'bitcoin-qt [ARGS]' or 'bitcoin-gui [ARGS]'.
```
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963558283)
This prints "bitcoin gui". One might get the impression that the command is "bitcoin gui", whereas it is just "gui". For comparison:
```
$ git help
usage: git ... <command> [<args>]
...
clone Clone a repository into a new directory
init Create an empty Git repository or reinitialize an existing one
...
```
So maybe:
```suggestion
gui [ARGS] Start GUI, equivalent to running 'bitcoin-qt [ARGS]' or 'bitcoin-gui [ARGS]'.
```