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

✨ Accept only CT decimals from 4 to 20 #279

Merged
merged 1 commit into from
May 2, 2024

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented May 1, 2024

What?

  • The decimals in the token information can now only be a number from 4 to 20 inclusive.

Why?

  • Too low numbers means a very big whole part in the Fixedu128, too high means a very big decimal part in the same type.
  • We want to make sure the price is representable

How?

  • add a new check on the metadata validation function

Testing?

unaccepted_ct_decimals on the application test file

Anything Else?

  • We need to also check that the allocation size is big enough to represent the decimals specified by the user. In another PR though

Copy link
Contributor Author

JuaniRios commented May 1, 2024

@JuaniRios JuaniRios changed the title accept only ct decimals from 4 to 20 ✨ Accept only CT decimals from 4 to 20 May 1, 2024
@JuaniRios JuaniRios marked this pull request as ready for review May 1, 2024 13:12
@JuaniRios JuaniRios requested review from lrazovic and vstam1 May 1, 2024 13:13
Copy link

graphite-app bot commented May 1, 2024

Graphite Automations

"Auto-assign PRs to author" took an action on this PR • (05/01/24)

1 assignee was added to this PR based on Juan Ignacio Rios's automation.

@JuaniRios JuaniRios force-pushed the 05-01-new_decimals_remainder_test branch from e1ba451 to 5c073a2 Compare May 2, 2024 07:53
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from b0b4644 to 4990206 Compare May 2, 2024 07:53
@JuaniRios JuaniRios force-pushed the 05-01-new_decimals_remainder_test branch from 5c073a2 to b16bdbd Compare May 2, 2024 07:57
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 4990206 to 8401d91 Compare May 2, 2024 07:57
@JuaniRios JuaniRios force-pushed the 05-01-new_decimals_remainder_test branch from b16bdbd to 118cf5a Compare May 2, 2024 07:57
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 8401d91 to 13c0fec Compare May 2, 2024 07:58
@JuaniRios JuaniRios force-pushed the 05-01-new_decimals_remainder_test branch from 118cf5a to b63a95f Compare May 2, 2024 07:59
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 13c0fec to 8addb0d Compare May 2, 2024 07:59
@JuaniRios JuaniRios force-pushed the 05-01-new_decimals_remainder_test branch from b63a95f to f69f6f3 Compare May 2, 2024 08:01
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch 2 times, most recently from 7f7e83c to 97e99f9 Compare May 2, 2024 08:13
@JuaniRios JuaniRios requested a review from vstam1 May 2, 2024 08:15
Copy link
Contributor Author

JuaniRios commented May 2, 2024

Merge activity

  • May 2, 5:12 AM EDT: @JuaniRios started a stack merge that includes this pull request via Graphite.
  • May 2, 5:19 AM EDT: Graphite rebased this pull request as part of a merge.
  • May 2, 5:20 AM EDT: @JuaniRios merged this pull request with Graphite.

@JuaniRios JuaniRios force-pushed the 05-01-new_decimals_remainder_test branch from f69f6f3 to 7db8804 Compare May 2, 2024 09:16
Base automatically changed from 05-01-new_decimals_remainder_test to main May 2, 2024 09:18
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 97e99f9 to edd2894 Compare May 2, 2024 09:18
@JuaniRios JuaniRios merged commit e476a7f into main May 2, 2024
@JuaniRios JuaniRios deleted the 05-01-accept_only_ct_decimals_from_4_to_20 branch May 2, 2024 09:20
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.

2 participants