💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1599669435)
changed to "weight" !

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1599661537)
Ah I missed that `P2PInterface` already had that! Thanks, deleted the unnecessary stuff.

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1599667375)
Good point, I've added a comment to help explain why we aren't asserting the exact error/disconnection.

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1599702152)
Added a sentence to the comment, though slightly different wording. I've also expanded it in general.

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1599660844)
fixed

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1599660492)
done

✅ glozow closed a pull request: "26.x: backport #29853 ("sign: don't assume we are parsing a sane Miniscript")"
(https://github.com/bitcoin/bitcoin/pull/29854)

💬 glozow commented on pull request "26.x: backport #29853 ("sign: don't assume we are parsing a sane Miniscript")":
(https://github.com/bitcoin/bitcoin/pull/29854#issuecomment-2109746773)
@darosior I've added to #29899 so I'm closing this. Thanks!

💬 glozow commented on pull request "depends: fix mingw-w64 Qt DEBUG=1 build":
(https://github.com/bitcoin/bitcoin/pull/29747#issuecomment-2109751220)
backported to 26.x in #29899

💬 josibake commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1599720289)
Right, but that's not the intent of the python code: it's merely to provide a more precise explanation of the definition of `H`. A unit test on secp256k1 seemed like the most natural fit.

Seems like what you actually want is a unit test in the C++ code to generate the value of `H` and then compare it to `XOnlyPubKey::NUMS_H`. I've added that in https://github.com/bitcoin/bitcoin/pull/30048/commits/8d4c70c054f73f5991408e2e2bed61f2c4384f2e

💬 glozow commented on pull request "doc: Suggest installing dev packages for debian/ubuntu qt5 build":
(https://github.com/bitcoin/bitcoin/pull/29764#issuecomment-2109751956)
backported to 26.x in #29899

💬 hebasto commented on pull request "refactor: simplify `FormatSubVersion` using strprintf/Join":
(https://github.com/bitcoin/bitcoin/pull/30098#discussion_r1599720839)
nit: Placing a non-trivial `if` statement body on its own line can be useful for code coverage and debugging.

👍 hebasto approved a pull request: "refactor: simplify `FormatSubVersion` using strprintf/Join"
(https://github.com/bitcoin/bitcoin/pull/30098#pullrequestreview-2054844843)
ACK 12d82817bf32396b58c8c65645012def606680b6, I have reviewed the code and it looks OK.

💬 glozow commented on pull request "sign: don't assume we are parsing a sane TapMiniscript":
(https://github.com/bitcoin/bitcoin/pull/29853#issuecomment-2109752682)
backported to 26.x in #29899

💬 hebasto commented on issue "windows: Newer libevent causes `http_request` fuzz target failure":
(https://github.com/bitcoin/bitcoin/issues/30096#issuecomment-2109759186)
> Is there something for this repo to do here? If that patch should be applied, then this should be a PR? If not, I guess close this?

I'm in the middle of the investigation.

💬 vasild commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#issuecomment-2109763258)
`13f438a667...c5f9afd946`: fix the above and return `std::string` instead of `std::string_view` which internally has a pointer to the `thread_local` variable. This way the API can't the misused to store the reference to the `thread_local` longer than the thread's lifespan.

👋 vasild's pull request is ready for review: "util: check for errors after close and read in AutoFile"
(https://github.com/bitcoin/bitcoin/pull/29307)

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2109769504)
Fixed the CI!

💬 hebasto commented on pull request "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP":
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2109785175)
@maflcko Would you mind having a look into this, please?

People who works on CMake migration are highly anticipating your opinion on use cases for [`APPEND_{CPP,C,CXX,LD}FLAGS`](https://github.com/hebasto/bitcoin/pull/184) in the CI scripts.

cc @theuni

👍 hebasto approved a pull request: "[27.x] Backports and probably finalize"
(https://github.com/bitcoin/bitcoin/pull/30092#pullrequestreview-2054913602)
ACK 9867e72b995e4a838c8e8e0feb159a9bdee8b76f, I have reproduced backporting locally.