💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938002644)
I don't think that's actually correct either as then passing SIGHASH_DEFAULT will be unable to sign for non-taproot at the same time.

It's also a much larger change that I think is out of scope for this PR.

💬 theuni commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2628435688)
@ryanofsky Thanks.

I think we might just have been disagreeing on the importance of a unified build for everyone for something that will presumably (in the future) become mission-critical. I can see how from your POV it might seems like I'm just being persnickety about flags, but to me it's about making sure we're being clear (internally and externally) about ownership/canon/etc... the flags were just illustrative (to me) of the lack of clarity there.

Thanks for your patience with me on th
...

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1938013882)
> I am not following. Can you describe the concern?

I would think if anything we are more vulnerable to supply-chain attack **without** this change, because, for example if a few bitcoin core developers conspired and made a version of libmultiprocess with a backdoor and pushed it to github, it wouldn't be obvious to other developers that the hashes pointed to a backdoor version. It also wouldn't be obvious to external observers diffing one version of bitcoin to the next to check for introduct
...

💬 davidgumberg commented on issue "crypto: secure erase memory":
(https://github.com/bitcoin/bitcoin/issues/31744#issuecomment-2628450376)
Thanks for reporting, I've opened #31774 to address this.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938014929)
Yes

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938015888)
Yes. Additionally, specifying `ALL` for taproot means adding another byte to the signature and also changes the sighash.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938016937)
No, this was added because without it there were test failures.

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2628463063)
> disagreeing on the importance of a unified build for everyone for something that will presumably (in the future) become mission-critical.

I really don't think this is true. I think you have made number a lot of claims that sound great, don't stand up to scrutiny. I probably have too. These issues can be hard to reason about, so it's good that we are going through this exercise.

But I don't take security or hardening less seriously than anyone else, and I believe this approach has been 10
...

💬 stickies-v commented on issue "RFC: multiprocess binaries in 29.0":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2628469196)
Thank you for your effort in approaching this issue in a structural way, @ryanofsky . I'm not advocating for or against shipping multiprocess in 29.0 - I could be okay with either, I haven't reviewed the code enough to have an opinion on that yet. My concerns quite closely align with [darosior's write-up](https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2622618673). It appears more review and dog-fooding could be helpful, and I just haven't seen an urgent reason to ship this with 29.
...

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

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1938031202)
This code is unused, the current value just affects testing, but it can literally be set to any positive integer, and the code will work (though at degraded performance for higher values). The real question is what to set the `max_cluster_count` value to in `MakeTxGraph()`, but I think that discussion belongs in #28676.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1938031622)
I have largely rewritten this, and expanded comments. Please have a look and let me know if it is clearer now.

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

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2628480335)

> But if we assume the miner was smart and used the available config options to optimize, they could have already reached 3,996,000 WU by setting `blockmaxweight` to 4,000,000 WU before this change, right?

No, the current behavior on master is that we always reserve `8,000 WU`, regardless of what `blockmaxweight` is. This is actually how I discovered the issue.

See this test https://github.com/ismaelsadeeq/bitcoin/commit/dbc17d1519247e366b39da54e87d0e866709d16a. It fails on master.
...

💬 fjahr commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2628496788)
> No, the current behavior on master is that we always reserve 8,000 WU, regardless of what blockmaxweight is. This is actually how I discovered the issue.

Ok, I am very confused now. In the release notes you write this:

```
- Users who manually set `-blockmaxweight` to its maximum value `4,000,000` can lower
`-blockreservedweight` to `4,000` to maintain the previous behaviour.
```

So you are saying setting `-blockreservedweight` to `4,000` will give you a block with `8000 WU` reserv
...

💬 theuni commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1938048845)
Ok, you're right. I should not have framed this as a potential attack.

We maintain hard-coded hashes in depends for 2 reasons: To protect against real supply-chain attacks (Someone hijacks github and replaces tarballs with malicious ones) and for determinism (our sources can be described exactly).

The output of a depends build is locally deterministic based on the build id, which is comprised of the environment, toolchain, and source hashes. After this change, as far as I can tell, that's
...

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938054826)
Fixed

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938054874)
Fixed

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938054955)
Indeed, Fixed

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938055136)
I've removed this sentence since we allow `DEFAULT` and `ALL`.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938055244)
Done