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

Issue-1184: no distinction between uint and int #1187

Conversation

jondoe1337
Copy link
Contributor

During regression testing the org.ethereum.util.StandaloneBlockchainTest.encodeTest1() failed because the assumption on the expected value was wrong, imho:

The written SolidityContract has a setter-alike function and the value that is set is tested. However the value is an unsigned-int, so i doubt that the expected value -1 was correct due to the wrong uint implementation, but feel free to clear my mind.

Maximilian Schmidt added 2 commits September 25, 2018 20:28
- added abstract numeric type for shared method between int and uint
- distinction between uint and int in getType
- added two new subclasses for SoldityType
- added two testcases for int and uint SoldityType
- fixed StandaloneBlockchainTest#encodeTest1 as the expectation was
wrong
@coveralls
Copy link

coveralls commented Sep 26, 2018

Coverage Status

Coverage increased (+0.1%) to 56.628% when pulling 196e0a7 on eth-events:fix/1184-no-distinction-between-uint-and-int into 78b4b27 on ethereum:develop.

Copy link
Collaborator

@zilm13 zilm13 left a comment

Choose a reason for hiding this comment

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

Looks great!
Just small polishing needed.

- formatting
- throw exception if a biginteger below zero is encoded to uint
- made old regression test encode1 visible again
- reuse of Hex-encoding in test
Copy link
Collaborator

@zilm13 zilm13 left a comment

Choose a reason for hiding this comment

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

minor marks to go

- formatting
- license intro added to test
@zilm13
Copy link
Collaborator

zilm13 commented Sep 27, 2018

@jondoe1337 cool, thank you!
Will merge after test finished

@mkalinin mkalinin merged commit a03131f into ethereum:develop Sep 27, 2018
@mkalinin mkalinin added this to the 1.9.0-Constantinople milestone Oct 12, 2018
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.

4 participants