Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Very basic EVM binary. #1574

Merged
merged 5 commits into from Jul 11, 2016
Merged

Very basic EVM binary. #1574

merged 5 commits into from Jul 11, 2016

Conversation

tomusdrw
Copy link
Collaborator

No description provided.

@tomusdrw tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Jul 10, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 10, 2016
@@ -95,11 +95,17 @@ impl<'a> Finalize for Result<GasLeft<'a>> {
}
}

/// Cost calculation type. For low-gas usage we calculate costs using usize instead of U256
Copy link
Contributor

Choose a reason for hiding this comment

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

for 32-bit usize, this may lead to a consensus issue - u64 should be a lot safer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why? If it doesn't fit in usize we fallback to U256 - it optimizes for u32 on 32-bit systems and for u64 for 64-bit ones.
Using optimized (whether it's u64 or u32) and non-optimized (u256) version should is not affecting consensus.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh - fair enough.

@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. A8-looksgood 🦄 Pull request is reviewed well. and removed A8-looksgood 🦄 Pull request is reviewed well. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Jul 10, 2016
@gavofyork gavofyork merged commit 2ed09de into master Jul 11, 2016
@gavofyork gavofyork deleted the evmbin branch July 11, 2016 07:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants