EmerisHQ / demeris

Emeris web app
https://app.emeris.com/
Apache License 2.0
12 stars 2 forks source link

fix: potential fix for unverified ibc denom overflow #1734

Closed fl-y closed 2 years ago

fl-y commented 2 years ago

Description

Fixes: #1726

Feature flags

N/A

Testing

Difficult to test, The current applied 'fix' was attempted as an attempt to not fix the issue by touching up the css but to display the 'unverifed' ibc denom in a shorter way, currently applied fixed truncates the majority of the hash and only displays the first 3 and last letters, which should be enough to differentiate with other ones

A way to test this out would be to change https://github.com/EmerisHQ/demeris/blob/3ad46253591878cc24114db387369a6d194db736/src/utils/actionHandler.ts#L390 to

if(false)

adding a throw new Error('force throw') above

https://github.com/EmerisHQ/demeris/blob/3ad46253591878cc24114db387369a6d194db736/src/utils/actionHandler.ts#L400

and reassigning name to some random ibc denom above here

https://github.com/EmerisHQ/demeris/blob/3ad46253591878cc24114db387369a6d194db736/src/utils/actionHandler.ts#L408

Since this is the only place where '(unverified)' is found in the entire repository, this will prevent overflows from happening in the future with relative certainty

caudurodev commented 2 years ago

Cannot reproduce original issue BTW - is it still applicable with new Swap aggregator widget?

fl-y commented 2 years ago

Cannot reproduce original issue BTW - is it still applicable with new Swap aggregator widget?

I wasn't able to reproduce it either - but I think this fix will prevent ugly UI issues where the ibc denom's 64 letter hash overflows and ruins your day.

caudurodev commented 2 years ago

Could be because of current state of swap, but all transactions fail for one reason or another: image

so cannot verify this fix works. Unless there is step-by-step to reproduce consistently?

github-actions[bot] commented 2 years ago

Visit the preview URL for this PR (updated for commit 8cf23a4):

https://emeris-app--pr1734-fix-1726-long-string-mris8s5o.web.app

(expires Tue, 10 May 2022 10:28:29 GMT)

πŸ”₯ via Firebase Hosting GitHub Action 🌎

fl-y commented 2 years ago

Could be because of current state of swap, but all transactions fail for one reason or another: image

so cannot verify this fix works. Unless there is step-by-step to reproduce consistently?

Well I could only reproduce by forcing it as can be seen in the testing section of this PR as well I'm afraid 😒. I'll ask the original issue-author and see if they can reproduce

Edit: I see you already have thanks @caudurodev πŸ™