💬 Sjors commented on pull request "rpc: fix mintime field testnet4, expand timewarp attack test":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2572654857)
> not clear from the description what the griefing attack is that it seeks to prevent.

It doesn't prevent it, it merely demonstrates. It can only be prevented if the miner actually uses `mintime` or `curtime`, or if a consensus change to increase `MAX_TIMEWARP` to well above `MAX_FUTURE_BLOCK_TIME`.

I expanded the comment.

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2572662414)
Maybe we should have a wallet or `importdescriptors` flag that restricts imported descriptors to BIP-388? Whether to make that the default would be a separate debate.

💬 laanwj commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2572695215)
See also #31420 which tried to achieve a similar thing by adding conversion functions.

👍 TheCharlatan approved a pull request: "doc: Clarify comments about endianness after #30526"
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2531711723)
ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40

💬 TheCharlatan commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1903895648)
Nit: Call it `hexadecimal` instead of `hex` like done in the `"data"` field for better consistency?

👍 laanwj approved a pull request: "refactor: modernize recent `ByteType` usages and read/write functions"
(https://github.com/bitcoin/bitcoin/pull/31601#pullrequestreview-2531751093)
Code review ACK 9908ab0581ec7a873514a09edb27a7cbaba3611d
Good to get rid of a few redundant `UCharCasts`.

👍 Sjors approved a pull request: "doc: Clarify comments about endianness after #30526"
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2531755996)
ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40

💬 hodlinator commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#issuecomment-2572740076)
re-ACK f919d919eb8425ef2bb25aa0ebf61c90ab9b07fa

Only commit message improvements since last ACK.

---

PR summary nit:

> Added dedicated padding tests to hardcode the failure behavior.

Not sure "hardcode" is the best verb there, how about:

"Added dedicated padding tests to cover failure behavior."?

📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31607)
<!--


Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly: trading

* Any test improvements or new tests that improve coverage are always welcome.
* All other changes shou
...

✅ willcl-ark closed an issue: "Fake bitcoin core website at the top of duckduckgo"
(https://github.com/bitcoin/bitcoin/issues/31602)

💬 willcl-ark commented on issue "Fake bitcoin core website at the top of duckduckgo":
(https://github.com/bitcoin/bitcoin/issues/31602#issuecomment-2572746178)
Hey @thebignaught, thanks for the report, here and on bitcoincore.org repo.

I this this issue will be best left open over there rather than in both repos, so I'm going to close here.

I have also reported the website as per @1440000bytes to duckduckgo and would encourage anyone else reading this to take 30 seconds to do the same. Reports could also be made to https://safebrowsing.google.com/safebrowsing/report_phish/ which DDG (and others) may? use.

I am not sure there is much else we ca
...

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

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

💬 maflcko commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#issuecomment-2572752778)
rebased for fresh CI, no code change.

💬 maflcko commented on pull request "[28.x] 28.1 backports and final changes":
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2572777372)
@Sjors I don't think it is useful to backport PRs which have neither been reviewed, nor merged into a final tag without a rc even. Especially, given that the fix is for test-only network with brittle mining anyway.

💬 maflcko commented on pull request "[28.x] 28.1 backports and final changes":
(https://github.com/bitcoin/bitcoin/pull/31594#discussion_r1903955230)
Good catch. Added missing credit line!

💬 Sjors commented on pull request "[28.x] 28.1 backports and final changes":
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2572790627)
@maflcko obviously I'm not advocating backporting the PR before it's merged, but rather to delay the release until it is.

> Especially, given that the fix is for test-only network with brittle mining anyway.

That's probably a good reason to not wait, but it seemed good to bring it up.

👍 hodlinator approved a pull request: "#31403 follow-up"
(https://github.com/bitcoin/bitcoin/pull/31599#pullrequestreview-2531822509)
ACK a3f5573ae3aa5edbe92d0388f36586ad02214de6

First commit follows recommendations by myself with improvements by maflcko in already merged PR.

Passed subset of functional tests locally.

---

Might squeeze a little bit more information into the title?

"#31403 follow-up" -> "qa: Improve framework.generate* enforcement (#31403 follow-up)"

See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#creating-the-pull-request

👍 Eunovo approved a pull request: "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC"
(https://github.com/bitcoin/bitcoin/pull/31179#pullrequestreview-2531822542)
Tested ACK https://github.com/bitcoin/bitcoin/pull/31179/commits/bf5c569898d0297de010102a623bf52009607ed8

Confirmed slight improvement in `BlockToJson` bench.
Before:
```
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 438,695.0
...

📝 brunoerg converted_to_draft 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