AustEcon / bitsv

BitSV: Bitcoin made easy. Documentation:
https://AustEcon.github.io/bitsv
MIT License
96 stars 28 forks source link

Features/whatsonchain #69

Closed AustEcon closed 4 years ago

AustEcon commented 4 years ago

In order to add whatsonchain (the only API that is both free + has reliable support for mainnet and testnet) I needed to simplify the Unspent data type by removing script - i.e. scriptpubkey (otherwise would require 3 x separate api calls just to get utxos... as it is we need 2 x api calls. One for latest block height and another for getting the utxos which are in ElectrumX format). To avoid this we could go further and change the Unspent.confirmations to height instead (which imo we should also do but for now I'm leaving it).

The network.transaction.Transaction, TxInput and TxOutput are purely cosmetic and have been simplified down also to match what is commonly available from various apis.

These changes may cause breaking changes for some but I don't feel I had much choice... As such I propose to release this soon as a 0.11.0 release.

Any thoughts / opinions welcomed. 😃

PS: I do think that for a larger application, it makes sense to keep the scriptpubkey attached to each Unspent to classify how to unlock the coins at signing but it is currently not used by any part of the codebase and would be a huge pain in the arse to keep it in. We know how to spend / sign because there is only one key and it only does p2pkh.

If we do want to keep the scriptpubkey on each utxo... you need to realise that this means that we now need to do a separate api query for each and every utxo to lookup the full parent transaction and parse it to get its scriptpubkey... performance would be atrocious and unacceptable imo. So if we want scriptpubkey, it will require a big redesign to something like using ElectrumX + asyncio etc... a reasonably big job and if I were to do it, I'd probably just make a new library inspired by or forked from bitsv.

ghost commented 4 years ago

Oh boy, big change. give me some time on this one :).

But bug me if you don't hear back.

ghost commented 4 years ago

I don't like changing BitIndex to MatterCloud and this huge WhatsOnChain change in one PR.

I think it's going to make it harder to review and spot bugs. How hard would it be to first do Mattercloud, then adjust for WhatsOnChain, if it makes sense to do so?

AustEcon commented 4 years ago

Mattercloud doesn't even work anyway for 95% of our users.

I have to change all of the APIs at the same time anyway because of the difference in Unspent, Transaction, TxOutput, TxInput.

If I do the Bitindex changes on it's own, bitsv is a completely non-functional library for most users without paying $50/month.

That's why I included it all as an 'omnibus' to take bitsv from broken -> working.

I can undo the renaming though of BitIndex3 -> MatterCloud if you like (maybe concerns over people using this client directly).

I guess my plan was 0.11.0 major breaking release and people will just have to re-tweak their code minimally if a) they directly use the BitIndex3 client b) they directly manipulate Unspent, Transaction, TxInput, TxOutput data types...

I have no choice the way I see it... bitsv is unuseable without doing this.

ghost commented 4 years ago

It's working for me right now with bchsvexplorer. Is there something I'm missing? Just doesn't work on testnet. Obviously more APIs would be better.

AustEcon commented 4 years ago

Yes. Whatsonchain is the only (free) api that works for testnet so is the only one rigged up for PrivateKey to use.

I've left MatterCloud rigged up (if API key provided) to testnet too but I think in reality it only works on mainnet right atm.

https://satoshi.io/ would be a good, robust one to add in for mainnet and testnet (would be a replica of Whatsonchain's api and requires the same ^^ changes)

AustEcon commented 4 years ago

I'm sorry I think I need to merge this. I see no other way forward. If people do not like it we'll just need to have two separate branches or something for compatibility: 0.11.x and 0.10.x (legacy)

ghost commented 4 years ago

Alright. Well tests pass minus one thing for me, so that's good. I'll need to roll this out to SporeStack and see if things are stable.