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

Balance conversion for balance types #376

Merged
merged 7 commits into from
Sep 29, 2023
Merged

Balance conversion for balance types #376

merged 7 commits into from
Sep 29, 2023

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Sep 27, 2023

closes #375

This adds the token at the end of the field for manual extrinsics in case the field is expecting a balance. It will then convert the value in plank with the right amount of 0s.

To test:

  • Craft a manual extrinsic that needs a value, for instance balances.transfer or bounties.proposeBounty
  • check that the token is visible
  • check that when the tx is done, the right amount in plank is submitted. Note that for balances.transfer, the overview converts it back to human readable values, like in my screenshot

Submitting
image

Verifying
image

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.

If it's possible please refactor the method transformParams. Both scenarios mentioned in the description work fine 👍

@Tbaut
Copy link
Collaborator Author

Tbaut commented Sep 28, 2023

The refactor is in, very inspired by your code, thank you very much. I chose to only extract to functions some logic when

  • it was more than 1 line
  • it was re-used
  • it did not require to duplicate code

This means that things like for instance the Balance handling is still on several lines, because I didn't want to duplicate the bnResult = inputToBn(chainInfo.tokenDecimals, converted);, but it should be much easier to read now still, on top of the simplification of not using a convert batching the map etc..

@Tbaut Tbaut requested a review from Lykhoyda September 28, 2023 16:50
Comment on lines +230 to +234
// console.log('getTypeDef', getTypeDef(arg.type.toString()))
// const instance = api.registry.createType(arg.type as unknown as 'u32')
// console.log('instance', instance)
// const raw = getTypeDef(instance.toRawType())
// console.log('raw', raw)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we maybe move it to the log method and comment it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After offline discussion we're keeping this around for a bit.

@Tbaut Tbaut merged commit 8711fb0 into main Sep 29, 2023
@Tbaut Tbaut deleted the tbaut-balance-conversion branch September 29, 2023 10:28
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.

Make sure all the Balanceof<> are denomitated in Token
2 participants