search

algorandfoundation / algokit-lora

10 stars 5 forks source link

Transaction visualiser amount rounding #238 #244

Closed satishccy closed 2 weeks ago

satishccy commented 2 weeks ago

I have changed the line in https://github.com/algorandfoundation/algokit-lora/blob/main/src/utils/compact-amount.ts#L4 from 8 to 9 for echancement #238 .

First i have tested out the scenario by making a transaction http://localhost:1420/testnet/transaction/HZXSLRMSVSHXYH3VILN62EFLZD6Y3XNOQPDTJ2QGJV73IJLI3U4Q as which was discussed in discord https://discord.com/channels/491256308461207573/1277185471214256138 and got below visual image which looks good & all 6 digits are visible.

Previous Snapshot - transactions-visual (8) Updated Snapshot - transaction-HZXSLRMSVSHXYH3VILN62EFLZD6Y3XNOQPDTJ2QGJV73IJLI3U4Q

I have ran the tests & got 3 errors in the following transactions due to snapshot mismatch

  1. http://localhost:1420/mainnet/transaction/WYEGSIGWZHTR6VYXC3EXFGZQHYKI6FQOZU2DOKHQCAWYEIHJBKEA -

Previous Snapshot - transactions-visual (5) Updated Snapshot - transaction-WYEGSIGWZHTR6VYXC3EXFGZQHYKI6FQOZU2DOKHQCAWYEIHJBKEA

  1. http://localhost:1420/mainnet/transaction/INDQXWQXHF22SO45EZY7V6FFNI6WUD5FHRVDV6NCU6HD424BJGGA -

Previous Snapshot - transactions-visual (6) Updated Snapshot - transaction-INDQXWQXHF22SO45EZY7V6FFNI6WUD5FHRVDV6NCU6HD424BJGGA

  1. http://localhost:1420/mainnet/block/36591812/group/%2FoRSr2uMFemQhwQliJO18b64Nl1QIkjA39ZszRCeSCI%3D -

Previous Snapshot - transactions-visual (7) Updated Snapshot - round-36591812-group-_oRSr2uMFemQhwQliJO18b64Nl1QIkjA39ZszRCeSCI=

I have reviewed the above 3 updated snapshots and they are visually looking good & showing upto 6 decimals. So i have updated the snapshots.

But even after updating the code, we are unable to show 6 decimals if the whole number is greater than 9, as you can see the in below scenario i have made payment transaction of amount 12.000115 algos and visual image is not showing 6 decimals. http://localhost:1420/testnet/transaction/ZZGHI5HFGRXCSNGY5RMPZHIAJF5G5E5NJXLQ7T6OAIIU6XBKOREQ

image

neilcampbell commented 2 weeks ago

@satishccy This is awesome. Thanks for your work on this. We could definitely write something to be even smarter, however I don't think it's necessary. My justification for choosing 8 characters, is that it'll prevent rounding for ALGO values less than 1 ALGO (or even 9 ALGO) and I think that is a nice balance.

satishccy commented 2 weeks ago

@neilcampbell Thanks for guiding me on my first contribution. Is there anything else I can work on? I'm really learning a lot by working on this repo. Previously, I was developing beginner and intermediate smart contracts with PyTeal and TealScript, implementing ARCs, and building APIs to interact with smart contracts. But now, I’m getting to see how an application in production is developed and maintained.

neilcampbell commented 1 week ago

@satishccy Sorry I've been a bit slow to respond. That's awesome to hear!

Any of the issues labelled as an enhancement should be good to work on. https://github.com/algorandfoundation/algokit-lora/issues?q=is%3Aissue+is%3Aopen+label%3Aenhancement

https://github.com/algorandfoundation/algokit-lora/issues/186 or https://github.com/algorandfoundation/algokit-lora/issues/173 would be quite interesting. Happy to chat through any of these in more detail.