💬 hebasto commented on pull request "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`":
(https://github.com/bitcoin/bitcoin/pull/27717#issuecomment-1558981977)
As some progress is being made in the review of the coupled https://github.com/hebasto/bitcoin/pull/15, friendly ping @MarcoFalke @stickies-v @ryanofsky :)
(https://github.com/bitcoin/bitcoin/pull/27717#issuecomment-1558981977)
As some progress is being made in the review of the coupled https://github.com/hebasto/bitcoin/pull/15, friendly ping @MarcoFalke @stickies-v @ryanofsky :)
💬 MarcoFalke commented on pull request "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`":
(https://github.com/bitcoin/bitcoin/pull/27717#issuecomment-1559014932)
lgtm ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
(https://github.com/bitcoin/bitcoin/pull/27717#issuecomment-1559014932)
lgtm ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
💬 hebasto commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-1559046252)
This change ignores non-main network data. I'm OK with that. But it seems worth mentioning in the docs.
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-1559046252)
This change ignores non-main network data. I'm OK with that. But it seems worth mentioning in the docs.
💬 fanquake commented on pull request "depends: remove redundant stdlib option":
(https://github.com/bitcoin/bitcoin/pull/27721#issuecomment-1559052397)
Guix Build:
```bash
ed1f83a77d00:/bitcoin# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
b2fc48fff5ef2c15a520582845a8af73c2af71f4f59a0d0b92b04db8530373b0 guix-build-4fe5f3c46752/output/aarch64-linux-gnu/SHA256SUMS.part
d074ce3bdc89db1d7d0e4fcbde818b9a2183c9a23541e2dabcb2401e88635959 guix-build-4fe5f3c46752/output/aarch64-linux-gnu/bitcoin-4fe5f3c46752-aarch64-linux-gnu-debug.tar.gz
890d8535d8590ceb5d17b0cbd254c4c17e37
...
(https://github.com/bitcoin/bitcoin/pull/27721#issuecomment-1559052397)
Guix Build:
```bash
ed1f83a77d00:/bitcoin# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
b2fc48fff5ef2c15a520582845a8af73c2af71f4f59a0d0b92b04db8530373b0 guix-build-4fe5f3c46752/output/aarch64-linux-gnu/SHA256SUMS.part
d074ce3bdc89db1d7d0e4fcbde818b9a2183c9a23541e2dabcb2401e88635959 guix-build-4fe5f3c46752/output/aarch64-linux-gnu/bitcoin-4fe5f3c46752-aarch64-linux-gnu-debug.tar.gz
890d8535d8590ceb5d17b0cbd254c4c17e37
...
💬 willcl-ark commented on issue "bitcoin core crashes when too many rpc calls are made":
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-1559055029)
2.2.1-alpha is [now out](https://github.com/libevent/libevent/releases/tag/release-2.2.1-alpha) 👀
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-1559055029)
2.2.1-alpha is [now out](https://github.com/libevent/libevent/releases/tag/release-2.2.1-alpha) 👀
💬 willcl-ark commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1202074906)
Right.
Do you think it's best to add to _00_setup_env_native_fuzz.sh_ ?
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1202074906)
Right.
Do you think it's best to add to _00_setup_env_native_fuzz.sh_ ?
👍 TheCharlatan approved a pull request: "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`"
(https://github.com/bitcoin/bitcoin/pull/27717#pullrequestreview-1439386025)
ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
The error reporting in this file is a bit opaque imo, but that is orthogonal to the work here.
(https://github.com/bitcoin/bitcoin/pull/27717#pullrequestreview-1439386025)
ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
The error reporting in this file is a bit opaque imo, but that is orthogonal to the work here.
👍 stickies-v approved a pull request: "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`"
(https://github.com/bitcoin/bitcoin/pull/27717#pullrequestreview-1439388072)
ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
(https://github.com/bitcoin/bitcoin/pull/27717#pullrequestreview-1439388072)
ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
📝 MarcoFalke opened a pull request: "rpc: Fix invalid bech32 handling"
(https://github.com/bitcoin/bitcoin/pull/27727)
Currently the handling of invalid bech32(m) addresses over RPC has many issues:
* No error for invalid addresses is reported, leading to internal bugs via `CHECK_NONFATAL`, see https://github.com/bitcoin/bitcoin/issues/27723
* The error messages use "data size" (the meaning of which is unclear to the user, because it the program data and data data are different) when they mean "program size"
Fix all issues. Also, use the BIP 173 and BIP 350 test vectors.
(https://github.com/bitcoin/bitcoin/pull/27727)
Currently the handling of invalid bech32(m) addresses over RPC has many issues:
* No error for invalid addresses is reported, leading to internal bugs via `CHECK_NONFATAL`, see https://github.com/bitcoin/bitcoin/issues/27723
* The error messages use "data size" (the meaning of which is unclear to the user, because it the program data and data data are different) when they mean "program size"
Fix all issues. Also, use the BIP 173 and BIP 350 test vectors.
💬 MarcoFalke commented on issue "Validation of malformed address fails with a peculiar message":
(https://github.com/bitcoin/bitcoin/issues/27723#issuecomment-1559080308)
Fun fact, this also happens with the BIP 173 and BIP 350 test vectors. Proposed fix in #https://github.com/bitcoin/bitcoin/pull/27727
(https://github.com/bitcoin/bitcoin/issues/27723#issuecomment-1559080308)
Fun fact, this also happens with the BIP 173 and BIP 350 test vectors. Proposed fix in #https://github.com/bitcoin/bitcoin/pull/27727
💬 MarcoFalke commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1202114591)
Oh, I meant ` 00_setup_env_native_fuzz_debug.sh`, but I see that this is maintained elsewhere. Maybe it can be added to just fuzz_msan, or just no fuzz task at all in this repo?
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1202114591)
Oh, I meant ` 00_setup_env_native_fuzz_debug.sh`, but I see that this is maintained elsewhere. Maybe it can be added to just fuzz_msan, or just no fuzz task at all in this repo?
💬 MarcoFalke commented on pull request "rpc: Fix invalid bech32 handling":
(https://github.com/bitcoin/bitcoin/pull/27727#issuecomment-1559099257)
(I'd say backport isn't blocking any release, but if a previous release is done anyway for other reasons, it may be good to backport)
(https://github.com/bitcoin/bitcoin/pull/27727#issuecomment-1559099257)
(I'd say backport isn't blocking any release, but if a previous release is done anyway for other reasons, it may be good to backport)
💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1202123731)
I agree that harder to debug test failure messages are to be avoided, but given the nature of the statement, I also don't mind the current approach to avoid having to create a separate var for the `optional` and then the `CNetAddr`, even if it's just the one line.
It's a shame the `BOOST_` functions don't support pass-through of return values, which would have solved this elegantly. We could use our `Assert` function (requires `#include <util/check.h>`), which yields an equally useful error
...
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1202123731)
I agree that harder to debug test failure messages are to be avoided, but given the nature of the statement, I also don't mind the current approach to avoid having to create a separate var for the `optional` and then the `CNetAddr`, even if it's just the one line.
It's a shame the `BOOST_` functions don't support pass-through of return values, which would have solved this elegantly. We could use our `Assert` function (requires `#include <util/check.h>`), which yields an equally useful error
...
💬 ajtowns commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1202129704)
We could; it'd mean adding an extra field to the mempool, and using both mockable time and steady time for dealing with next invs (mockable time so we can skip over delays in sending invs; steady time for this purpose).
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1202129704)
We could; it'd mean adding an extra field to the mempool, and using both mockable time and steady time for dealing with next invs (mockable time so we can skip over delays in sending invs; steady time for this purpose).
👍 MarcoFalke approved a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1439540106)
can squash to avoid touching the same line twice?
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1439540106)
can squash to avoid touching the same line twice?
💬 hebasto commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1202205772)
Why this change?
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1202205772)
Why this change?
💬 MarcoFalke commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1202222611)
Might have to use `LossyChronoFormatter` to avoid the fuzz crash?
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1202222611)
Might have to use `LossyChronoFormatter` to avoid the fuzz crash?
🚀 fanquake merged a pull request: "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`"
(https://github.com/bitcoin/bitcoin/pull/27717)
(https://github.com/bitcoin/bitcoin/pull/27717)
💬 willcl-ark commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1559240120)
> can squash to avoid touching the same line twice?
Now squashed :)
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1559240120)
> can squash to avoid touching the same line twice?
Now squashed :)
👍 hebasto approved a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1439642762)
ACK 59c89447499bd9d6202269879555b8bc37373aa2
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1439642762)
ACK 59c89447499bd9d6202269879555b8bc37373aa2