search

MetacoSA / NBitcoin

Comprehensive Bitcoin library for the .NET framework.
MIT License
1.86k stars 840 forks source link

Remove "3 pubkeys in top level multisig" limit #1151

Closed dennisreimann closed 1 year ago

dennisreimann commented 1 year ago

This was introduced in #696 and it seems like an arbitrary limit.

@joemphilips What is the reason for this?

joemphilips commented 1 year ago

This limit is taken from original implementation in bitcoind https://github.com/bitcoin/bitcoin/blob/71abee86dba8f311775104c518644c55c3e123cf/src/test/descriptor_tests.cpp#L440

Why you want to use bare multisig? IIUC it is deprecated and you should use wsh(multi()) or sh(multi()) or sh(wsh(multi()))

dennisreimann commented 1 year ago

This isn't a bare multisig, I'm coming across it in a wsh(sortedmulti(2,[4 PUBKEYS])) scenario when importing a wallet file containing that output descriptor in BTCPay Server.

But now that you hinted at that, I think the error isn't in the implementation here, but in our DerivationSchemeParser, which uses OutputDescriptor.Parse recursively, @NicolasDorier.

dennisreimann commented 1 year ago

@joemphilips I was able to fix this issue on our end — thanks for the pointer, you are correct!