Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction visualiser amount rounding #238 #244

Conversation

satishccy
Copy link
Contributor

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
Copy link
Contributor

@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.

@neilcampbell neilcampbell merged commit 6f45235 into algorandfoundation:main Aug 30, 2024
1 check passed
@satishccy
Copy link
Contributor Author

@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.

@satishccy satishccy deleted the transaction-visualiser-amount-rounding branch August 30, 2024 16:32
@neilcampbell
Copy link
Contributor

@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

#186 or #173 would be quite interesting.
Happy to chat through any of these in more detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants