💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1599508314)
06c2c713c52b60231efc3e00d2c5eb0bf9e345f9: we have to remember to update this hardcoded value in three different places if we reset testnet4 (before the release). I verified that it currently matches the actual genesis block (`getblockhash 0`).

It's not dangerous to mainnet if we forget, it would only break testnet4. But I would prefer to introduce an `enum class Network` so we can do `consensusParams.network == Network::Testnet4`. It can wait for a followup.

If you're worried about growing
...

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1599609418)
e172a96c0781de2bbd69312905d2c16cc1745c2f: `params.fPowAllowMinDifficultyBlocks &&` is redundant. Though you could an assert.

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1599635493)
e172a96c0781de2bbd69312905d2c16cc1745c2f: this had me confused again, so maybe a comment is helpful.

If miners go away in the middle of difficulty adjustment period A, this rule will find the last "real" `nBits` value and apply it to the first block of period B. If nothing changes, then for period C this seeks back to the first block of period B, finds the real value and applies it.

But then how do the first blocks of B and C ever mined?

Well, remember that `GetNextWorkRequired` ignore
...

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1599648112)
@russeree the use case for these test vectors is for regression testing, so you'd want to spin up a testnet4 node, sync a few thousand blocks and delete it again. Having such a small number of blocks might allow you to store the actual blocks on the CI server - so it doesn't need to make network requests.

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1599652146)
> I'm assuming the transactions are sorted from parents to children.

Yes, they are.

The fee estimator does not take all transaction with parents into account already see
https://github.com/bitcoin/bitcoin/blob/7fcf4e99791ca5be0b068ac03a81a50ece11dba3/src/policy/fees.cpp#L615

Assuming A->B->C->D are topologically sorted from parent to descendant.

In this case B, C, D are already not considered for fee estimation, After this PR A will also be ignored.

👍 TheCharlatan approved a pull request: "refactor: simplify `FormatSubVersion` using strprintf/Join"
(https://github.com/bitcoin/bitcoin/pull/30098#pullrequestreview-2054780864)
tACK 12d82817bf32396b58c8c65645012def606680b6

🤔 glozow reviewed a pull request: "p2p: index TxOrphanage by wtxid, allow entries with same txid"
(https://github.com/bitcoin/bitcoin/pull/30000#pullrequestreview-2054751541)
Thanks @mzumsande @AngusP @instagibbs! Addressed comments. I've also expanded the "cast from txid to wtxid" part to be slightly more verbose but hopefully easier to understand.

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

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