💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#issuecomment-2571308883)
Small changes:
* dropped testnet4 genesis block
* explained in the test how to generate / verify testnet4.hex
* added `bits` and `target` to `getblockchaininfo` and `getchainstates` (didn't realise @tdb3 had already implemented it)

💬 Sjors commented on pull request "rpc: fix mintime field testnet4, expand timewarp attack test":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2571310453)
@remcoros a testnet4 pool that uses `min_time` could produce an invalid block each first block of the retarget period, since that's the only time when `mintime` can be wrong.

At first glance, it seems that might happen with `public-pool`. The easiest workaround would be to use `curtime` for now.

💬 Sjors commented on pull request "[28.x] 28.1 backports and final changes":
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2571311189)
In light of https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2571285614 it may be worth back-porting that PR as well. On the other hand it only impacts testnet4.

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2571312484)
Changing anything about `getblocktemplate` can be tricky in my experience https://github.com/bitcoin/bitcoin/pull/27433#issuecomment-1607709381, because it's defined in BIP22: https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki

So I would leave "making it a runtime option" to a followup. That said, I'm skeptical how useful that would be. Legacy pool software probably won't use it, Ocean has a fixed value that they tell users to configure and modern mining pool software should just
...

💬 i-am-yuvi commented on pull request "contrib: fix BUILDDIR in gen-bitcoin-conf script":
(https://github.com/bitcoin/bitcoin/pull/31332#issuecomment-2571312721)
Can I take this up if no one is working?

💬 i-am-yuvi commented on pull request "contrib: fix BUILDDIR in gen-bitcoin-conf script":
(https://github.com/bitcoin/bitcoin/pull/31332#issuecomment-2571316023)
> `contrib/devtools/README.md` needs to be updated to mention this as then the script would no longer work for in-tree builds (if we ever do that with cmake, but I have asked for it before).
>
> Furthermore, that documentation is pretty clear that out of tree builds should set `BUILDDIR`, and with cmake, all builds will be out of tree, so I'm not sure that this change is actually useful.

Agree, I think we can close this.

🤔 tdb3 reviewed a pull request: "rpc: add gettarget , target getmininginfo field and show next block info"
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2530747942)
Approach ACK

Verified the addition of `bits` and `target` on mainnet:
```
$ build/src/bitcoin-cli getblockchaininfo
{
"chain": "main",
"blocks": 877759,
"headers": 877796,
"bestblockhash": "00000000000000000001cdf88323d957f92d07693b953c32646bac7c797634af",
"bits": "1702905c",
"target": "00000000000000000002905c0000000000000000000000000000000000000000",
"difficulty": 109782075598905.2,
"time": 1735981113,
"mediantime": 1735976878,
"verificationprogress": 0.9998
...

💬 tdb3 commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1903105642)
Looks like this line is a duplicate (`target` added twice)

💬 starius commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2571317126)
@hugohn Great work!

Does `/**` in descriptors mean a combination of `/0/*` and `/1/*`? I.e. a receive and a change descriptor.

💬 fjahr commented on pull request "rpc: fix mintime field testnet4, expand timewarp attack test":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2571319020)
Concept ACK

📝 brunoerg opened a pull request: "descriptor: check whitespace in `ParsePubkeyInner`"
(https://github.com/bitcoin/bitcoin/pull/31603)
Currently, we successfully parse descriptors which contains spaces in the beginning or end of the fragment (e.g. `pk( KEY)`, `pk(KEY )` or `pk( KEY )`. I noticed that one of the reasons is that the `DecodeBase58` function simply ignore these whitespaces. This PR changes the `ParsePubkeyInner` function to reject pubkeys that contain a whitespace at the beginning and/or at the end.

For context: https://github.com/brunoerg/bitcoinfuzz/issues/72

💬 hebasto commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2571327656)
> See [python/cpython#128005](https://github.com/python/cpython/issues/128005) (long discussion, skip to [this post](https://github.com/python/cpython/issues/128005#issuecomment-2547582533) to save time) for more background on this. This is NetBSD-specific, and apparently caused by a pkgsrc patch which inadvertently caused `sys.float_repr_style` to be "legacy". So the question is if this qualifies as a bug in NetBSD or not and whether we need to accomodate it by moving to Decimals.

The NetBSD
...

✅ hebasto closed a pull request: "qa: Ensure consistent use of decimals instead of floats"
(https://github.com/bitcoin/bitcoin/pull/31595)

💬 TheCharlatan commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1903111922)
If I understand correctly passing `-m -M` will give precedence to `-M`. Should we really support this? Might it better if providing multiple flags at once be an error? E.g. currently running
```
./bitcoin -h -m daemon
```
will print the help string and run the daemon, which might be confusing, since it is not doing the same for `-v`.

💬 hebasto commented on pull request "ci: Switch to latest macOS and Windows images":
(https://github.com/bitcoin/bitcoin/pull/31597#issuecomment-2571328087)
@maflcko

> > Alternatively, these comments should be removed.
>
> Makes sense to remove the comment and explain that the xcode version is supposed to denote the minimum supported version to compile from xcode. I presume this is equal to https://github.com/bitcoin/bitcoin/blame/6aa0e70ccbd5491ec9d7c81892820f3341ccd631/doc/release-notes-empty-template.md#L46, where 13.0+, means 13+. I assume that anyone on 13.0 is able to, and must upgrade to the latest 13.x anyway, so that the minimum requ
...

💬 l0rinc commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2571328486)
I could see this going in a different way: not storing RPC parameters as python floats, and changing all such usages to strings instead (as shown in https://github.com/bitcoin/bitcoin/pull/31595#discussion_r1901722976).

💬 hebasto commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2571330376)
> I could see this going in a different way: not storing RPC parameters as python floats...

I agree with that plus https://github.com/bitcoin/bitcoin/pull/31595#discussion_r1901893390.

💬 l0rinc commented on pull request "sync: improve CCoinsViewCache ReallocateCache - 2nd try":
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2571333030)
@fjahr, I've measured the effect of this change on a full IBD - <1%, doesn't seem significant.

<details>
<summary>benchmarks</summary>

> master
```bash
Benchmark 1: COMMIT=d73f37dda221835b5109ede1b84db2dc7c4b74a1 ./build/src/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=870000 -printtoconsole=0
Time (mean ± σ): 39690.021 s ± 333.717 s [User: 51244.778 s, System: 3430.580 s]
...

💬 MarnixCroes commented on pull request "contrib: fix BUILDDIR in gen-bitcoin-conf script":
(https://github.com/bitcoin/bitcoin/pull/31332#issuecomment-2571336672)
sorry forgot to reply.

while `BUILDDIR` can be set, it's odd that simply executing the script doesn't work imo

It's not clear for me what is the desired way forward.

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2571340498)
Rebased on top of the merged https://github.com/bitcoin/bitcoin/pull/31542 and a commit picked from https://github.com/bitcoin/bitcoin/pull/31428.