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

[x/programs] Utilize safe math for programs #591

Closed
exdx opened this issue Oct 30, 2023 · 5 comments
Closed

[x/programs] Utilize safe math for programs #591

exdx opened this issue Oct 30, 2023 · 5 comments
Labels
bug Something isn't working lifecycle/stale

Comments

@exdx
Copy link
Contributor

exdx commented Oct 30, 2023

Arithmetic operations in smart contracts require usage of a "safe math" library to prevent overflows and underflows.

There are well supported safe math libraries in Ethereum.

It should be clear that an overflow is an error that is handled gracefully. Both the NFT and token program are currently vulnerable to overflows.

@exdx exdx added the bug Something isn't working label Oct 30, 2023
@exdx exdx added this to the v0.1.3 (WASM Programs + Parallel Execution) milestone Oct 30, 2023
@richardpringle
Copy link
Contributor

I don't think we need a safe-math library. Rust has all the saturating_*/checked_* functions built-in and also has a much more powerful type system (NonZero* types, for example). It doesn't make sense just to wrap methods and types that already exist in std.

It might be more beneficial to gently guide hyper-program developers to fuzz and/or prop-testing their public methods, such that they realize which std types and methods they need to use to prevent overflows/underflows, etc.

@patrick-ogrady
Copy link
Contributor

I don't think we need a safe-math library. Rust has all the saturating_/checked_ functions built-in and also has a much more powerful type system (NonZero* types, for example). It doesn't make sense just to wrap methods and types that already exist in std.

IMO we should use Rust-native functionality wherever possible. We should optimize for as minimal of a cognitive leap as required to go from Rust to writing HyperSDK Programs. The nice byproduct of this is that it reduces the size of the codebase we need to maintain.

@exdx
Copy link
Contributor Author

exdx commented Nov 2, 2023

Apparently in in solidity 0.8.0+ any integer overflows or underflows automatically cause the transaction to revert. Maybe panicking on case of an overflow is the best solution?

@exdx
Copy link
Contributor Author

exdx commented Nov 2, 2023

My understanding is that in debug mode Rust will panic but in release it will actually perform the wrap. But there may be a compilation setting to change that behavior.

@patrick-ogrady patrick-ogrady removed this from the v0.1.3 (WASM Programs + Parallel Execution) milestone Nov 9, 2023
Copy link

github-actions bot commented Jan 9, 2024

This issue has become stale because it has been open 60 days with no activity. Adding the lifecycle/frozen label will exempt this issue from future lifecycle events.

@github-project-automation github-project-automation bot moved this from Backlog 🗄️ to Done ✅ in Platform Engineering Group Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lifecycle/stale
Projects
Archived in project
Development

No branches or pull requests

3 participants