bitcoin / bitcoin

Bitcoin Core integration/staging tree
https://bitcoincore.org/en/download
MIT License
78.39k stars 36.16k forks source link

Unexpected behaviour when using `sortedmulti_a` descriptor #30518

Open KonradStaniec opened 1 month ago

KonradStaniec commented 1 month ago

Is there an existing issue for this?

Current behaviour

Currently running:

getdescriptorinfo "tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),multi_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e)))"

will respond with success for such descriptor:

{
  "descriptor": "tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),multi_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e)))#dy2rfhga",
  "checksum": "dy2rfhga",
  "isrange": false,
  "issolvable": true,
  "hasprivatekeys": false
}

Though running the same command with the same data but with sortedmulti_a instead of multi_a will generate error:

getdescriptorinfo "tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),sortedmulti_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e)))"

and error:

'and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),sortedmulti_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e))' is not a valid descriptor function

Expected behaviour

I would expect that the call

getdescriptorinfo "tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),sortedmulti_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e)))"

will also succeed

Steps to reproduce

call command

getdescriptorinfo "tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),sortedmulti_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e)))"

Relevant log output

No response

How did you obtain Bitcoin Core

Other

What version of Bitcoin Core are you using?

v27.0

Operating system and version

MacOs Sonoma 14.5

Machine specifications

No response

willcl-ark commented 1 month ago

Thanks for the report.

I would also expect to see these decodes behave the same as the only difference between them should be the sorting. I didn't investigate further yet, but incidentally I see the same behaviour in a toy rust-miniscript parser I have:

Loading descriptor: tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),multi_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e)))
Loaded descriptor
Descriptor type: Tr
Segwit version: V1
Has wildcard: false
Max weight to satisfy: 272
Satisfaction structure:
  Taproot:
    Internal Key: 50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0
    Has script tree

Loading descriptor: tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),sortedmulti_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e)))
Error analyzing descriptor: unexpected «sortedmulti_a(3 args) while parsing Miniscript»
sipa commented 1 month ago

This apparent inconsistency is due to the fact that multi_a exists both as a top-level descriptor fragment and inside miniscript, but sortedmulti_a only as a top-level fragment.

We should probably add sortedmulti_a to miniscript too.

darosior commented 1 month ago

I'm not sure. sortedmulti_a was introduced for compatibility with legacy pre-descriptor practices. I can't think of any reason to ever prefer sortedmulti_a/sortedmulti to multi_a/multi inside Miniscript. And there is no legacy compatibility concern since any Miniscript-using wallet must have been created in a post-descriptor world.

So i don't think it's worth adding sorted* fragments to Miniscript.

sipa commented 1 month ago

@darosior I vaguely recall discussing this before, but I don't remember the reasoning. Imagine some particular application uses a specific fancy timelock/hashlock/2-of-3 combination that (when the script path is used) readily gives away it's a transaction by that particular application due to its unusual script. If the descriptor for that uses multi_a(mobile_xpub,desktop_xpub,server_xpub), then the location of the key signed with will additionally publish which parties are involved on chain. Using sortedmulti_a there instead would reduce that information leak.