💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902893)
> Thanks for adding me as a co-author, if you force push again, please change the email to `Lőrinc <pap.lorinc@gmail.com>`

fixed

💬 paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902969)
Not exactly: https://github.com/libevent/libevent/blob/release-2.1.12-stable/http.c#L3186-L3188

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573904156)
Hm, the code you are linking to is part of how libevent handles the `+`. Or did you want to refer to `?`? I think if it's not very common for `&` to be decoded as space then we don't need to mention it.

💬 paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573905184)
? seems to be a control char, `If -1, when true we transform plus to space only after we've seen a ?. -1 is deprecated.`, no need to mention it, thanks for checking.

💬 asctime commented on issue "Crash on fail ~CCheckQueue() destructor?":
(https://github.com/bitcoin/bitcoin/issues/29930#issuecomment-2068196611)
Any setting, or just clicking ok (cancel is fine). It's probably something slightly different in MinGW pthread support.
so assert(m_worker_threads.empty()); causing a crash still analyzing.

So I understand assert will tell us what part of the code crashed, but it doesn't really tell us anything about the worker that wont stop, (thread id etc) so far it's only the settings box -> everything else seems to work after #29931 fingers crossed. gui syncing now.

💬 ismaelsadeeq commented on pull request "test: Refactor fee calculation to remove satoshi_round function":
(https://github.com/bitcoin/bitcoin/pull/29566#issuecomment-2068218318)
The C.I failure here is due to unused import @naiyoma
```
{'-zmqpubhashblock', '-zmqpubsequencehwm', '-zmqpubsequence', '-zmqpubrawblock', '-zmqpubhashtxhwm', '-zmqpubhashblockhwm', '-includeconf', '-zmqpubhashtx', '-zmqpubrawtx', '-testdatadir', '-zmqpubrawblockhwm', '-zmqpubrawtxhwm'}
test/functional/test_framework/util.py:8:1: F401 'decimal.ROUND_DOWN' imported but unused
^---- failure generated from lint-python.py

^---- ⚠️ Failure generated from lint-*.py scripts!
```

💬 luke-jr commented on pull request "Don't permit port in proxy IP option":
(https://github.com/bitcoin-core/gui/pull/813#issuecomment-2068230372)
@jarolrod Isn't that a bug independent from the validation one?

💬 fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943040)
done

💬 fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943357)
I added a counter and check it at the end

💬 fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943413)
fixed

💬 fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943494)
renamed

💬 fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943566)
leftover from a previous approach, removed

💬 fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2068231606)
So far I have addressed the minor feedback in the latest push and I am now working on the extended metadata for the snapshot. I am currently planning to add the following:

1. magic bytes - the ones suggested by @Sjors
2. snapshot version
3. Network magic (pchMessageStart) - This lets us distinguish between the different networks and since it’s generated from the challenge for signets, we can deduce that it’s probably a custom signet if we don’t recognize them and give an appropriate feedba
...

🤔 luke-jr requested changes to a pull request: "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection"
(https://github.com/bitcoin-core/gui/pull/815#pullrequestreview-2013545536)
This feels like the wrong place to fix it. Why not inside `setWalletActionsEnabled` (or whatever is enabling it to begin with)?

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-2068240859)
My point was more that we should probably not be making cookie files read-only to begin with. So only `rw-r--r--`, `rw-r-----`, and `rw-------` should be supported. Bringing us back to this maybe being best as `-rpccookieperms=user/group/all` rather than an octal modeset.

And then how to handle if the cookie file already exists read-only... it *might* make sense to just use it unmodified in that scenario?

🤔 mzumsande reviewed a pull request: "net: Decrease nMaxIPs when learning from DNS seeds"
(https://github.com/bitcoin/bitcoin/pull/29850#pullrequestreview-2013547652)
utACK f2e3662e57eca1330962faf38ff428a564d50a11

💬 laanwj commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2068586717)
One important thing to test here is that the resulting binaries work on a range of different Linux distributions (also older ones that people still care about).

There's no inherent reason to assume this will make compatibility worse, but it currently loads *all* symbols from the headers for the respective libraries (https://github.com/laanwj/qtsowrap?tab=readme-ov-file#versions) which might enforce the lower bound on version stronger than used to be done.

Might consider downgrading the he
...

💬 hebasto commented on pull request "test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/pull/29907#issuecomment-2068726017)
Friendly ping @theuni @sipa @fanquake :)

💬 maflcko commented on issue "ReadAnchor throws exception on second run":
(https://github.com/bitcoin/bitcoin/issues/29931#issuecomment-2068755285)
> Compile from source MinGW64, run twice..

What are the exact steps to reproduce, including the exact operating system version?

Does it happen with the compiled release version as well?

Ref: https://en.cppreference.com/w/cpp/filesystem/remove

💬 laanwj commented on pull request "depends: build expat with CMake":
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2068765922)
FWIW #29923 removes the need to deal with expat in any way (as it's a dependency of a dependency).