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

Add function + tests to convert decimals with token #61

Merged
merged 5 commits into from
Sep 28, 2023

Conversation

joYyHack
Copy link
Contributor

  • Since this PR suggests a bug fix, the tests have been added and the coverage is 100%.
  • Since this PR introduces a new feature, the update has been discussed in an Issue or with the team.
  • This PR is just a minor change, like a typo fix.

Added token decimals converter. The function accepts:

  1. The amount of the base token.
  2. The base token address.
  3. The destination token address.

Under the hood, the function casts the addresses of the tokens to the IERC20Metadata interface and retrieves the decimals for both tokens. Then, it converts the amount with the decimals of the base token to the decimals of the destination token.

All necessary tests are added. Coverage 100%.

@joYyHack joYyHack self-assigned this Sep 15, 2023
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (0b4265c) 100.00% compared to head (49627d3) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #61   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           35        35           
  Lines         1280      1301   +21     
  Branches       156       157    +1     
=========================================
+ Hits          1280      1301   +21     
Files Coverage Δ
contracts/libs/decimals/DecimalsConverter.sol 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Hrom131
Copy link
Member

Hrom131 commented Sep 24, 2023

Let's add functions to18 and from18, which will take the address of the token, instead of decimals

@Arvolear
Copy link
Member

Let's add functions to18 and from18, which will take the address of the token, instead of decimals

This is indeed what will be useful

@joYyHack joYyHack closed this Sep 25, 2023
@joYyHack
Copy link
Contributor Author

Let's add functions to18 and from18, which will take the address of the token, instead of decimals

This is indeed what will be useful

Okay, that's a good idea, but I need to warn you that it appears impossible to overload 'safe' functions because they all utilize the 'convert' function, which accepts a function as a parameter. So, for example, if we have both
'to18(uint256, uint256)' and 'to18(uint256, address)' along with 'convert(function(uint256, uint256)),' and we attempt to call 'function test { convert(to18) },' the compiler will report an error, stating 'No matching declaration found after variable lookup.' It seems that the compiler can't determine which of the overloaded functions we're trying to use.
image

Overall the idea is that I will just name it differently :D

@joYyHack joYyHack reopened this Sep 25, 2023
@Arvolear Arvolear force-pushed the feat/TokenDecimalsConverter branch from 7aa6cbe to 49627d3 Compare September 28, 2023 08:50
@Arvolear Arvolear merged commit 0505a3d into master Sep 28, 2023
@Arvolear Arvolear deleted the feat/TokenDecimalsConverter branch September 28, 2023 09:16
@gitpoap-bot
Copy link

gitpoap-bot bot commented Sep 28, 2023

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 Distributed Lab Solidity Library Contributor:

GitPOAP: 2023 Distributed Lab Solidity Library Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

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.

3 participants