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

safe-num support #1025

Closed
3 of 5 tasks
lemunozm opened this issue Oct 10, 2022 · 3 comments
Closed
3 of 5 tasks

safe-num support #1025

lemunozm opened this issue Oct 10, 2022 · 3 comments
Labels
D2-notify Pull request can be merged and notification about changes should be documented. I6-refactoring Code needs refactoring. P0-someday-maybe Issue might be worth doing someday. Q1-easy Can be done by primarily duplicating and adapting code.

Comments

@lemunozm
Copy link
Contributor

Which part of the code is the issue addressing?

  • Feature

Description

I open this issue to track the changes of safe-num repo with the intention of using into centrifuge-chain.

As an overview, the safe-num aim is to allow safe arithmetic operation checks without losing readability which means, to be as close as possible to the "normal" arithmetic style while being sure all operations are checked.

How will this affect the code base

  • Better readability.
  • If incorporated into centrifuge-chain, most arithmetic calculations in the pallets will use it.
    The adoption could be gradual

What are forseen obstacles or hurdles to overcome?

How to allow FixedPointNumber to use it with FixedPointOperand?

The first idea of safe-num crate was to be agnostic from the framework used. But in order to enable FixedPointNumber support, we need to implement Substrate traits for SafeNum, which can only be done in the safe-num crate because of the orphan rule.

Is a substrate feature that enables these traits implementations ok?

Pending tasks

  • Saturating ops implementation for SafeNum
  • Assign ops implementation for SafeNum
  • Ops over generic operators for SafeNum
  • Accurate Overflow/Underflow for multiplication with sign (not crucial)
  • FixedPointNumber support.
@lemunozm lemunozm added Q1-easy Can be done by primarily duplicating and adapting code. D2-notify Pull request can be merged and notification about changes should be documented. P0-someday-maybe Issue might be worth doing someday. I6-refactoring Code needs refactoring. labels Oct 10, 2022
@LuckyPigeon
Copy link

@lemunozm
Can I give this one a shot?

@lemunozm
Copy link
Contributor Author

Hi @LuckyPigeon, this crate is not used (at least by this moment) in our repo. It's quite experimental, but of course, you can give it a try and test if it works for you. What we are currently using (as a less aggressive solution) is our ensure module (docs here) to manage the arithmetic issues.

Currently there is an open issue to merge this in the Substrate repo: paritytech/substrate#12754

@lemunozm
Copy link
Contributor Author

Closed in favor of substrate method's ensure_ops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D2-notify Pull request can be merged and notification about changes should be documented. I6-refactoring Code needs refactoring. P0-someday-maybe Issue might be worth doing someday. Q1-easy Can be done by primarily duplicating and adapting code.
Projects
None yet
Development

No branches or pull requests

2 participants