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

Where possible, prevent Magic numbers #2916

Open
MoonLabsDev opened this issue Oct 7, 2023 · 0 comments
Open

Where possible, prevent Magic numbers #2916

MoonLabsDev opened this issue Oct 7, 2023 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@MoonLabsDev
Copy link

MoonLabsDev commented Oct 7, 2023

packages/vm/core/evm/evmimpl/iscmagic_state.go contains the magic number MAX_UINT256.

for token approval it is only possible to approve anything <= MAX_UINT64. There is 1 exception, which is that if the value is == MAX_UINT256, then ithe approval amount is clamped to MAX_UINT64. That means that it became a magic number and all values > MAX_UINT64 && values < MAX_UINT256 you throw an error. To remove the magic number and make it cleaner, just clamp everything above MAX_UINT64 to MAX_UINT64 as it all means approve as much as posible. This also makes the code easier:

OLD:

if !numTokens.IsUint64() {
    // Calling `approve(MAX_UINT256)` is semantically equivalent to an "infinite" allowance
    if numTokens.Cmp(gethmath.MaxBig256) == 0 {
        numTokens = big.NewInt(0).SetUint64(math.MaxUint64)
    } else {
        panic(errBaseTokensMustBeUint64)
    }
}
allowance.BaseTokens = numTokens.Uint64()

NEW:

if !numTokens.IsUint64() {
    // Calling `approve( > MAX_UINT64)` is semantically equivalent to an "infinite" allowance
    numTokens = big.NewInt(0).SetUint64(math.MaxUint64)
}
allowance.BaseTokens = numTokens.Uint64()

Also it prevents unexpected behaviour for smart contract developers.

@MoonLabsDev MoonLabsDev added the bug Something isn't working label Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants