💬 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_r1949653191)
> You mean a transaction with both a taproot and non-taproot input?

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_r1949655955)
> Is it because the `finalizepsbt` doesn't actually sign the inputs again because there are signatures already in the PSBT albeit using a different sighash type?

`finalizepsbt` does not actually sign even though it calls the sign functions. It does not have access to private keys.

> Shouldn't `PSBTInputSignedAndVerified()` fail here because the sighash type doesn't match with the signature?

The BIP states that the sighash field is merely a suggestion and signers are allowed to ignore it
...

💬 achow101 commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r1949663437)
The benefit is in the MuSig2 PRs where `GetPrivKey` can insert multiple private keys to the `FlatSigningProvider`, none of which are the private key for the MuSig2 aggregate pubkey itself.

For the `ConstPubkeyProvider`, it's useful to have another `GetPrivKey` that returns the single private key since this is behavior is used in a few places within this class's member functions.

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1949666663)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1949666789)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1949666916)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1949667018)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1949667173)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1949667324)
Removed "double".

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1949667494)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1949667700)
Done

💬 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_r1949668555)
Done

💬 ryanofsky commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2648890461)
> @ryanofsky [ab831be](https://github.com/bitcoin/bitcoin/commit/ab831be2a93d9c30408192ca4eaa1552ac6bc3dc) didn't help, same `undefined reference to `LLVMFuzzerTestOneInput'`: https://cirrus-ci.com/task/5233064575500288?logs=ci#L3205

Thanks, it seems like only option here it to get rid of the mputil library in libmultiprocess. Point of that library was to allow code to be shared between multiprocess runtime library and multiprocess code generator and not need to be compiled twice for no reaso
...

💬 furszy commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#issuecomment-2648901099)
Thanks for the review Eunovo. Feedback tackled.

> While testing, I noticed that trying to abandon a tx in mempool wasn't covered in the functional tests

That seems to be material for a different PR (a quick one). Happy to ACK it if you push it.

👋 fjahr's pull request is ready for review: "fuzz: Extend mini_miner fuzz coverage to max block weight"
(https://github.com/bitcoin/bitcoin/pull/31803)

💬 fjahr commented on pull request "fuzz: Extend mini_miner fuzz coverage to max block weight":
(https://github.com/bitcoin/bitcoin/pull/31803#issuecomment-2648910685)
https://github.com/bitcoin/bitcoin/pull/31384 was merged, so this is rebased and ready for review.

💬 Eunovo commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#issuecomment-2648918983)
> That seems to be material for a different PR (a quick one). Happy to ACK it if you push it.

It'd be a really tiny PR. I think you can just add it but I'm happy to open a new one if needed.

💬 Eunovo commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#issuecomment-2648923341)
ReACK https://github.com/bitcoin/bitcoin/pull/31794/commits/409241db5dca0b23f5c7714f99be52411fc5541e

📝 mzumsande opened a pull request: "test: add missing sync to p2p_tx_download.py"
(https://github.com/bitcoin/bitcoin/pull/31837)
If the outbound peer hasn't processed the inv before the mocktime bump, it won't be preferred after the timeout, failing the test . Therefore, add a sync, just like there is one in the previous `send_message` calls.

Fixes #31833

💬 mzumsande commented on issue "ci: intermittent p2p_tx_download.py timeout (test_preferred_tiebreaker_inv)":
(https://github.com/bitcoin/bitcoin/issues/31833#issuecomment-2648924987)
Looks like it's just a missing sync, see #31837.