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

PoS - use checked arithmetics #17

Closed
Tracked by #2006
tzemanovic opened this issue Sep 7, 2021 · 8 comments · Fixed by #2178
Closed
Tracked by #2006

PoS - use checked arithmetics #17

tzemanovic opened this issue Sep 7, 2021 · 8 comments · Fixed by #2178
Assignees
Milestone

Comments

@tzemanovic
Copy link
Member

tzemanovic commented Sep 7, 2021

Use checked arithmetics in the PoS library code (has a TODO left in code)

┆Issue is synchronized with this Asana task by Unito

@cwgoes
Copy link
Contributor

cwgoes commented Jan 12, 2023

@brentstone do you know what the status of this is? we should definitely do it 😄

@tzemanovic
Copy link
Member Author

@brentstone do you know what the status of this is? we should definitely do it 😄

still open I think

@cwgoes cwgoes added this to the 0.15 milestone Jan 16, 2023
@sug0
Copy link
Contributor

sug0 commented Jul 18, 2023

given everything is using the updated amounts now, since v0.18.0, which .unwrap()s overflows/underflows, I think we can close this, right?

@brentstone
Copy link
Collaborator

given everything is using the updated amounts now, since v0.18.0, which .unwrap()s overflows/underflows, I think we can close this, right?

I think it's prob still good to keep open as a reminder to go through and ensure this is universally true at some point. I'd bet there are still some PoS things which could use this.

@cwgoes
Copy link
Contributor

cwgoes commented Aug 17, 2023

Would it be possible to configure our Rust linter to warn/error when unchecked arithmetic is used?

@tzemanovic
Copy link
Member Author

Would it be possible to configure our Rust linter to warn/error when unchecked arithmetic is used?

I don’t see anything out of box, but it should be possible to add this to clippy

@tzemanovic
Copy link
Member Author

ah, there is a way - we can configure clippy "disallowed-methods" https://doc.rust-lang.org/nightly/clippy/lint_configuration.html#disallowed-methods

@brentstone
Copy link
Collaborator

It seems like the original instances that this issue referenced have already been fixed, but nevertheless I've added some more checked arithmetics in the PoS code in #2178.

@github-project-automation github-project-automation bot moved this from Todo to Tested in Devnet in Namada-Old Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Tested in Devnet
Development

Successfully merging a pull request may close this issue.

5 participants