Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 ryanofsky commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853811005)
re: https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853683566

> What's the reason for not wanting an `about` window regardless of the `-version` flag's value? Doesn't `gArgs.IsArgSet("-version")` make more sense here?

The idea is for -noversion and -version=0 to be the same as not specifying a -version argument. -version is just a binary option, it should choose between 2 behaviors, not 3
💬 ryanofsky commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853779968)
re: https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853551532

I think the idea here is just for -version to take precedence over -help if both are specified. I don't know if this is the best behavior, but I don't see a reason to change in a commit that is just trying to handle -version=0 and -noversion more sensibly.
💬 ryanofsky commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853807905)
re: https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853700315

> given that checking the args is not for free (triggers locks and merges from multiple sources), maybe we could introduce a `IsArgEnabled` which gets rid of this confusing combination (e.g. where `-noconf=0` would be set and not negated, I think)

#31260 gets away from this error-prone repeated retrieval of settings by forcing the type of every setting to be declared up front, and only providing a single `Get` accesso
...
💬 l0rinc commented on issue "ci: how to run native arm job on Apple silicon?":
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493612110)
Not sure it helps, but I have tried reproducing it on my M4 Max, emulating an s390x platform (as I've done in https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2453108430) - while it's still very (very) slow:

<details>
<summary>docker setup</summary>

```bash
brew install podman pigz
softwareupdate --install-rosetta
podman machine init
podman machine start
docker run --platform linux/s390x -it ubuntu:22.04 /bin/bash
```

</details>

```bash
apt update && apt install -y
...
💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2493614105)
Well that's nice, looks like only the ASan + LSan + UBsan job is failing.
📝 Sjors opened a pull request: "Set notifications m_tip_block in LoadChainTip()"
(https://github.com/bitcoin/bitcoin/pull/31346)
Ensure KernelNotifications `m_tip_block` is set even if no new block arrives.

Suggested in https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2486457573
💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2493619203)
See #31346.
👍 rkrux approved a pull request: "contrib: skip missing binaries in gen-manpages"
(https://github.com/bitcoin/bitcoin/pull/30986#pullrequestreview-2454288885)
re-ACK 4bbd28baf33382231f4f1dab20681c05f9915af2

Minimal range diff.

<details>
<summary>git range-diff f080618...4bbd28b</summary>

```
1: 7060d64bf7 ! 1: 299e2220e9 gen-manpages: implement --skip-missing-binaries
@@ contrib/devtools/gen-manpages.py: BINARIES = [
+ "--skip-missing-binaries",
+ action="store_true",
+ default=False,
-+ help="skip generation for binaries that are not found",
++ help="skip generation for binaries that are no
...
💬 l0rinc commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#discussion_r1853828160)
nit: at this point it may make more sense to just remove the `[[nodiscard]]` from `blockTip` instead
💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853836054)
I'm not against that, my concern is just that reproducing old behavior before the fix isn't very useful activity - it's a lot more useful to see that a test fails before the fix with the correct error (i.e TDD), but that's not really possible currently - unless we add something like https://github.com/spockframework/spock/blob/master/spock-core/src/main/java/spock/lang/PendingFeature.java#L13-L17 (where we invert the test, making sure it fails (that's when the test is green) and remove the annot
...
Sjors closed an issue: "ci: how to run native arm job on Apple silicon?"
(https://github.com/bitcoin/bitcoin/issues/31344)
💬 Sjors commented on issue "ci: how to run native arm job on Apple silicon?":
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493642543)
> It is not possible to run 32-bit arm binaries on a CPU that only supports 64-bit mode.

Ah, in that case it's probably pointless for a performance improvement.

> select qemu-arm in UTM. (Is there a reason why you haven't done this in the first place?)

My initial goal for that Ubuntu VM was to make Guix builds.

I could spin up a separate machine in 32 bit mode, but that seems like a world of pain on its own. Closing for now.
💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853838097)
what about `-noversion=0` :p

There has to be a better way, this is so confusing :/
💬 Sjors commented on issue "ci: how to run native arm job on Apple silicon?":
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493644965)
I guess my confusion came from the use of `arm64` as the label for cirrus.
💬 Sjors commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#discussion_r1853843127)
Maybe, or we could do something with the return value. I'm not familiar enough with init code and kernel notifications to have a strong opinion on this. cc @ryanofsky, @TheCharlatan
💬 maflcko commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#issuecomment-2493675258)
This is incomplete. You'll have to update the docs as well: "If the tip was not connected on
* startup, this will wait.", and "which may
//! be true even long after startup, until shutdown.". Also, in init.cpp you can remove the if-guard?
💬 orangesurf commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2493677462)
A miner (earning coinbase rewards to [tb1q2dsc...](https://mempool.space/testnet4/address/tb1q2dsc94zq40nwnz27w5rxljwllutnwjtlxk44fz) is currently overwhelmingly dominant on testnet4.

![image](https://github.com/user-attachments/assets/09db2517-29ae-403b-afbb-6ab7cbfdc926)

Since block [000000000000000167286a4bea3bca60d6c3ab2cbbef79537bdf07b6420a2d2d / 54780](https://mempool.space/testnet4/block/000000000000000167286a4bea3bca60d6c3ab2cbbef79537bdf07b6420a2d2d) this miner has mined every bl
...
💬 maflcko commented on issue "ci: how to run native arm job on Apple silicon?":
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493687553)
Yes, the cirrus machine is intentionally an aarch64 (also referred to as arm64) machine, because arm64 is generally available as performant hardware, compared to 32-bit arm. It was also intentionally selected to support 32-bit mode. (If you are looking for a cloud solution, IIRC only Hetzner cloud ships compatible VMs). Otherwise, you'll have to shop for compatible bare-metal yourself, or use qemu (or similar).
⚠️ rkrux opened an issue: "Add support for creating v3 raw transactions in `createrawtransaction` RPC"
(https://github.com/bitcoin/bitcoin/issues/31348)
### Please describe the feature you'd like to see added.

Currently, `createrawtransaction` RPC creates only v2 raw transaction, i.e. the first byte of the serialised transaction hex is `02`. It'd be helpful for the RPC to conditionally create V3 raw transactions if such intent is passed in the arguments of the RPC call.

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

- A new argument to the RPC call can be passed that al
...
💬 ryanofsky commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853870363)
> I'm not against that, my concern is just that reproducing old behavior before the fix isn't very useful activity

Strongly disagree :). In 2ca1460ae3a7217eaa8c5972515bf622bedadfce if tests had not been added *before* that commit checking for incorrect HTTP error codes, you would not know from looking at the change what the behavior was before the bugfix, or how new tests were connected to it. It is very useful to have test coverage for bugs before they are fixed.

> it's a lot more useful
...