Add Taproot (tr) output descriptor support #101
Conversation
| case parse_closing_bracket(remaining) do | ||
| {:ok, rest_with_checksum} -> | ||
| checksum = extract_checksum(rest_with_checksum) | ||
| type = if sorted?, do: :sortedmulti, else: :multi |
There was a problem hiding this comment.
Should we verify the sortedness of the keys if sorted?
| defp extract_checksum(""), do: nil | ||
| defp extract_checksum("#" <> checksum), do: checksum |
There was a problem hiding this comment.
We should validate the checksum
| Converts an output descriptor to a script. | ||
| """ | ||
| @spec to_script(t()) :: Script.t() | ||
| def to_script(%__MODULE__{type: :pk, key: key}) do |
There was a problem hiding this comment.
i think in each of these, you will need to add a when key != nil otherwise key: key will always match.
There was a problem hiding this comment.
also, there's no handling of cases where an xpub is provided, meaning this function would crash. we should add sth like def to_script(_), do: {:error, "..."} or, even better, allow the caller to pass in an opts which contains a derivation path, or other necessary args to derive a valid script.
| serialized = Point.sec(key) | ||
| # Remove the first byte (compression marker) to get the x-only key | ||
| binary_part(serialized, 1, 32) |
There was a problem hiding this comment.
we have a function called Point.x_bytes which does this.
|
|
||
| binary when is_binary(binary) and byte_size(binary) == 33 -> | ||
| # Remove first byte from compressed pubkey | ||
| binary_part(binary, 1, 32) |
There was a problem hiding this comment.
we should validate this is a valid pubkey.
| end | ||
|
|
||
| # Create a pay-to-taproot script: OP_1 <32-byte x-only pubkey> | ||
| {:ok, script} = Script.create_p2tr(xonly_key) |
There was a problem hiding this comment.
the create_p2tr function does take a Point fwiw.
| Converts an output descriptor to an address. | ||
| """ | ||
| @spec to_address(t(), atom()) :: {:ok, String.t()} | {:error, String.t()} | ||
| def to_address(descriptor, network \\ :mainnet) do |
There was a problem hiding this comment.
just my opinion, but i'd prefer avoiding a default value for network, like in Script.to_address
| script = to_script(descriptor) | ||
| Script.to_address(script, network) |
There was a problem hiding this comment.
nit (feel free to ignore): pipe?
| # Test script generation | ||
| script = OutputDescriptor.to_script(descriptor) | ||
| assert Script.is_multi?(script) | ||
| end |
There was a problem hiding this comment.
Please add tests of parsing and serializing the examples provided here: https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md
| @@ -0,0 +1,140 @@ | |||
| defmodule Bitcoinex.OutputDescriptorTest do | |||
There was a problem hiding this comment.
test files should be named ..._test.exs
2 participants
Description
This PR adds support for Bitcoin Taproot (tr) output descriptors, extending the current functionality to include Segwit v1 addresses. The implementation follows the Bitcoin Core descriptor specification and supports key path spending for Taproot addresses.
Features Added
tr()descriptorsImplementation Details
tr()descriptorsCurrent Limitations
Testing
All tests pass, including the new Taproot descriptor tests that verify:
This makes the library more complete by supporting all current Bitcoin address types, including the latest Taproot addresses, which enable more efficient and private Bitcoin transactions.