💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1903097152)
Just this embarrassing bash script:

```sh
for i in {0..2015}
do
hash=`build/src/bitcoin-cli -testnet4 getblockhash $i`
block=`build/src/bitcoin-cli -testnet4 getblock $hash 0`
echo $block >> test/functional/data/testnet4.hex
done
```

💬 remcoros commented on pull request "rpc: fix mintime field testnet4, expand timewarp attack test":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2571285614)
ckpool uses curtime: https://bitbucket.org/ckolivas/ckpool/src/bb7b0aebe08ed99d45b8701af2d65bfcdbef0932/src/bitcoin.c#lines-196
public-pool uses mintime: https://github.com/benjamin-wilson/public-pool/blob/master/src/services/stratum-v1-jobs.service.ts#L76

does this mean that currently, public-pool will create an invalid block on testnet4 if in a difficulty-adjustment period and timestamps have moved too far forward?

👍 i-am-yuvi approved a pull request: "Feature: Use different datadirs for different signets"
(https://github.com/bitcoin/bitcoin/pull/29838#pullrequestreview-2530735302)
Tested ACK f1b40b4de460173e927ebe93cdb5f4ad2ac02859

- I tried testing using `./src/bitcoind -signetchallenge=512102ee856c56a5aaadd1656f849bafa4c9dacc86a2878fe546c6189185f842ae2c1851ae`
- All the test(functional) passes!!

👍 i-am-yuvi approved a pull request: "fuzz: Abort if system time is called without mock time being set"
(https://github.com/bitcoin/bitcoin/pull/31549#pullrequestreview-2530735643)
Tested ACK fdc5224d205c69ebcb2d2b7bcd95386ef89181ca on x86_64

- removing `setmocktime ()` from any one of the targets would cause a crash.
- Not testes on macos

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