Blockstream / Jade

Jade hardware wallet
MIT License
322 stars 50 forks source link

liquid: elementsd burn outputs are shown as Unknown address #122

Open LeoComandini opened 7 months ago

LeoComandini commented 7 months ago

Elements Core creates burn outputs with a single OP_RETURN [1]

            } else if (name_ == "burn") {
                CScript datascript = CScript() << OP_RETURN;
                CAmount nAmount = AmountFromValue(output[name_]);
                out.nValue = nAmount;
                out.scriptPubKey = datascript;

Jade uses wally_scriptpubkey_get_type to recognize and render OP_RETURN and burn outputs. [2][3]

However wally_scriptpubkey_get_type considers a single OP_RETURN as an unknown script type

>>> assert wally.scriptpubkey_get_type(b"\x6a\x00") == wally.WALLY_SCRIPT_TYPE_OP_RETURN
>>> assert wally.scriptpubkey_get_type(b"\x6a") == wally.WALLY_SCRIPT_TYPE_UNKNOWN

Thus burn outputs consisting in <OP_RETURN> only are shown as "To: Unknown address".

Should we change jade code to render such burn outputs with "To: Burning Asset - ..."? Or should we change wally to have assert wally.scriptpubkey_get_type(b"\x6a") == wally.WALLY_SCRIPT_TYPE_OP_RETURN?


[1] https://github.com/ElementsProject/elements/blob/master/src/rpc/rawtransaction_util.cpp#L328-L332 [2] https://github.com/Blockstream/Jade/blob/master/main/utils/address.c#L125 [3] https://github.com/Blockstream/Jade/blob/master/main/utils/address.c#L150

JamieDriver commented 7 months ago

I believe Jade should render elements OP_RETURNs that have a sats amount as a burn, as you suggest - but yes, agree it relies on wally.scriptpubkey_get_type() identifying the output/script as an OP_RETURN.

LeoComandini commented 7 months ago

Change on the wally side https://github.com/ElementsProject/libwally-core/pull/448

JamieDriver commented 2 months ago

Jade fw 1.0.31 includes wally version 1.3.0, which I believe includes your changes in this area - so should now be fixed. :+1: