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

Generically show token value without decimals #384

Merged
merged 7 commits into from
Oct 10, 2023

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Oct 4, 2023

closes #380
addresses a bit #134

You can test with either a done transaction or by creating a new TX from callData, and looking at the decoded call, since the component is the same.

e.g with callData: 0x23000b00b04e2bde6f24736f6d652064657363

image

There's also a huge improvement for batch, e.g callData 0x18000800002c736f6d652072656d61726b12000b00b04e2bde6f00f2c116d8c1f412f2dfc91702660acba3c4e9d487de4f7ac7577cb117264dd9aa

image

it used to be displayed like this
image

@Tbaut Tbaut marked this pull request as ready for review October 6, 2023 10:24
@Tbaut Tbaut requested review from Lykhoyda and asnaith October 6, 2023 10:24
@asnaith
Copy link
Member

asnaith commented Oct 6, 2023

@Tbaut This is a really nice improvement, makes a lot more sense with the token value, and the display is much improved too.

However, one issue I noticed is that "ROC" is being appended in a place it shouldn't be:

CleanShot 2023-10-06 at 10 06 47@2x

@Tbaut
Copy link
Collaborator Author

Tbaut commented Oct 6, 2023

You must have been testing an older commit could it be? The latest refractor fixes this bug that I found earlier.

Copy link
Member

@asnaith asnaith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You must have been testing an older commit could it be? The latest refractor fixes this bug that I found earlier.

I double-checked to make sure and then see what the confusion was.

I had clicked the most recent "View Deployment" button and I was testing on there and still seeing the issue (on https://6ddde8e1.multix.pages.dev/?network=rococo).

However, when I pulled the latest changes and tested on localhost I saw that it was fixed. Seems like there wasn't a new staging deployment for the very latest commits? Not sure why.

This PR looks good to go though, no further concerns :)

Copy link
Collaborator

@Lykhoyda Lykhoyda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a method for verifying the args data

@Tbaut Tbaut merged commit 753cfe7 into main Oct 10, 2023
6 checks passed
@Tbaut Tbaut deleted the tbaut-values-callinfo-generic branch October 10, 2023 13:51
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.

Show the value in token with the right decimals, rather than in plank
3 participants