🤔 mzumsande reviewed a pull request: "net, init: derive default onion port if a user specified a -port"
(https://github.com/bitcoin/bitcoin/pull/31223#pullrequestreview-2424908328)
>Another alternative: don't listen for Tor on regtest by default (were all reports for this for regtest?).

I don't know, but I could see people / orgs having local setups with multiple nodes on mainnet, e.g. with one node facing random peers from the internet/allowing inbounds, and others that only connect to local trusted peers. In general, I don't like regtest-specific rules so I would see them as a last resort if nothing else works.

> What about setups that use e.g. -port=5000 and have
...

💬 mzumsande commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1835050453)
dropped as suggest. I guess I was also thinking about local port contention: If bitcoind binds to a port from the list that is usually used by other protocols, then this might affect other programs from starting up or working correctly. Do you think that is an issue we should care about?

💬 mzumsande commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1835065201)
I think that `-bind=...=onion` in connection with `-port` doesn't resolve the problem completely, because that node wouldn't have any other binds than the onion bind (in particular no bind-on-any) and therefore couldn't receive any non-onion inbound connections.
Not sure if there is a way to avoid the problem other than not using `-port` in the first place.

💬 mzumsande commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1835061247)
Sound interesting - will try to test this a bit more / and will probably change it to that early next week.

Is this interaction between `-port` and `-bind` without port useful / necessary for anything or more of a gimmick? (it already exists on master) I kind of think as `-port` as an option I'd choose if I didn't want to use `-bind` (and would use `-bind` with a port if I used it at all), so the combination seems a bit confusing.

💬 ryanofsky commented on pull request "WIP: scripted-diff: Type-safe settings retrieval":
(https://github.com/bitcoin/bitcoin/pull/31260#issuecomment-2465902292)
Current status of this PR is that `bitcoind` and `test_bitcoin` binaries work and functional and unit tests pass, but there are compile errors in the other binaries that need to be fixed, and this also needs to be rebased. The PR is complete with all functionality described above implemented, but it probably needs more documentation. I also would like to add more commits replacing last remaining GetArg / GetIntArg / GetBoolArg / GetArgs / IsArgSet / IsArgNegated method uses with `Setting::Get` a
...

📝 secp512k2 opened a pull request: "doc: Clarify GUI Compilation Instructions for Qt Libraries in build-n…"
(https://github.com/bitcoin/bitcoin/pull/31261)
This PR updates build-netbsd.md to improve clarity regarding the installation of Qt libraries for GUI compilation.

This small change enhances clarity for developers by ensuring that instructions for Qt installation and the GUI build option are more explicit, especially for those less familiar with the process on NetBSD.

⚠️ Aste525 opened an issue: "AST283"
(https://github.com/bitcoin/bitcoin/issues/31262)
### Please describe the feature you'd like to see added.

import secrets
import math
import json
from datetime import datetime
from decimal import Decimal

class MetaphysicalConstants:
"""Konstanta Metafisika untuk Bitcoin"""
def __init__(self):
self.BITCOIN_SUPPLY = 21000000
self.HALVING_INTERVAL = 210000
self.METAPHYSICAL_BYTES = 283 # Transformasi dari 399
self.N = 21 # Konstanta dasar Bitcoin
self.Z = self.calculate_z_ratio()

...

✅ fanquake closed an issue: "AST283"
(https://github.com/bitcoin/bitcoin/issues/31262)

:lock: fanquake locked an issue: "AST283"
(https://github.com/bitcoin/bitcoin/issues/31262)

💬 0xB10C commented on pull request "doc: Fix missing comma in JSON example in REST-interface.md":
(https://github.com/bitcoin/bitcoin/pull/31259#issuecomment-2466122408)
LLM-PR?

Not sure how useful it is to fix the example output in an interface doc. LLM operator and reviewer time is probably spent better elsewhere.

✅ maflcko closed a pull request: "doc: Clarify GUI Compilation Instructions for Qt Libraries in build-n…"
(https://github.com/bitcoin/bitcoin/pull/31261)

💬 maflcko commented on pull request "doc: Clarify GUI Compilation Instructions for Qt Libraries in build-n…":
(https://github.com/bitcoin/bitcoin/pull/31261#issuecomment-2466187878)
I don't see how this is an improvement. I think the previous wording was even more explicit in mentioning libqrencode.

Closing for now. Generally, when there isn't a bug or obvious mistake, the style is left as-is. Shuffling around wording in one way or the other doesn't improve anything and can be done endlessly (with or without the help of LLMs). However, the review bandwidth is limited.

💬 maflcko commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1835348907)
should be 28?

💬 maflcko commented on pull request "ci: make ctest stop on failure":
(https://github.com/bitcoin/bitcoin/pull/31257#issuecomment-2466191528)
lgtm ACK 36a22e5683375b7925767de9daa9df4c48831c15

I think the same happened when the project was using `make`, but I haven't double-checked.

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2466194861)
> Ok, that force-push actually contained a behavior change.

Ugh, I missed that, I didn't see the `!` negation. Generally writing a simple if with an else branch as `if (!a) ...` is confusing, because each branch will have a (double) negation. It would be better to just write `if (a)` and have a single negation in the else branch. But you didn't change that code, so it can be done in a follow-up.

👍 maflcko approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2425303884)
re-ACK 4bbcc704464a470f54f372f93b2649d67b7a60c1

Only change is the default behavior change, which I somehow missed.

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1835351305)
nit: My preference would be to use the current time (maybe in nanosecond precision) and make the path `time/test_name`. This way, it is easy to find the last test execution easily, if there is more than one.

With the current approach, one would have to look up which rand32 corresponds to the last execution.

⚠️ gmart7t2 opened an issue: "importdescriptors always rescans"
(https://github.com/bitcoin/bitcoin/issues/31263)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Running `importdescriptors` with `timestamp: now` rescans the last 2 hours worth of blocks.

The docs say that '"now" can be specified to bypass scanning' but that doesn't appear to be true:

```
"timestamp": timestamp | "now", (integer / string, required) Time from which to start rescanning the blockchain for this descriptor, in UNIX epoch time

...

📝 maflcko opened a pull request: "doc: Fixup bitcoin-wallet manpage chain selection args"
(https://github.com/bitcoin/bitcoin/pull/31264)
The sentence is missing `-testnet4` and `-chain`. Instead of duplicating the full list (and having to keep it in sync), just refer to them as `(test)chain selection arguments`.

💬 maflcko commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1835353534)
Fixed in https://github.com/bitcoin/bitcoin/pull/31264