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

Arithmetic overflow detection for the EVM #1051

Merged
merged 4 commits into from
May 2, 2018

Conversation

Arachnid
Copy link
Contributor

@Arachnid Arachnid commented May 2, 2018

This EIP proposes a pair of flags and opcodes to efficiently detect arithmetic overflow, reducing gas costs for contracts that need to prevent arithmetic overflow.

@Arachnid Arachnid merged commit f160e2f into ethereum:master May 2, 2018
@Arachnid Arachnid deleted the overflow branch May 2, 2018 18:10
@axic
Copy link
Member

axic commented May 3, 2018

I think this is practically a duplicate of #159 ?

@Arachnid
Copy link
Contributor Author

Arachnid commented May 3, 2018

@axic not really; that eip sets the flag on many other conditions, and clears it before each op; it also doesn't support signed overflow.

@axic
Copy link
Member

axic commented May 3, 2018

I do feel that discussion of this should have happened there prior to merging this.

With this new EIP process I have seen that a lot of prior work is just thrown away entirely, which I do not think is fair for all those people contributing their time and thoughts.

@Arachnid
Copy link
Contributor Author

Arachnid commented May 4, 2018

@axic Merging it as a draft just means it's a draft; discussion and editing is still ongoing. There's a discussion URL in the header.

We're trying to move away from a situation where PRs and issues remain open indefinitely, with no stable URL for referencing a draft, and instead move to aggressively merging drafts and keeping discussion moving through multiple revisions. That way, a draft has a stable URL immediately, and a persistent place to discuss it. Merging doesn't mean approval, or that it will be confirmed as a standard.

@Arachnid
Copy link
Contributor Author

Arachnid commented May 4, 2018

@axic I'd appreciate your feedback at https://ethereum-magicians.org/t/eip-arithmetic-overflow-detection-for-the-evm/261 on the proposal. I certainly didn't mean to erase your earlier work; I wasn't aware of it, and it hasn't been touched since 2016.

@RenanSouza2
Copy link
Contributor

This may be too late, but I have some toughth on thi EIP wich is one crucial to join security and effiency

I like how signed and unsigned are treated separetely
how you can perform a batch of operations and the flags only need to be checked at the end (with some exception if you mix signed and unsigned operations)

but I also have some remarks:

The ovf flag should trigger in the operations EXP and SHL

an signed sum of a positive and a negative number should not trigger the signed and should trigger the unsigned flag. The specification says "The sovf flag is set whenever the ovf flag is set, and additionally in the following circumstances"

Division by zero, and this include operations div, mod, sdiv, smod, addmodm, mulmod as well; should be treated in the flags

The signed division -(2**255) / -1 should trigger the sovf flag

The flags related to signed multiplication is wrong, it should trigger when the modulo of the ideal multiplication is bigger equal than 2**255

The signed flag should trigger with the SHL operation, this is trickier to define when

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.

3 participants