📝 Eunovo converted_to_draft a pull request: "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict"
(https://github.com/bitcoin/bitcoin/pull/29680)
This PR implements a fix for the issue described in https://github.com/bitcoin/bitcoin/issues/29435.

The problem is that the wallet is unable to abandon transactions that have unrelated parent conflicts. The solution implemented here, augments the mempool transaction `REPLACED` signal with the double-spending transaction which the wallet stores and watches for in Block notifications. A map is added to the wallet to track conflicting tx ids and their child transactions. The entry is erased wh
...

🤔 TheCharlatan reviewed a pull request: "mining: bugfix: Fix duplicate coinbase tx weight reservation"
(https://github.com/bitcoin/bitcoin/pull/31384#pullrequestreview-2530880952)
Approach ACK

💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903255292)
Typo nit: `s/for coinbase/for the coinbase/`

💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903257389)
Nit: I don't think adding these comments after an include is useful. They can become stale easily and it adds to the general inconsistencies of the codebase. Can you remove it again?

💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903271412)
Is there a material difference between this test block and the previous one that actually exercises the default coinbase weight? Could more exact weight accounting where the total weight is checked with `assert_equal` instead of `assert_greater_than_or_equal` in `verify_block_template` be useful?

💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903265703)
Nit: Since you are creating a constant for the normal feerate, maybe create one for the fee rate of the large transactions too?

💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903276616)
Does this have an effect on the log line in `miner.cpp::158`? Maybe clarify there that the numbers are given without the coinbase?

💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903268301)
Nit: This is a bit confusing, I think something like `(LARGE_TXS_COUNT - 1) + NORMAL_TXS_COUNT` would be more accurate, or just give the number like you do in the next test block.

💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903278355)
I'm a bit confused by this second sentence. How does the behaviour introduced here allow for overriding the default?

✅ fjahr closed a pull request: "sync: improve CCoinsViewCache ReallocateCache - 2nd try"
(https://github.com/bitcoin/bitcoin/pull/30370)

💬 fjahr commented on pull request "sync: improve CCoinsViewCache ReallocateCache - 2nd try":
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2571656801)
Alright, closing if the effect isn't significant. I have a few too many things on my plate at the moment anyway so maybe someone else wants to investigate further from here.

📝 kehiy opened a pull request: "doc: upgrade Bitcoin Core license to 2025"
(https://github.com/bitcoin/bitcoin/pull/31605)

💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304101)
fixed

💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304182)
`hasBlocks()` is already used one line above where I use `getPruneHeight()`. The problem is it doesn't tell us if we have been pruning or not.

Is there a concerns with exposing `GetPruneHeight()` explicitly? I think the function could be moved outside of RPC code, it just wasn't needed anywhere else so far.

💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304370)
Yeah, this works, I replaced it. Thanks!

💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2571684342)
Addressed feedback from @mzumsande

💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1903304573)
IIUC trying to infer a multisig redeem script that is too large under a p2sh context will return `addr(...)#...` instead of nullptr. Is this what you were going for?

💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1903306172)
> IIUC trying to infer a multisig redeem script that is too large under a p2sh context will return `addr(...)#...` instead of nullptr. Is this what you were going for?

This diff ensures that `nullptr` is returned under p2sh context (if that's what you were going for) and adds a test case that covers this if block.

```diff
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index a2eb764706..7bc2f8cc88 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cp
...

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2571716425)
> Sample descriptors: `tr(musig([15d62cdf/87'/0'/0']xpub6CpM1svHYyNMTVdmDh5syFXCJHKctJNajbyLEdA8pAgAeg1jotmg9G1aVkND5Rzf37uhwhs8o2Lvq22iRpWwcbNGCrYxAozQfYQYi1eduES/**,[7f15646b/87'/0'/0']xpub6ChFTmSdBrhN3D16Rna7hJVQe4w56Gx83U4uNhT3oJaEXiPv7LKnY2gXi3FbbusCb145c3SMEUsSLMRdkxa82MNKqkatnK5b77BXPc3aK8h/**),{{{pk(musig([15d62cdf/87'/0'/0']xpub6CpM1svHYyNMTVdmDh5syFXCJHKctJNajbyLEdA8pAgAeg1jotmg9G1aVkND5Rzf37uhwhs8o2Lvq22iRpWwcbNGCrYxAozQfYQYi1eduES/**,[07895d1c/87'/0'/0']xpub6DF4oz8Ws6Qcd87qKeKFJCMMvc
...

🤔 theStack reviewed a pull request: "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases"
(https://github.com/bitcoin/bitcoin/pull/31495#pullrequestreview-2530939442)
Concept ACK