💬 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

💬 theStack commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1903322566)
in commit 850962d0074683cccbe0f63c864a70e0f31a8caf: is this a left-over from debugging or introduced intentionally?

💬 theStack commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1903312194)
seems like the picked commit 9c21b8f581542aee0f599f1d8996ac97071bdf6f is out-dated, as this sub-test was already changed to use `generate_keypair` in #30328 (see e.g. https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1878288769)

👍 TheCharlatan approved a pull request: "util: Add missing types in make_secure_unique"
(https://github.com/bitcoin/bitcoin/pull/31464#pullrequestreview-2530954372)
ACK fa397177acfa1006ea6feee0b215c53e51f284de

👍 TheCharlatan approved a pull request: "lint: Move assertion linter into lint runner"
(https://github.com/bitcoin/bitcoin/pull/31435#pullrequestreview-2530958970)
ACK e8f0e6efaf555a7d17bdb118464bd572bd5f3933

💬 laanwj commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2571756240)
Concept ACK.
All in all we should be using decimals or strings for amounts and not floats. Using floats for monetary amounts is dangerous, although arguably it's not critical in tests it's good to give the right example.

⚠️ Liljz8 opened an issue: "git@github.com:Liljz8/contract-metadata.git"
(https://github.com/bitcoin/bitcoin/issues/31606)

✅ pinheadmz closed an issue: "git@github.com:Liljz8/contract-metadata.git"
(https://github.com/bitcoin/bitcoin/issues/31606)

💬 hugohn commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2572082601)
Thanks @bigspider , we’ll take a look.

Our main concern would be compatibility with other wallets. Do you know what wallets support BIP-388 right now (besides Ledger)? Does Sparrow support it?

🤔 Xyvern26 reviewed a pull request: "doc: upgrade bitcoin core license to 2025."
(https://github.com/bitcoin/bitcoin/pull/31605#pullrequestreview-2531147258)
Danke

📝 MarcelMonde opened a pull request: "ANEW.Wall"
(https://github.com/bitcoin/bitcoin/pull/31607)
<!--


Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly: trading

* Any test improvements or new tests that improve coverage are always welcome.
* All other changes shou
...

🤔 MarcelMonde reviewed a pull request: "ANEW.Wall"
(https://github.com/bitcoin/bitcoin/pull/31607#pullrequestreview-2531313008)
Let's End Mental Illness, by paying off our debts and preventing a recession! -Marcell of America

🤔 MarcelMonde reviewed a pull request: "ANEW.Wall"
(https://github.com/bitcoin/bitcoin/pull/31607#pullrequestreview-2531315531)
APPROVE

✅ willcl-ark closed a pull request: "ANEW.Wall"
(https://github.com/bitcoin/bitcoin/pull/31607)

💬 maflcko commented on pull request "doc: upgrade bitcoin core license to 2025.":
(https://github.com/bitcoin/bitcoin/pull/31605#issuecomment-2572523772)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 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-2572540702)
> Thanks @bigspider , we’ll take a look.
>
> Our main concern would be compatibility with other wallets. Do you know what wallets support BIP-388 right now (besides Ledger)? Does Sparrow support it?

BIP-388 is the base for the miniscript implementation used currently in Ledger, BitBox and Jade.
Sparrow only supports the standard multisig types, and I suppose it works with most devices.

The `musig` part of BIP-388 is currently only implemented in Ledger in a test application (details [h
...

💬 maflcko commented on pull request "test: fix typo in mempool_ephemeral_dust":
(https://github.com/bitcoin/bitcoin/pull/31604#issuecomment-2572569354)
lgtm ACK 29bca9713d21916b315c2ca0e9183bf567f39351