Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inability to hold multiple tapret tweaks in Tr RGB wallet for the same derivation #183

Open
crisdut opened this issue Apr 30, 2024 · 6 comments
Labels
bug Something isn't working epic Epic task covering multiple steps of implementation refactoring Code refactoring
Milestone

Comments

@crisdut
Copy link
Member

crisdut commented Apr 30, 2024

Description

I discovered a strange behavior when we tried to use TapretKey::derive after making a successful tapret1st. If I retry using the same terminal change of the previous transaction, the pay operation returns TapretKeyError::TapTreeNonEmpty:

use of taproot script descriptors is not yet supported. 
You may also check the latest version of the software which may already support this feature.

This occurs because when creating the PSBT, we use the TapretKey::derive inside of the Psbt::construct_change method (see here), and derivation uses the tap tweak to build a taptree.

In version 0.10, we could derive without tap-tweak; I believe we need to implement something similar.

@zoedberg
Copy link
Contributor

@crisdut could you please tell me on which branch you've found this issue?

@crisdut
Copy link
Member Author

crisdut commented Apr 30, 2024

@crisdut could you please tell me on which branch you've found this issue?

Hi @zoedberg ,

I first found it in beta5, but it remains in beta6 (master). It's easier to find it using rgb-rt as a lib and not in the cli.

@zoedberg
Copy link
Contributor

If I understood correctly what you did is:

  • issued an asset (not sure if this error is connected to a particular schema or happens with every implemented schema)
  • successfully sent the asset from the issuer to another wallet using tapret1st
  • tried to send the asset again from the issuer, using the RGB change produced by previous transfer but failed at PSBT construction time

Is this correct?

It's easier to find it using rgb-rt as a lib and not in the cli.

Why so? Please provide more information

@crisdut
Copy link
Member Author

crisdut commented Apr 30, 2024

Hi @zoedberg, sorry for the delay.

Is this correct?

So so, the first and second steps are correct. But the last step is a little different.

  1. issued an asset (not sure if this error is connected to a particular schema or happens with every implemented schema)

OK!

I think it is not related to a particular schema, but fungible/fractional assets are easier to reproduce because you use the asset change to make the next transfer.

  1. successfully sent the asset from the issuer to another wallet using tapret1st

OK!

  1. tried to send the asset again from the issuer, using the RGB change produced by the previous transfer, but failed at PSBT construction time

Here, if you use Runtime::construct_psbt with TxParams->change_shift = false, the Psbt::construct_change remains use the same change_terminal of the previous (i.e. &/10/1). When Psbt::construct_change calls TapretKey::derive, the derivation result uses terminal + plus taptweak instead of the only terminal, and this causes the error reported.

Why so? Please provide more information.

Because you can use the rgb-rt lib directly, you can modify the TxParams, as mentioned above.

I hope this help

@dr-orlovsky dr-orlovsky added this to the v0.11.0 milestone May 1, 2024
@dr-orlovsky dr-orlovsky added the bug Something isn't working label May 1, 2024
@dr-orlovsky
Copy link
Member

Yes, basically if you derive for a key which already has an associated at-tweak it will always derive it in a tweaked way. It is also the fact that currently we can only handle only single at-tweak per key... Both things should be changed, but it would require some deep refactoring of the whole descriptor and derivation APIs starting from bp-std, and I was planning it for v0.12...

@dr-orlovsky
Copy link
Member

I believe it is better to leave this as is for v0.11 to prevent people from creating more than one tweak per key and losing their funds. With v0.12 we will refactor the api and allow re-use of key derivation paths

@dr-orlovsky dr-orlovsky removed this from the v0.11.0 milestone May 1, 2024
@dr-orlovsky dr-orlovsky added epic Epic task covering multiple steps of implementation refactoring Code refactoring labels May 1, 2024
@dr-orlovsky dr-orlovsky added this to the v0.12.0 milestone Jun 8, 2024
@dr-orlovsky dr-orlovsky changed the title TapretKeyError::TapTreeNonEmpty Error Inability to hold multiple tapret tweaks in Tr RGB wallet for the same derivation Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working epic Epic task covering multiple steps of implementation refactoring Code refactoring
Projects
None yet
Development

No branches or pull requests

3 participants