Bitcoin-com / bitbox-sdk

BITBOX SDK for Bitcoin Cash
https://developer.bitcoin.com/bitbox
MIT License
89 stars 62 forks source link

Consider deprecating sendRawTransaction() with GET calls #129

Open christroutner opened 5 years ago

christroutner commented 5 years ago

This may be a bit of a corner case, but I think the bad UX will continue to plague users and the BITBOX team if not nipped. The scope of this issue is to remove GET calls from BITBOX to rest when executing the RawTransactions.sendRawTransactions(hex) endpoint.

Background

I was working with the bch-cli-wallet and tried to send all the funds from my Honest.cash wallet to another address. The wallet is made up of a lot of small UTXOs. The original command I had in the bch-cli-wallet to transmit a transaction was:

const txid = await this.BITBOX.RawTransactions.sendRawTransaction(hex)

and it errored out with this message:

Error in util.js/broadcastTx()
Error in send-all.js/run:  <html>
<head><title>414 Request-URI Too Large</title></head>
<body bgcolor="white">
<center><h1>414 Request-URI Too Large</h1></center>
<hr><center>cloudflare</center>
</body>
</html>

Because that endpoint was ultimately making a GET call to the rest.bitcoin.com server, it was putting he transaction data in the URI. Because the transaction had so many inputs, it went over the maximum allowed size for a URI.

I was able to fix the problem by using this command instead: const txid = await this.BITBOX.RawTransactions.sendRawTransaction([hex])

By encapsulated the tx hex inside an array, BITBOX used the POST call instead of a GET call and the large tx body traveled in the POST body instead of the URI.

Here is the TXID on the block explorer if you'd like to see it. There are a lot of inputs and single output: 4c9b2504543c8a31d7d5d1b7b6e43114cabbcb027608433a04c9906e9ca9d86a

Summary

I recommend that we alter the BITBOX SDK source code to always send the sendRawTransaction() endpoint as a POST call. This is not a breaking change. Users can continue to use the same syntax and these changes would be backwards compatible. Internal to the library, we would simply force a POST call instead of a GET call.