Avoid provably unspendable outputs
#50
Conversation
There was a problem hiding this comment.
Am I right that the problem is only related to the PSBTs containing OpRet commitments?
Please see my review comment. We need to move all checks into command-lines, no prints should happen inside the library code. This will also help to solve the compexity of the review: too many identation changes such that the actual logic that was added/changed remains hard to find.
No, this occurs with TapRet commitments as well. Example: https://mempool.space/testnet/tx/94dd3f7eb24d63306917796b363f0bb6addc3647a4e5838560020508ea3c1e4f
Sorry about this mistakes, I will fix this and push again. |
crisdut
force-pushed
the
fix/avoid-unspentable
branch
from
e4db696 to
4f9df2a
Compare
|
Hi @dr-orlovsky, I followed your recommendations and move the validation to command-line. |
About TapRet: I think the problem occurs because the Expected ScriptPubKey Current ScriptPubKey Could you confirm my assumption? If the assumption is correct, I will investigate more and try to solve it in another PR (probably in another project). |
There was a problem hiding this comment.
Sorry for being unclear, what I ment is that the library should return a proper error, which has to be printed in the cli tool, but the change output still should be generated on the lib side. Otherwise it will be impossible to use the lib from the wallets since they would have to duplicate the change creation logic.
Don't worry, It's make sense! I'll treat this event as a error. However, I am investigating the problem I mentioned above. |
crisdut
force-pushed
the
fix/avoid-unspentable
branch
from
4f9df2a to
e648f03
Compare
|
I will close this PR for now, I found the bug above in I will start tests with both methods (opret and tapret). If the problem persists, I will reopen the PR. Thanks |
|
@crisdut Which bug is that? I just checked https://github.com/BP-WG/bp-core/issues |
|
Oh, sorry, @cryptoquick, I wasn't clear. I found the bug in code of the I think the error is here. The code writes the commitment directly in the output script, but the correct way is to write it in the most-right leaf of the taptree. This method is called inside of the method |
|
@crisdut Wow, good find! I don't think I would have noticed that if I was just glancing at the code. I'd be very curious if that'd resolve the issue. |
|
Hey @crisdut! Any update on this issue? Maybe it would be better to open one directly on |
@crisdut did some digging recently and has opened two issues on this: |
Yes, I see. Well, I will publish my PR about TapRet in this week. I will open other PRs to complete the workflow with LNPBP stack. |
|
Hi @cryptoquick and @louneskmt, I updated the discuss: https://github.com/BP-WG/bp-core/discussions/20#discussioncomment-3966106 |
Description
I made tests with RGB, and the PSBT always generate a 'provably unspendable' bitcoins after mining.
This situation occurred because the 'btc-cold construct' adds satoshis remaining in the taproot output, even setting the value in the change address.
I changed the method to spend only the fee and add the value remaining at the change address. See example bellow:
The nonstd output is the outpoint used to anchor the RGB asset.
PS: I talked about this situation in another PR (link here and here)