Bitcoin Core Github
43 subscribers
122K links
Download Telegram
🤔 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?
💬 laanwj commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2068827925)
No, it isn't. Nor is piling on the `EVENT_CFLAGS`. Good catch @hebasto.
💬 virtu commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#discussion_r1574349440)
```suggestion
#!/usr/bin/env python3
# Copyright (c) 2022 Pieter Wuille
```
💬 virtu commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#discussion_r1574356520)
The last non-f-string left, I think.
💬 virtu commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#discussion_r1574368473)
nit: I searched other scripts in `contrib` for appending to `sys.path` to find out if there's precedent one could follow:

```python
testgen/gen_key_io_test_vectors.py
14:sys.path.append(os.path.join(os.path.dirname(__file__), '../../test/functional')
message-capture/message-capture-parser.py
16:sys.path.append(os.path.join(os.path.dirname(__file__), '../../test/functional'))
```
📝 vasild opened a pull request: "doc: suggest only necessary Qt packages for installation on FreeBSD"
(https://github.com/bitcoin/bitcoin/pull/29932)
The previously suggested `qt5` package is a meta package that does not install anything itself but depends on a bunch of others and is used as a convenience to install "everything" Qt5 related: 253 packages / 3 GiB.

We only need a subset of those which amounts to 71 packages / 224 MiB, so suggest just that.

For comparison:

```
Updating local repository catalogue...
local repository is up to date.
All repositories are up to date.
Checking integrity... done (0 conflicting)
The follow
...