hirosystems / connect

A library for building excellent user experiences with Stacks.
https://connect.stacks.js.org
MIT License
76 stars 40 forks source link

Support BTC Transfer Requests #283

Closed yknl closed 1 year ago

yknl commented 1 year ago

Add a openBTCTransfer function call to allow wallets to support BTC transaction requests from apps.

The arguments for this function would be similar to openSTXTransfer:

Description Type Example
recipient The recipient BTC address string '1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN2'
amount The amount (in satoshis) to transfer Integer (e.g. number, bigint) 10000

Stacks wallets like Xverse already support Bitcoin and some apps/projects like the BTC NFT Working Group require this functionality.

aulneau commented 1 year ago

small nit: this would likely actually be more in-line by existing in the wallet repo, as connect is just an interface for whatever the wallet exposes. Additionally, it would be good to start working on a SIP for the provider that injects this kind of event into webpages. Ideally xverse and the hiro wallet would work on a standard together

yknl commented 1 year ago

small nit: this would likely actually be more in-line by existing in the wallet repo, as connect is just an interface for whatever the wallet exposes.

You can essentially argue the same from the wallet perspective. It's probably best to start working from both ends to prevent a deadlock. Regardless, I think this particular issue is needed here to track the implementation on Stacks connect's side.

Additionally, it would be good to start working on a SIP for the provider that injects this kind of event into webpages. Ideally xverse and the hiro wallet would work on a standard together

I'm leaning towards being able to get this through quicker. In which case we skip the SIP process. Especially since the original stacks provider implementation did not have a SIP. Starting a SIP for this now would probably require us to at least define the existing Stacks provider implementation.

We can always revisit this and create a SIP afterwards. In the mean time, we can release a beta onto an experimental tag on NPM. Which would solve the immediate problem that the BTC NFT working group has.

aulneau commented 1 year ago

Sure! Totally agree, additionally, if this is for the btc nft working group, we should make an issue in the micro-stacks repo as the MVP app is not currently using stacks connect ☺️

yknl commented 1 year ago

Cool, we can create an issue there as well.

janniks commented 1 year ago

Yes, we should definitely check-in with the Hiro Wallet team on this.

A beta tag works for now. But I'd also push for starting a SIP (even if it's a draft during the release of these features), simply to avoid any breaking changes later that may impact application developers.

markmhendrickson commented 1 year ago

I've created related issues for SIPs and the Hiro Wallet here:

I also suspect we can design this in line with openSTXTransfer and we can pursue the SIP in parallel with initial implementations.

friedger commented 1 year ago

I suggest to add an optional memo field

whoabuddy commented 1 year ago

@friedger how would that be done with BTC? My understanding is you'd need a special script to pass data, e.g. OP_RETURN (which invalidates the tx) or OP_DROP, etc.

janniks commented 1 year ago

Yeah, there’s no dedicated memo field in “normal” BTC transfers (even though some payment protocol BIPs etc. aim to add something similar). I’d recommend a plain BTC tx for now, anything more would have to be sketched out to fit the needs of the use-case…

friedger commented 1 year ago

We have btc wire formats for different stack features. Like stx transfer with memo: https://github.com/stacks-network/stacks-blockchain/blob/26bfd5fcdc1f25106288f18ced05290b569f6abb/src/chainstate/burn/operations/transfer_stx.rs

Can we add them here as well?

Or should we add them in a new issue? openBtcStxTransfer, openBtcStackStx, openBtcDelegateStx,...

janniks commented 1 year ago

That should work, I’ll look into it further. (Will create new issues, for these — they can be added later as well)

whoabuddy commented 1 year ago

I think this got off track a little bit - the idea here is to provide a simple method that will initiate a Bitcoin transaction with the connected wallet.

That way a front-end could pass the destination and amount to this function, and a prompt would be opened similar to how we create STX transfers now for the user to sign and broadcast.

Our initial use case would be allowing someone to mint a Bitcoin NFT by sending Bitcoin directly to the artist address, and this flow would simplify the steps required versus showing them an address or QR code and/or expecting them to copy all the information correctly.

I'll add my comments regarding the STX-on-BTC transactions in the other issue - but I think this should be separate as it's just initiating a transfer - nothing fancy!

yknl commented 1 year ago

Since you might want to send to more than one recipient in a single Bitcoin transaction. We should probably modify the interface to accept an array of recipient addresses and amounts.

Additionally, the need to include data in the OP_RETURN or similar seems to be increasing. We should also add an additional optional argument for that.

janniks commented 1 year ago

Seems important, yes!

These APIs sound interesting, but may grow quickly in scope and need more-and-more new functions/interfaces.

I have two thoughts/questions -- still early

1: A more generic RPC

Similar to other existing wallet-protocols, an interface could be provided for requesting and listening:

The could send arbitrary json-content as options. Allowing for simpler development on the wallet end, especially for new request-types (e.g. here btc transfers). Similar to non-standard headers, a non-standard wallet-request could be prefixed by 'x-btc-transfer' until it becomes standardized. This way wallets easily can provide early features and e.g. later simply alias previous work to the standard.

Browser clients can expose the rpc, but in addition provide nicely typed interfaces for the most basic/standard actions.

2: Any Transaction API

Separately, would it make sense to also add an arbitrary tx API? i.e. the client/connect sends a hex-string (serialized partial tx) to the wallet, and the wallet then parses the string and determines whether it supports the tx-type etc.

openWallet('bitcoin', '0x58c1...de8e') // unsigned tx serialized

Not sure if this is a good idea -- maybe it adds additional burden in verifiying that transactions are not confusing/malicious...

kyranjamie commented 1 year ago

+1 to a simpler RPC based API, like Ethereum's requestAccounts EIP. We've been hoping to add something similar to the Hiro Wallet.

This'll make it easier to implement cross-wallet standards, and remove the unnecessary JWT encoding/decoding currently used. I've been working on a SIP describing this, see here https://github.com/stacksgov/sips/issues/117

yknl commented 1 year ago

We've added support for WalletConnect in Xverse recently. The API is similar to what has been described here.

An example contract function call via wallet connect looks like:

const result = await client.request({
  chainId: "stacks:1", 
  topic: session.topic, 
  request: {
    method: "stacks_contractCall",
    params: {
      pubkey: address, 
      postConditions,
      contractAddress: "SP1H1733V5MZ3SZ9XRW9FKYGEZT0JDGEB8Y634C7R",
      contractName: "my_contract_name",
      functionName: "transfer",
      functionArgs: [
        uintCV("123"),
        standardPrincipalCV("SP1BEEN4WP9YT42PR70FYSG6C3WFG54QJDEN0KWR"),
        standardPrincipalCV("SP3F7GQ48JY59521DZEE6KABHBF4Q33PEYJ823ZXQ"),
        noneCV(),
      ],
      postConditionMode: PostConditionMode.Deny,
      version: "1",
    },
  },
});

The full reference is here: https://docs.xverse.app/wallet-connect/reference/api_reference

We could simply adopt this standard for wallet API across platforms. There's little reason to create something unique to the Stacks ecosystem. With this, developers coming from other ecosystems could easily adapt their apps considering the ubiquity of WalletConnect. This also works for Bitcoin transactions for which we're finalizing the standard.

janniks commented 1 year ago

Great point @yknl -- thx for sharing

I agree, but I would also like to see it easily possible to keep using simple APIs without a lot of additional things like wallet-connect utils/sessions, etc.

Maybe there'e a common ground to standardize the rpc-methods and make them usable accross different interfaces.

janniks commented 1 year ago

I'll close this for now, folks have their working solutions right now, and the upcoming ideas we can discuss in the working group. If anyone disagrees feel free to reopen