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

Loans: Quantity as fixed point with 18 decimals #1471

Merged
merged 5 commits into from Aug 2, 2023

Conversation

lemunozm
Copy link
Contributor

@lemunozm lemunozm commented Jul 28, 2023

Description

The current Rate type used from cfg_types uses 27 decimals. Using an underlying 128bits number means that we only ca reach values up to 10^9, which can not be enough for quantity purposes. Rate should only be used for interest rate computations. Other rates should use 18 decimals.

This PR fixes this, using a rate with 18 decimals.

@lemunozm lemunozm added Q1-easy Can be done by primarily duplicating and adapting code. I3-annoyance The code behaves as expected, but "expected" is an issue. P7-asap Issue should be addressed in the next days. D2-notify Pull request can be merged and notification about changes should be documented. crcl-protocol Circle protocol related. labels Jul 28, 2023
@lemunozm lemunozm self-assigned this Jul 28, 2023
@lemunozm lemunozm enabled auto-merge (squash) July 28, 2023 11:23
@lemunozm lemunozm requested a review from wischli July 28, 2023 15:28
@lemunozm lemunozm added the D8-migration Pull request touches storage and needs migration code. label Aug 2, 2023
thea-leake
thea-leake previously approved these changes Aug 2, 2023
Copy link
Contributor

@thea-leake thea-leake left a comment

Choose a reason for hiding this comment

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

LGTM!

@lemunozm
Copy link
Contributor Author

lemunozm commented Aug 2, 2023

Thanks @thea-leake! Could you give the approval again? I've rebased it to main.

@lemunozm lemunozm merged commit 35c9b70 into main Aug 2, 2023
11 checks passed
@lemunozm lemunozm deleted the loans/quantity-18-decimals branch August 3, 2023 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crcl-protocol Circle protocol related. D2-notify Pull request can be merged and notification about changes should be documented. D8-migration Pull request touches storage and needs migration code. I3-annoyance The code behaves as expected, but "expected" is an issue. P7-asap Issue should be addressed in the next days. Q1-easy Can be done by primarily duplicating and adapting code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants