💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1595707431)
Seems this is a bugfix regardless?
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1595707431)
Seems this is a bugfix regardless?
💬 glozow commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1595707676)
Oh yes I was confused. Then I wonder why this is needed...?
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1595707676)
Oh yes I was confused. Then I wonder why this is needed...?
💬 ishaanam commented on pull request "wallet, rpc: document and update `sendall` behavior around unconfirmed inputs":
(https://github.com/bitcoin/bitcoin/pull/28979#discussion_r1595715668)
Should be fixed now, sorry for the delay.
(https://github.com/bitcoin/bitcoin/pull/28979#discussion_r1595715668)
Should be fixed now, sorry for the delay.
💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1595720234)
In package RBF contexts, the 2nd time `Finalize()` is called in a subpackage, the child tx, it will try and enumerate the `m_all_conflicts` list again, but those transactions were already removed, so we would crash.
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1595720234)
In package RBF contexts, the 2nd time `Finalize()` is called in a subpackage, the child tx, it will try and enumerate the `m_all_conflicts` list again, but those transactions were already removed, so we would crash.
💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1595723053)
elaborated on the comment a bit more
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1595723053)
elaborated on the comment a bit more
📝 hebasto opened a pull request: "test, subprocess: Improve coverage report correctness"
(https://github.com/bitcoin/bitcoin/pull/30075)
As a child process uses the [`execvp()`](https://linux.die.net/man/3/execvp) call, an explicit dumping of the collected profile information is required.
Coverage:
- on the master branch:

- with this PR:

(https://github.com/bitcoin/bitcoin/pull/30075)
As a child process uses the [`execvp()`](https://linux.die.net/man/3/execvp) call, an explicit dumping of the collected profile information is required.
Coverage:
- on the master branch:

- with this PR:

💬 josibake commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1595777133)
Oh! I see , I misinterpreted what you were suggesting. This does seem better, will update.
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1595777133)
Oh! I see , I misinterpreted what you were suggesting. This does seem better, will update.
💬 mjdietzx commented on pull request "tests: improve wallet multisig descriptor test and docs":
(https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1595783449)
> but I don't see how it matters since all participants are equivalent anyway
exactly, all participants are equivalent, and the intention of this `random.sample` is to prove that. maybe it's a little extra to even have this `random.sample`, but participants being equivalent is critical to the intention of this multisig so I thought the randomness was a nice to have
(https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1595783449)
> but I don't see how it matters since all participants are equivalent anyway
exactly, all participants are equivalent, and the intention of this `random.sample` is to prove that. maybe it's a little extra to even have this `random.sample`, but participants being equivalent is critical to the intention of this multisig so I thought the randomness was a nice to have
💬 mjdietzx commented on pull request "tests: improve wallet multisig descriptor test and docs":
(https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1595783876)
Done
(https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1595783876)
Done
💬 mjdietzx commented on pull request "tests: improve wallet multisig descriptor test and docs":
(https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1595792597)
All participants are equivalent [as discussed in related review comment](https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1595783449). Maybe it's extra and we shouldn't even bother with `random.sample`. If you'd prefer that lmk and I can remove
Why I included it in first place:
- all participants being equivalent is a critical property of this multisig. I know it's clear behavior of `thresh`. But I went ahead and asserted it in this test. Otherwise as signers decay, without a change
...
(https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1595792597)
All participants are equivalent [as discussed in related review comment](https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1595783449). Maybe it's extra and we shouldn't even bother with `random.sample`. If you'd prefer that lmk and I can remove
Why I included it in first place:
- all participants being equivalent is a critical property of this multisig. I know it's clear behavior of `thresh`. But I went ahead and asserted it in this test. Otherwise as signers decay, without a change
...
💬 theuni commented on pull request "crypto, refactor: add method for applying the taptweak":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1595821023)
Updated as suggested here: https://github.com/theuni/bitcoin/commits/keypairtaptweak/
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1595821023)
Updated as suggested here: https://github.com/theuni/bitcoin/commits/keypairtaptweak/
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845274)
@glozow pushed some more detailed docs, thanks!
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845274)
@glozow pushed some more detailed docs, thanks!
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845313)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845313)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845340)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845340)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845388)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845388)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845419)
swapped them and updated tests accordingly
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845419)
swapped them and updated tests accordingly
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845454)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845454)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2103229267)
@glozow thanks for the review, all comments addressed
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2103229267)
@glozow thanks for the review, all comments addressed
💬 wiz commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2103237059)
> that might be premature, if we decide to do another genesis block.
From reading this thread it seems like there is rough consensus for the testnet4 genesis block now, and the discussion has mostly shifted to tweaking the difficulty adjustment algorithm and what fancy test transactions to mine into blocks - so I imagine the blockchain might get re-org'd and genesis block probably doesn't change, but as @craigraw said even if it does that's totally fine too.
As for being "premature", to be
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2103237059)
> that might be premature, if we decide to do another genesis block.
From reading this thread it seems like there is rough consensus for the testnet4 genesis block now, and the discussion has mostly shifted to tweaking the difficulty adjustment algorithm and what fancy test transactions to mine into blocks - so I imagine the blockchain might get re-org'd and genesis block probably doesn't change, but as @craigraw said even if it does that's totally fine too.
As for being "premature", to be
...
👍 willcl-ark approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2048629110)
ACK 46890065e59875dd3d736d04f427e410bbd8faed
Without my earlier misunderstanding, this all looks good to me.
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2048629110)
ACK 46890065e59875dd3d736d04f427e410bbd8faed
Without my earlier misunderstanding, this all looks good to me.