💬 fanquake commented on pull request "[28.x] Backports & 28.1rc1":
(https://github.com/bitcoin/bitcoin/pull/31104#issuecomment-2511697266)
Thanks, fixed up the message.

💬 fanquake commented on pull request "build: Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31307#issuecomment-2511697621)
Added a commit for 28.x to #31104 to work around the same MSVC failure.

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2511719781)
Concept ACK.

It would be nice if the asmap data blob didn't need to be converted to `std::vector<bool>` at runtime anymore, because that implies using memory twice (the blob itself, plus the decoded vector, despite containing effectively the same data). To improve upon that, I don't think it's hard to make the asmap interpreter code operate on a `std::span<const std::byte>` instead of a `std::vector<bool>`. Do you feel like looking into that, or do you want me to do so as a follow-up?

💬 instagibbs commented on pull request "test: Call generate RPCs through test framework only":
(https://github.com/bitcoin/bitcoin/pull/31403#issuecomment-2511736165)
any way you can see to prevent "regressions"?

💬 maflcko commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2511739493)
... https://cirrus-ci.com/task/6390830670282752?logs=ci#L4462

💬 maflcko commented on pull request "test: Call generate RPCs through test framework only":
(https://github.com/bitcoin/bitcoin/pull/31403#issuecomment-2511751001)
I don't see a trivial way to enforce this (even more) in Python, but given that there are only 9 lines that needed to be touched, it seems that the majority of code is fine, which is nice.

💬 darosior commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2511789684)
> By normal circumstance you mean when nobody else on the network is attempting a timewarp attack? But what if there is one?

Sure, a timewarp attack is not normal circumstances. But even then this change would not be harmful. It would merely not be supporting attackers. Which as i argued in OP i see as a benefit.

> Let's say the soft fork is deployed in the future with MAX_TIMEWARP set to 300. But honest miner Alice is still running the v29 release with this PR it, using 600. Mean miner Bo
...

💬 ismaelsadeeq commented on pull request "test: enable running independent functional test sub-tests":
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1866053557)
Happy to update when retouching.

💬 danielabrozzoni commented on issue "Unable to compile for test coverage on Nixos 24.05":
(https://github.com/bitcoin/bitcoin/issues/31087#issuecomment-2511853906)
Awesome, now it works! Thank you :)

💬 darosior commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-2511864012)
This has unfortunately gotten lower in my list of priorities. I'll close this PR for now to clarify its status. I hope to get back to this at some point. Anyone willing to work on this, or parts of this feel free to grab or/and reach out.

✅ darosior closed a pull request: "PoC: fuzz chainstate and block managers"
(https://github.com/bitcoin/bitcoin/pull/29158)

👍 willcl-ark approved a pull request: "[28.x] Backports & 28.1rc1"
(https://github.com/bitcoin/bitcoin/pull/31104#pullrequestreview-2473058165)
ACK 8fef83a0a03f884e0c5399b318eb55064b84b718

Agree that it's reasonable to apply the MSVC exclusion to all MSVC versions in this fixup.

<details>
<summary>system info</summary>

| k | v |
|-------------|-------|
| OS | Ubuntu 23.10 (mantic) |
| Arch | x86_64 |
| Kernel | 6.5.0-44-generic |
| System | Linux |
| CPU Model | 11th Gen Intel(R) Core(TM) i7-11700K @ 3.60GHz |
| CPU Cores | 16 |
| Memory | 62Gi |
| CC | Homebre
...

👋 brunoerg's pull request is ready for review: "fuzz: wallet: add target for spkm migration"
(https://github.com/bitcoin/bitcoin/pull/29694)

💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2511884253)
Force-pushed changing the fuzz target to address the `ApplyMigrationData` function. Now, after migrating the spk to descriptor, it creates some transactions and apply the migrated data.

Note that the CI failure is expected (see #31374):
```sh
[09:11:48.112] Run spkm_migration with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-max_total_time=60']INFO: Running with entropic power schedule (0xFF, 100).
[09:11:48.112] INFO: Seed: 2346481497
[09:11:48.1
...

💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1866096570)
We are currently using the nullness of `m_config_path` to signal something different, initialized/not, although I think it should probably be more explicit. See https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2470510859, and this code:

https://github.com/bitcoin/bitcoin/blob/2ac863bdf974d7b4b4e8314876d02270400b6240/src/common/args.cpp#L761-L772

We could possibly do `optional<optional<path>> m_config_path` to flag both initialization and negation... but I'm not sure I want to go
...

📝 furszy opened a pull request: "[RFC] descriptors: inference process, do not return unparsable multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31404)
---- Pushing it under RFC to gather some feedback ----

The internal script-to-descriptor inference procedure shouldn't return invalid
descriptors or ones that fail the string-to-descriptor parsing process. These
two procedures should always support seamless roundtrips.

This inlines `InferScript`/`InferDescriptor` to the actual descriptors
`ParseScript` logic for the multisig path, ensuring only valid descriptors are
produced.

Examples where this presents an issue:
1) Because the l
...

💬 edilmedeiros commented on issue "release: ship codesigned MacOS arm64 binaries":
(https://github.com/bitcoin/bitcoin/issues/29749#issuecomment-2511949059)
This is still an issue in v28. Would be great to at least include a readme file in the tarball stating that the binaries should be signed after download.

💬 sipa commented on pull request "[RFC] descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1866127089)
Can you introduce a constant for this number 3 (if it doesn't exist already)?

💬 sipa commented on pull request "[RFC] descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1866126191)
The limit is 520, not 512.

💬 willcl-ark commented on issue "release: ship codesigned MacOS arm64 binaries":
(https://github.com/bitcoin/bitcoin/issues/29749#issuecomment-2511980210)
@edilmedeiros notarization (and codesigning) is being actively worked on by @achow101. It's my understanding (and I could be wrong here) that, if we get the notarization working, we can apply it retroactively to other binaries which would resolve this issue in a preferable way.