💬 hanmz commented on pull request "Fix typos in description.md and wallet_util.py":
(https://github.com/bitcoin/bitcoin/pull/29938#issuecomment-2076626155)
> Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

OK DONE

💬 laanwj commented on pull request "guiconstants: update ORG_DOMAIN to bitcoincore.org":
(https://github.com/bitcoin-core/gui/pull/818#issuecomment-2076626737)
Right. i think qt-qml does something substantially different with regard to settings, so it may be warranted there.

💬 maflcko commented on pull request "Fix typos in description.md and wallet_util.py":
(https://github.com/bitcoin/bitcoin/pull/29938#issuecomment-2076650860)
ACK 03e36b3da093e2c23cf51b46f6901cb84ddbf867

Please add the `doc: ` prefix to the title

💬 paplorinc commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1579103153)
thanks for checking

💬 hebasto commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2076675361)
> This is a proof-of-concept of using the same methodology as in #29923 to support both Wayland and X11 windowing backends, for the Linux release binary, without any extra build-time nor fixed run-time dependencies.

Concept ACK on this.

Obviously :)

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579111090)
I didn't see compiler errors locally and the other CI jobs also didn't seem to fail on compilation. So I assumed clang does some implicit conversion at that point and MSVC doesn't.

👍 kristapsk approved a pull request: "Fix typos in description.md and wallet_util.py"
(https://github.com/bitcoin/bitcoin/pull/29938#pullrequestreview-2021917530)
ACK 03e36b3da093e2c23cf51b46f6901cb84ddbf867

👍 kristapsk approved a pull request: "lint: scripted-diff verification also requires GNU grep"
(https://github.com/bitcoin/bitcoin/pull/29689#pullrequestreview-2021925047)
cr utACK 3bf4f8db669e1e274ce2633cf84add2938b9914b

💬 maflcko commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579125570)
Yes, IIRC a standard library is free to define an iterator as a pointer, or allow iterators to convert to pointers implicitly, but there is no requirement for it do so.

👍 theStack approved a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2021985089)
Code-review ACK 992c714451676cee33d3dff49f36329423270c1c

💬 m3dwards commented on pull request "depends: build libnatpmp with CMake":
(https://github.com/bitcoin/bitcoin/pull/29708#issuecomment-2076797188)
My Guix builds:

```shell
a134241bc9ff9823ce078ea10dbac544ff63536ee3f66fb557414ca191b1d62d guix-build-3c1ae3ee33d4/output/aarch64-linux-gnu/SHA256SUMS.part
f5a46ab7a5faae9ce50b2d084cf65d14372b354b9f8369178f1348b70a3b675b guix-build-3c1ae3ee33d4/output/aarch64-linux-gnu/bitcoin-3c1ae3ee33d4-aarch64-linux-gnu-debug.tar.gz
e2353ed4f0738026fe16cd90b503ef2f02f31e0b68ba42ea80924997225ae374 guix-build-3c1ae3ee33d4/output/aarch64-linux-gnu/bitcoin-3c1ae3ee33d4-aarch64-linux-gnu.tar.gz
780608996
...

💬 Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2076802405)
I was able to build `bitcoind` (didn't try QT) with `llvm@17`, but _not_ with the current `llvm` 18.1.4. See log: https://gist.github.com/Sjors/194eb5d67e2e1132df7ff86f103ba16b

It's probably fine to drop `@17` anyway, and maybe suggest trying lower versions if building fails. But it would be nice if there's a simple fix for this.

💬 vasild commented on issue "Error: Cannot resolve -bind address: 'bitcoind:8334=onion'":
(https://github.com/bitcoin/bitcoin/issues/26484#issuecomment-2076815988)
Yes, line `1841` is for when there is no `=` inside the argument, like in `-bind=1.2.3.4:8333`. Line `1853` is for when there is `=`, like `-bind=1.2.3.4:8333=onion`.

👍 maflcko approved a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2022081812)
Did some differential fuzzing, as this is an ideal fit for it.

ACK 992c714451676cee33d3dff49f36329423270c1c 👈

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4
...

💬 maflcko commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579216072)
Some more tests:

`%%ffg`
`%fg`

🤔 paplorinc reviewed a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2022089197)
I think we should document at least that we thought of the `%00` corner case, e.g. via a test case or a change.

I have verified that the behavior is otherwise the same before and after via:
```
std::string GenerateRandomString() {
std::random_device rd;
std::mt19937 gen(rd());
std::uniform_int_distribution<> length_dis(1, 10);
std::uniform_int_distribution<> char_dis(1, 255);

size_t length = length_dis(gen);
std::string res;
res.reserve(length);

f
...

💬 paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579220761)
There's still one sneaky difference that I've found: `%00`.
Previously this prematurely terminated the string, now it's kept:
```C++
BOOST_CHECK_EQUAL(urlDecode("a%00b"), std::string("a\u0000b", 3));
```
this failed previously, passes now.

💬 maflcko commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579230097)
> `%00`

Have you seen 992c714451676cee33d3dff49f36329423270c1c ?

💬 paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579232922)
Yes https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1572090699

💬 paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579235693)
Those cases were added since, I see, thanks.