💬 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).

✅ maflcko closed an issue: "Signed Integer Overflow in GetBlockSubsidy at block height 2,147,483,647 (During Epoch 10,227, halving 10,226) could increase the block subsidy, total supply, or potentially halt the network"
(https://github.com/bitcoin/bitcoin/issues/29928)

💬 maflcko commented on issue "Signed Integer Overflow in GetBlockSubsidy at block height 2,147,483,647 (During Epoch 10,227, halving 10,226) could increase the block subsidy, total supply, or potentially halt the network":
(https://github.com/bitcoin/bitcoin/issues/29928#issuecomment-2068767323)
The year 2106 will happen before that (https://en.bitcoin.it/wiki/Protocol_documentation#cite_ref-2), and this can be fixed at the same time, when the time has come. I don't think there is a need to keep this issue open, or open an issue for every single function that will be affected by a `int` block-height signed overflow.

Moreover, this is the wrong place to discuss protocol or consensus changes.


General bitcoin questions and/or support requests are best directed to the [Bitcoin Stack
...

💬 laanwj commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1574339319)
For better or worse, this silent accepting behavior for invalid hex values in URL decoding is really common. See for example Python:

```python3
>>> import urllib.parse
>>> urllib.parse.unquote('%12%%xx%0a')
'\x12%%xx\n'
```
It's the web philopsophy of "in case of doubt, just do something!" 😅 . Anyhow, in this refactor it doesn't make sense imo to add error handling here.

👍 laanwj approved a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2014113911)
Concept and code review ACK 175d57ba953cdc1da3a54ae0208dad813f7ea4cf

🤔 hebasto reviewed a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2014144499)
Is `if USE_LIBEVENT` guard in https://github.com/bitcoin/bitcoin/blob/175d57ba953cdc1da3a54ae0208dad813f7ea4cf/src/Makefile.am#L715-L718 still needed?

💬 maflcko commented on issue "Crash on fail ~CCheckQueue() destructor?":
(https://github.com/bitcoin/bitcoin/issues/29930#issuecomment-2068824789)
This was fixed in Bitcoin Core 27.x

✅ maflcko closed an issue: "Crash on fail ~CCheckQueue() destructor?"
(https://github.com/bitcoin/bitcoin/issues/29930)

💬 maflcko commented on issue "Crash on fail ~CCheckQueue() destructor?":
(https://github.com/bitcoin/bitcoin/issues/29930#issuecomment-2068825320)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the exact steps to reproduce?