leather-io / extension

Leather browser extension
https://leather.io
MIT License
294 stars 142 forks source link

Add optional OP_RETURN parameter for API to initiate BTC transaction #3848

Closed markmhendrickson closed 1 year ago

markmhendrickson commented 1 year ago

For effectively supporting memos within BTC transactions. Requested by the ALEX team.

https://hirowallet.gitbook.io/developers/bitcoin/sign-transactions/fully-signed-bitcoin-transactions

fbwoolf commented 1 year ago

Not 100% sure on the implementation here, but it does look btc-signer lib handles opcodes. Prob wouldn't take long to figure out.

markmhendrickson commented 1 year ago

ALEX has subsequently indicated a preference for OP_RETURN over OP_DROP here

fbwoolf commented 1 year ago

I am not understanding here why we would have to choose the opcode to handle as the wallet? The opcode can be anything the app wants, right? We would simply use it for the tx output. @fiftyeightandeight are you hoping to pass us an encoded script, or what are you hoping to pass us? What exactly are you asking for here from the api?

fiftyeightandeight commented 1 year ago

I am not understanding here why we would have to choose the opcode to handle as the wallet? The opcode can be anything the app wants, right? We would simply use it for the tx output. @fiftyeightandeight are you hoping to pass us an encoded script, or what are you hoping to pass us? What exactly are you asking for here from the api?

Hi @fbwoolf we are looking to encode arbitrary message in the tx output, like the below. We are aware that currently OKX Wallet supports this via OP_RETURN. https://mempool.space/tx/8bae12b5f4c088d940733dcd1455efc6a3a69cf9340e17a981286d3778615684

fbwoolf commented 1 year ago

Hi @fbwoolf we are looking to encode arbitrary message in the tx output, like the below. We are aware that currently OKX Wallet supports this via OP_RETURN.

https://mempool.space/tx/8bae12b5f4c088d940733dcd1455efc6a3a69cf9340e17a981286d3778615684

I understand that, but how do you want to pass the data to us? What data type?

fbwoolf commented 1 year ago

Also, curious here, if you want more control over the tx, why not use PSBTs?

fbwoolf commented 1 year ago

I am reluctant here to say, We are going to handle OP_RETURN, it doesn't make any sense. Apps should be able to pass us any opcode in a script to be applied to a tx output. I think you should pass us the encoded script, we shouldn't know what it is. @kyranjamie your thoughts here on the api change?

fiftyeightandeight commented 1 year ago

If you could add an optional string parameter (say, opReturn) in the sendTransfer tuple and apply that opcode to the tx output, that will work for us (https://hirowallet.gitbook.io/developers/bitcoin/sign-transactions/sending-bitcoin).

Below is an example of a Bitcoin wallet (imToken) on its handling of OP_RETURN from UI perspective.

telegram-cloud-photo-size-5-6104845637154616606-y

telegram-cloud-photo-size-5-6104845637154616607-x

fbwoolf commented 1 year ago

Ok, got it, thanks for the feedback. 👍

fbwoolf commented 1 year ago

@markmhx @mica000 wondering about the UI here with Advanced again, how many users really know what an opcode is?

EDIT: Maybe we aren't displaying this for the user?

Wondering also if we should make it more flexible, where the app passes us a param, opcode and opcodeData? Nvm here, prob not possible to handle all. We can expand later.

kyranjamie commented 1 year ago

My understand aligns with @fbwoolf's here

I am not understanding here why we would have to choose the opcode to handle as the wallet?

OP_RETURNs are described in the transaction itself. This guide shows how to create a tx with an op return https://bitcoindev.network/guides/bitcoinjs-lib/embedding-data-with-op_return/

@fiftyeightandeight what stops you from following these steps, turning the tx into a PSBT, and asking the Hiro Wallet to sign it?

Imo sendTransfer should not have an op return flag. This is simple "send money" feature, and adding op return might confuse our intentionally simplified API. For complex use cases, we have the PSBT feature that intends to allow any possible transaction.

See a demos an unspendable return here.

fiftyeightandeight commented 1 year ago

Hi @kyranjamie yes, we can handle this as part of PSBT, but were also wondering if there could be an easier way (for the casual users) to include "memo" when sending a transaction. I also see your point on sendTranfer staying as a simple "send money" feature, though being able to include memo in that simple feature would be nice.

fbwoolf commented 1 year ago

Closing this issue in favor of apps using PSBTs for this functionality.