hodgerpodger / staketaxcsv

python repo to create blockchain CSVs
MIT License
251 stars 115 forks source link

Add cosmoshub-2 and cosmoshub-1 support #197

Closed liamsi closed 2 years ago

liamsi commented 2 years ago

By digging a bit through the code it seems that it should be fairly easy to add support for older cosmos hub versions as well.

The API used to retrieve cosmoshub-3 txs is in fact the same as for hub-2 and hub-1. Cosmostation added this not too long ago (see e.g. here or here).

I think by removing the filtering in: https://github.com/hodgerpodger/staketaxcsv/blob/e35cae782467b28836b2fa85e88686a73fd80ad3/src/atom/cosmoshub3/api_cosmostation.py#L28-L29 you could actually return all Txs.

Is that sth you would consider adding (i.e. would you accept a pull request that extents this to hub-2/-1?) or is there a particular reason why this is limited to hub-3?

hodgerpodger commented 2 years ago

Nice find. I'm guessing you're right.

I believe I didn't explore examples of users with cosmoshub-1, cosmoshub-2 data at the time. Main thing is just finding examples via mintscan etc for private testing to confirm you're right.

Definitely would approve a PR for this change if you feel so inclined. I should get to it next week otherwise.

liamsi commented 2 years ago

I'm not in a rush. Will try to find examples that I can share with you in the meantime.

Something else I've noticed when trying this locally is that this line: https://github.com/hodgerpodger/staketaxcsv/blob/1d5ac41a96fc4ef73bb2fd5aa21acb4ef8a5cdc6/src/atom/cosmoshub3/api_cosmostation.py#L32 always returns None as the result contains only 50 results per request for me (even though there are more txs). When changing this limit to 50: https://github.com/hodgerpodger/staketaxcsv/blob/1d5ac41a96fc4ef73bb2fd5aa21acb4ef8a5cdc6/src/atom/cosmoshub3/api_cosmostation.py#L6 I get more pages than the single first one with 50 txs. But maybe you have other limits with the mintscan team? (I also noticed that the rate limit for me is quite low).

liamsi commented 2 years ago

There is more: the JSON returned from a hub-1 block (from https://api.cosmostation.io/v1/account/new_txs/{wallet_address}) differs in the sense that logs do not contain an events field. That means that this and similar handle logic won't work: https://github.com/hodgerpodger/staketaxcsv/blob/ecede9d17d352b5255f16ccffdda5e5b9ecdb4e9/src/atom/cosmoshub3/processor.py#L91

hodgerpodger commented 2 years ago

Nice limit find bug. Either bug always existed or they changed limit parameter allowed.

hodgerpodger commented 2 years ago

https://github.com/hodgerpodger/staketaxcsv/commit/579d4ff01212de75fc1e5ab3ee654cba52cb9554

TODO: handle cosmoshub1 tx formats (maybe includes older cosmoshub2 txs)

hodgerpodger commented 2 years ago

Alrighty added support for cosmoshub2 transfers/withdraw rewards in the processor.

https://github.com/hodgerpodger/staketaxcsv/commit/032f457d4dceb5f6da6ad6dbeb1484d2326f58fb

Claim reward amounts in delegate/redelegate not supported (treated as _UNKNOWN) because I'm unable to locate that data in the older data format.

Closing issue but re-open or make new one if there's any improvements that might be possible.

[Update] Fixes around addition of cosmoshub1/cosmoshub2: https://github.com/hodgerpodger/staketaxcsv/commit/862a014a1d546ec9cace4ab932999e9c67a01d80

liamsi commented 2 years ago

Cool! Awesome, thanks! I'll check it out during the weekend 👍🏼