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

uint: add casting from/to U64 #691

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

halo3mic
Copy link
Contributor

@halo3mic halo3mic commented Nov 7, 2022

Add uint casting support for U64 in primitive-types

  • implement From trait for casting uint from U64
  • implement TryFrom trait for casting uint to U64
  • implement full_mul for U64
  • add tests for added functionality

Fixes #568

@halo3mic
Copy link
Contributor Author

halo3mic commented Nov 7, 2022

After opening PR I realized the tests might not be sufficient, so I will add more. Will appreciate feedback if this is the right direction.

construct_uint! {
/// 64-bit unsigned integer.
#[cfg_attr(feature = "scale-info", derive(TypeInfo))]
pub struct U64(1);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not keen on adding this type to primitives.
In fact, we wanted to remove it altogether, see #473 for the context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad for not reading this issue thoroughly, and thanks for pointing it out.

I understand now why you suggested adding this to ethereum-types a year ago in #473. Just for clarity, you are still okay with adding this functionality in general, right?

My motivation behind this is that there are still libraries that use U64 in certain cases (eg. ethers-rs block-num) and conversion to other uint types is currently not the most elegant as already pointed out in the referenced issue.

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.

U types do not convert to each other
2 participants