💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577947487)
Ok, marking this as resolved
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577947487)
Ok, marking this as resolved
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577949702)
Added a similar test
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577949702)
Added a similar test
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577950806)
Added
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577950806)
Added
💬 hebasto commented on pull request "refactor: Rename `subprocess.hpp` to follow our header name conventions":
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2075053593)
> > Resolved linter warnings.
>
> Can you mention this in the PR description?
Thanks! The PR description has been updated.
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2075053593)
> > Resolved linter warnings.
>
> Can you mention this in the PR description?
Thanks! The PR description has been updated.
👍 instagibbs approved a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2020006914)
ACK https://github.com/bitcoin/bitcoin/pull/28970/commits/30c9e6bc4e9f8b8b606e55435dc3f743cb2dd670
Changes are test only, and new subtest causes a debug-build crash when the `PCKG_POLICY` change is reverted.
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2020006914)
ACK https://github.com/bitcoin/bitcoin/pull/28970/commits/30c9e6bc4e9f8b8b606e55435dc3f743cb2dd670
Changes are test only, and new subtest causes a debug-build crash when the `PCKG_POLICY` change is reverted.
💬 iotamega commented on issue "Crashes in v27 on Windows 10 and 11":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2075063698)
Hate to be the deliverer of bad news but I am also experiencing this issue on my Ubuntu installations after extensive testing.
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2075063698)
Hate to be the deliverer of bad news but I am also experiencing this issue on my Ubuntu installations after extensive testing.
💬 maflcko commented on issue "Manually Banning Peers Results in Crash":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2075073807)
On Ubuntu it should be trivial to run in gdb, can you try that please?
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2075073807)
On Ubuntu it should be trivial to run in gdb, can you try that please?
👍 TheCharlatan approved a pull request: "refactor: Rename `subprocess.hpp` to follow our header name conventions"
(https://github.com/bitcoin/bitcoin/pull/29910#pullrequestreview-2020030582)
ACK 08f756bd370c3e100b186c7e24bef6a033575b29
(https://github.com/bitcoin/bitcoin/pull/29910#pullrequestreview-2020030582)
ACK 08f756bd370c3e100b186c7e24bef6a033575b29
📝 hebasto opened a pull request: "doc: Bash is needed in gen_id and is not installed on FreeBSD by default"
(https://github.com/bitcoin/bitcoin/pull/29953)
On FreeBSD 14.0, in the `depends` directory:
- without `bash`:
```
$ gmake print-bdb_build_id_long
env: bash: No such file or directory
env: bash: No such file or directory
bdb_build_id_long=bdb-4.8.30-4b0c6f8e95251b9c6731844fc34111c04b75fd9f15c671d6e34f2a4d014ec1be-release
$ gmake print-final_build_id
env: bash: No such file or directory
env: bash: No such file or directory
final_build_id=722b2d3e264
```
- with `bash`:
```
$ gmake print-bdb_build_id_long
bdb_build_id_long=
...
(https://github.com/bitcoin/bitcoin/pull/29953)
On FreeBSD 14.0, in the `depends` directory:
- without `bash`:
```
$ gmake print-bdb_build_id_long
env: bash: No such file or directory
env: bash: No such file or directory
bdb_build_id_long=bdb-4.8.30-4b0c6f8e95251b9c6731844fc34111c04b75fd9f15c671d6e34f2a4d014ec1be-release
$ gmake print-final_build_id
env: bash: No such file or directory
env: bash: No such file or directory
final_build_id=722b2d3e264
```
- with `bash`:
```
$ gmake print-bdb_build_id_long
bdb_build_id_long=
...
💬 hebasto commented on pull request "doc: Bash is needed in gen_id and is not installed on FreeBSD by default":
(https://github.com/bitcoin/bitcoin/pull/29953#issuecomment-2075088822)
cc @vasild @fanquake
(https://github.com/bitcoin/bitcoin/pull/29953#issuecomment-2075088822)
cc @vasild @fanquake
💬 fanquake commented on pull request "doc: Bash is needed in gen_id and is not installed on FreeBSD by default":
(https://github.com/bitcoin/bitcoin/pull/29953#issuecomment-2075094287)
This should be added to the depends docs.
(https://github.com/bitcoin/bitcoin/pull/29953#issuecomment-2075094287)
This should be added to the depends docs.
💬 hebasto commented on pull request "doc: Bash is needed in gen_id and is not installed on FreeBSD by default":
(https://github.com/bitcoin/bitcoin/pull/29953#issuecomment-2075104778)
> This should be added to the depends docs.
Thanks! Added.
(https://github.com/bitcoin/bitcoin/pull/29953#issuecomment-2075104778)
> This should be added to the depends docs.
Thanks! Added.
💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578006194)
Hm, ok, that's another behavior change I didn't anticipate. Let me first describe my understanding of happens: In [the libevent version](https://github.com/libevent/libevent/blob/e0a4574ba2cbcdb64bb2b593e72be7f7f4010746/http.c#L3492) each of the two characters following the `%` are [looked at individually if they are valid hex characters](https://github.com/libevent/libevent/blob/e0a4574ba2cbcdb64bb2b593e72be7f7f4010746/http.c#L3508). The space is not so the if fails and the fallback case takes
...
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578006194)
Hm, ok, that's another behavior change I didn't anticipate. Let me first describe my understanding of happens: In [the libevent version](https://github.com/libevent/libevent/blob/e0a4574ba2cbcdb64bb2b593e72be7f7f4010746/http.c#L3492) each of the two characters following the `%` are [looked at individually if they are valid hex characters](https://github.com/libevent/libevent/blob/e0a4574ba2cbcdb64bb2b593e72be7f7f4010746/http.c#L3508). The space is not so the if fails and the fallback case takes
...
💬 fanquake commented on pull request "doc: Bash is needed in gen_id and is not installed on FreeBSD by default":
(https://github.com/bitcoin/bitcoin/pull/29953#discussion_r1578008214)
So now it can be removed here.
(https://github.com/bitcoin/bitcoin/pull/29953#discussion_r1578008214)
So now it can be removed here.
💬 hebasto commented on pull request "doc: Bash is needed in gen_id and is not installed on FreeBSD by default":
(https://github.com/bitcoin/bitcoin/pull/29953#discussion_r1578012276)
Sure. Removed.
(https://github.com/bitcoin/bitcoin/pull/29953#discussion_r1578012276)
Sure. Removed.
💬 achow101 commented on issue "Calling `walletprocesspsbt` to sign multisig containing `OP_GREATERTHANOREQUAL` op_code":
(https://github.com/bitcoin/bitcoin/issues/29949#issuecomment-2075118310)
Bitcoin Core cannot sign non-standard scripts or scripts that are not Miniscript. This is expected behavior. `walletprocesspsbt` does not return an error because signing is not the only thing that it does, so returning an error for a failing to sign is not appropriate.
(https://github.com/bitcoin/bitcoin/issues/29949#issuecomment-2075118310)
Bitcoin Core cannot sign non-standard scripts or scripts that are not Miniscript. This is expected behavior. `walletprocesspsbt` does not return an error because signing is not the only thing that it does, so returning an error for a failing to sign is not appropriate.
💬 maflcko commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578019052)
The hex checking is done inside of from_chars (locale independent), so you can re-use that. Something like this may work:
```suggestion
if (c == '%' && i + 2 < url_encoded.size()
) {
int decoded_value{0};
auto [p, ec] = std::from_chars(url_encoded.data() + i + 1, url_encoded.data() + i + 3, decoded_value, 16);
if(ec == std::errc{} && p==url_encoded.data() + i + 3) { // hex
res += static_cast
...
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578019052)
The hex checking is done inside of from_chars (locale independent), so you can re-use that. Something like this may work:
```suggestion
if (c == '%' && i + 2 < url_encoded.size()
) {
int decoded_value{0};
auto [p, ec] = std::from_chars(url_encoded.data() + i + 1, url_encoded.data() + i + 3, decoded_value, 16);
if(ec == std::errc{} && p==url_encoded.data() + i + 3) { // hex
res += static_cast
...
🚀 fanquake merged a pull request: "ci: disable `_FORTIFY_SOURCE` with MSAN"
(https://github.com/bitcoin/bitcoin/pull/29837)
(https://github.com/bitcoin/bitcoin/pull/29837)
💬 sipa commented on issue "Calling `walletprocesspsbt` to sign multisig containing `OP_GREATERTHANOREQUAL` op_code":
(https://github.com/bitcoin/bitcoin/issues/29949#issuecomment-2075135656)
For some more background, Bitcoin Core has logic to try various things when signing, which includes Miniscript. The script variant you tried with `OP_NUMEQUAL` at the end is valid Miniscript (`multi_a(2,<key1>,<key2>,<key3>)` specifically), but the variant with `OP_GREATERTHANOREQUAL` is not.
When signing fails, Bitcoin Core doesn't know why it failed; it simply didn't find any patterns it knew how to sign for, and as @achow101 points out, it doesn't even know your intent was to sign.
(https://github.com/bitcoin/bitcoin/issues/29949#issuecomment-2075135656)
For some more background, Bitcoin Core has logic to try various things when signing, which includes Miniscript. The script variant you tried with `OP_NUMEQUAL` at the end is valid Miniscript (`multi_a(2,<key1>,<key2>,<key3>)` specifically), but the variant with `OP_GREATERTHANOREQUAL` is not.
When signing fails, Bitcoin Core doesn't know why it failed; it simply didn't find any patterns it knew how to sign for, and as @achow101 points out, it doesn't even know your intent was to sign.
🚀 fanquake merged a pull request: "refactor: Rename `subprocess.hpp` to follow our header name conventions"
(https://github.com/bitcoin/bitcoin/pull/29910)
(https://github.com/bitcoin/bitcoin/pull/29910)