🤔 EthanHeilman reviewed a pull request: "test: Add encodable PUSHDATA1 examples to feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32114#pullrequestreview-2718936522)
ut_ack

💬 EthanHeilman commented on pull request "test: Add encodable PUSHDATA1 examples to feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32114#discussion_r2015191194)
Why are two secs and two pubs created if only one is used? Is to show a pattern of generating more than one so other people can add additional tests to this?

💬 EthanHeilman commented on pull request "test: Add encodable PUSHDATA1 examples to feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32114#discussion_r2015194312)
This is helpful for understanding how to write these tests. I might make sense to put this as the first testcase just below `# === Actual test cases ===`. I worry people might not scroll down the 600 lines to find this. It is a good introduction to the rest of the tests

👍 ryanofsky approved a pull request: "doc: Fix and clarify description of ZMQ message format"
(https://github.com/bitcoin/bitcoin/pull/31862#pullrequestreview-2718876311)
Code review 882375f2eb3942be644591ae45fbe57458ed40ee. I think the changes here describing endianess look good, and I like new descriptions pointing out the common parts of messages. But I also found the formatting and structure of the new section hard to parse (old section was also bad but it was a little simpler). I couldn't even figure out what is was saying until I looked at the `zmq_send_multipart` call. So I would recommend making some more changes to try to clarify, and I left some sugges
...

💬 ryanofsky commented on pull request "doc: Fix and clarify description of ZMQ message format":
(https://github.com/bitcoin/bitcoin/pull/31862#discussion_r2015153965)
In commit "doc: Fix and clarify description of ZMQ message format" (882375f2eb3942be644591ae45fbe57458ed40ee)

This is confusing because it says the messages have 3 parts, but the text above says there are two parts: topic and body, and seems treats the sequence number as part of the body. Looking at the code I think your version is more accurate, but if you want to keep this description I think you should update the paragraph above to be consistent.

💬 ryanofsky commented on pull request "doc: Fix and clarify description of ZMQ message format":
(https://github.com/bitcoin/bitcoin/pull/31862#discussion_r2015185506)
In commit "doc: Fix and clarify description of ZMQ message format" (882375f2eb3942be644591ae45fbe57458ed40ee)

I had a hard time figuring out how how to read this section, because the list of topics is broken up by explanations and indented diagrams so it wasn't clear what the text was referring to and where descriptions ended and began. Was also confused about why the first line only mentioned topics and now bodies but for some reason left off sequence numbers. And then even confused by extre
...

💬 ryanofsky commented on pull request "doc: Fix and clarify description of ZMQ message format":
(https://github.com/bitcoin/bitcoin/pull/31862#discussion_r2015159054)
Maybe say "block and transaction hashes" instead of "32 byte-hashes" to provide more context because there is the first mention of "32 byte hashes" in the document and it is not clear what it refers to.

:lock: fanquake locked an issue: "Accedence; SAI-15; SLH-DSA."
(https://github.com/bitcoin/bitcoin/issues/32147)

:lock: fanquake locked an issue: "Write access to Testing Guide for 29.0 RC"
(https://github.com/bitcoin/bitcoin/issues/32102)

🤔 ryanofsky requested changes to a pull request: "test: avoid treating hash results as integers"
(https://github.com/bitcoin/bitcoin/pull/32050#pullrequestreview-2719038929)
Code review a82829f37e1ed298b6c2b6d2859d4bea65fe3dcc and thanks for the updates! It looks like this might introduce a minor bug though (see comment below)

💬 ryanofsky commented on pull request "test: avoid treating hash results as integers":
(https://github.com/bitcoin/bitcoin/pull/32050#discussion_r2015267166)
In commit "test: avoid unneeded hash -> uint256 -> hash roundtrips" (346a099fc1e282c8b53d740d65999d8866d17953)

Could simplify this to `ZERO_HASH = bytes(32)`

💬 ryanofsky commented on pull request "test: avoid treating hash results as integers":
(https://github.com/bitcoin/bitcoin/pull/32050#discussion_r2015340120)
In commit "test: simplify (w)txid checks by avoiding .calc_sha256 calls" (a82829f37e1ed298b6c2b6d2859d4bea65fe3dcc)

It seems like there might be a test bug here, and this check will no longer fail even if the hash is the same because `rehash` returns an `int` while `getwtxid` returns a hex `str`

📝 pablomartin4btc opened a pull request: "wallet, migration: Fix empty wallet crash"
(https://github.com/bitcoin/bitcoin/pull/32149)
Same as with a blank wallet (#28976), wallets with no legacy records (i.e. empty, non-blank, watch-only wallet) do not require to be migrated.

Steps to reproduce the issue:

1.- `createwallet "empty_wo_noblank_legacy_wallet" true false "" false false`
2.- `migratewallet`

```
wallet/wallet.cpp:4071 GetDescriptorsForLegacy: Assertion `legacy_spkm' failed.
Aborted (core dumped)
```

📝 murchandamus opened a pull request: "refactor: Optimize BnB exploration"
(https://github.com/bitcoin/bitcoin/pull/32150)
This PR refactors the implementation of the BnB coinselection algorithm to skip the duplicate evaluation of previously visited input selections.

💬 murchandamus commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2756305377)
It was easier than expected: https://github.com/bitcoin/bitcoin/pull/32150

📝 murchandamus converted_to_draft a pull request: "refactor: Optimize BnB exploration"
(https://github.com/bitcoin/bitcoin/pull/32150)
This PR refactors the implementation of the BnB coinselection algorithm to skip the duplicate evaluation of previously visited input selections.

📝 sipa opened a pull request: "Follow-ups for txgraph #31363"
(https://github.com/bitcoin/bitcoin/pull/32151)
This addresses a few review comments in #31363 after merge:
* https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2012730654 (rename `group_data` to `group_entry`)
* https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2012786771 (introduce `MAIN_LEVEL` and `STAGING_LEVEL` constants, together with a few doc improvements).
* https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015003889 (typo in `FixLinearization`)

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015468627)
Done in #32151

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015469265)
Done in #32151

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015469749)
Done in #32151