Migrate from ttmath to stint #24
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yglukhov
reviewed
May 7, 2018
| @@ -5,33 +5,36 @@ | |||
| # * MIT license ([LICENSE-MIT](LICENSE-MIT) or https://opensource.org/licenses/MIT) | |||
| # at your option. This file may not be copied, modified, or distributed except according to those terms. | |||
|
|
|||
| import ttmath, constants, strformat, strutils, sequtils, endians, macros, utils / padding, rlp | |||
| import stint, constants, strformat, strutils, sequtils, endians, macros, utils / padding, rlp | |||
|
|
|||
| # some methods based on py-evm utils/numeric | |||
|
|
|||
| # TODO improve | |||
|
|
|||
| proc intToBigEndian*(value: UInt256): Bytes = | |||
There was a problem hiding this comment.
These procs seem to know too much about stint internal structure. Can we just move them to stint lib?
There was a problem hiding this comment.
Yes I will move them this week, for now I just focused on porting 1-to-1 and just exposing Stint internal but tested functionality (like countLeadingZeroBits) and avoid introducing new procs/tests/reviews in Stint.
| @@ -21,7 +21,7 @@ type | |||
|
|
|||
| # TODO: The Int256 value is stored as array[32, byte], and we bitcast it | |||
| # back and forth. This is a hacky workaround for the problem that clang | |||
| # doesn't let you store ttmath types inside nim variant types (unions). Things | |||
| # doesn't let you store stint types inside nim variant types (unions). Things | |||
There was a problem hiding this comment.
I suppose this is no longer the case, so we can just revert this hack completely.
There was a problem hiding this comment.
Good catch, it seems like only vint is used externally here: https://github.com/status-im/nimbus/blob/master/src/runner.nim#L86
Will remove that hack as well.
coffeepots
approved these changes
May 7, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.