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

Prevent fail on u8 overflow #418

Merged
merged 3 commits into from
Feb 11, 2022
Merged

Prevent fail on u8 overflow #418

merged 3 commits into from
Feb 11, 2022

Conversation

oxade
Copy link
Contributor

@oxade oxade commented Feb 11, 2022

This fixes #417

Rust will panic if overflow occurs in debug mode. u8 has relatively low probability of being 255 after a test run, hence the difficulty reproduce.
This fix subtracts instead of adds if at u8::MAX. The goal here is to create an offending object id, so it doesn't matter if add or subtract

@lxfind
Copy link
Contributor

lxfind commented Feb 11, 2022

Is there something special about the object ID right after the gas object ID? Trying to understand what it is trying to test.

@oxade
Copy link
Contributor Author

oxade commented Feb 11, 2022

Is there something special about the object ID right after the gas object ID? Trying to understand what it is trying to test.

I think it's just checking that the logic fails as expected for unknown objects

@lxfind
Copy link
Contributor

lxfind commented Feb 11, 2022

Is there something special about the object ID right after the gas object ID? Trying to understand what it is trying to test.

I think it's just checking that the logic fails as expected for unknown objects

In that case, you could do something simple like u8::MAX - x[last_index]

@oxade
Copy link
Contributor Author

oxade commented Feb 11, 2022

Is there something special about the object ID right after the gas object ID? Trying to understand what it is trying to test.

I think it's just checking that the logic fails as expected for unknown objects

In that case, you could do something simple like u8::MAX - x[last_index]

Indeed that concise. But 1AM brain did not see it lol

@oxade oxade merged commit a6a9957 into main Feb 11, 2022
@oxade oxade deleted the bugfix/overflow-auth-tests branch February 11, 2022 07:39
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.

authority_tests.rs failing sometimes
2 participants