-
Notifications
You must be signed in to change notification settings - Fork 374
Include section on arithmetic checks #237
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
Conversation
|
@bohendo : can you do a first pass of review? |
learn_evm/arithmetic-checks.md
Outdated
| This is equivalent, because `type(uint256).max` is a 256-bit integer with all its bits set to `1`. | ||
| Subtracting `b` from `type(uint256).max` can be viewed as inverting each bit in `b`. | ||
| This can be demonstrated by the transformation `~b = ~(0 ^ b) = ~0 ^ b = MAX ^ b = MAX - b`. | ||
| > It's worth noting that `a - b = a ^ b` is **NOT** a general rule, except in special cases, such as when one of the values `MAX`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/values `MAX`/values is `MAX`/
learn_evm/arithmetic-checks.md
Outdated
| ``` | ||
|
|
||
| Nevertheless, by utilizing `xor`, which is the bitwise exclusive-or operation, we can combine these checks into one. | ||
| The code is written in assembly, because Solidity lacks an `xor` operation for boolean values. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't ^ the xor operator in solidity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm ^ is the bitwise xor, which might be different than "xor for boolean values"? If so, adding a sentence here to distinguish between bitwise and boolean xor would be illuminating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but it's not possible to write a ^ b where a or b are booleans.
Maybe re-phrasing might make it clearer?
"Nevertheless, using the boolean exclusive-or lets us combine these checks into one step. Solidity doesn't have a built-in operation for boolean values, but we can still make use of it through inline-assembly. In doing so, we need to take care that both inputs are actually boolean (either 0 or 1), as the xor operation works bitwise and isn't restricted to boolean values."
bohendo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an extremely informative page, great job! I left a few editorial/formatting nitpicks in this review + a couple stand-alone comments but besides those, this looks good to go
learn_evm/arithmetic-checks.md
Outdated
| } | ||
| ``` | ||
|
|
||
| When it comes to integer multiplication, it's important to handle the case hen `a < 0` and `b == type(int256).min`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/hen/when/
learn_evm/arithmetic-checks.md
Outdated
| = 0x????????????????????????????????????????????????0000000000000001 // int64(1) | ||
| ``` | ||
|
|
||
| > It is crucial to be mindful of when to clean the bits before and after operations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these block quotes refer text from external sources, it'd be informative to include links to them. Otherwise, we might want to adjust the formatting so it doesn't look like we're quoting another source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I didn't have any external sources for this. I removed the block quotes in this case. I generally just used them for extra trivia/further information.
|
Failing CI seems to be unrelated & due to slither itself. |
montyly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partial review
learn_evm/arithmetic-checks.md
Outdated
| @@ -0,0 +1,668 @@ | |||
| # A Guide on Performing Arithmetic Checks in the EVM | |||
|
|
|||
| The EVM is a peculiar machine that many of us have come to love and hate for all its quirks. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should re-word the introduction. secure-contract is meant to be objective, so we should avoid this type of description
learn_evm/arithmetic-checks.md
Outdated
| Although opcodes for signed integers (such as `sdiv`, `smod`, `slt`, `sgt`, etc.) exist, | ||
| arithmetic checks must be implemented within the constraints of the EVM. | ||
|
|
||
| > Note: [EIP-1051](https://eips.ethereum.org/EIPS/eip-1051)'s goal is to introduce the opcodes `ovf` and `sovf`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This EIP has not be discussed with 2021, I think we can remove the note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still find it interesting to include the note that there at least had been a discussion around including native operations. I added the remark that the status is stagnant here. Do you still want me to remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imho, it's interesting, but it breaks the flow of read. This is the beginning of the doc, and if you start by pointing out EIP that will (most likely) not be included, it makes more difficult to follow.
Maybe we can add a conclusion to this document, with
- A tldr
- Some references
- the wikipedia page to the two's complement system (or similar)
- this EIP
- anything else that is relevant
learn_evm/arithmetic-checks.md
Outdated
| > We also obtain the relation `~b + 1 = 0 - b = -b` if we add `1` mod `2**256` to both sides of the previous equation. | ||
|
|
||
| By computing the result of the addition first and then performing a check on the sum, | ||
| modern versions of Solidity can eliminate the need for performing extra arithmetic operations in the comparison. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- why
modern versions of Solidity? (0.8.16?) can eliminate the need for performing extra arithmetic operations in the comparisonI am not sure what we mean here. My assumption is that its cheaper to do the+and then the check, that to do a separate check, and then the+, but its not clear if its what we are trying to convey here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, 0.8.16 as seen in the code comment below. And that's pretty much what I mean. The code below doesn't need additional operations before the comparison. But I agree, it's a bit hard to understand, I'll try to re-phrase it.
learn_evm/arithmetic-checks.md
Outdated
| = 0x0000000000000000000000000000000000000000000000000000000000000000 | ||
| ``` | ||
|
|
||
| Newer versions of Solidity prevent integer overflow by using the computed result `c = a + b` to check for overflow/underflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the newer versions here? (0.8.16?)
Remind that this page will be still published when solidity 10 will be released ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by solidity 10? Solidity v0.10.0?
Would you always prefer to have the versions included in the text? I moved them to the code instead to de-clutter and to make it clearer where exactly they belong to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I meant 0.10.0. My point is that this text will still be here in 1 year, so it's better to avoid "new version"/"older version" etc, because it won't make sense later.
Adding the version in both the text and the code snippet is probably our best solution here
learn_evm/arithmetic-checks.md
Outdated
| ``` | ||
|
|
||
| Nevertheless, using the boolean exclusive-or lets us combine these checks into one step. | ||
| Solidity doesn't have a built-in operation for boolean values, but we can still make use of it through inline-assembly. In doing so, we need to take care that both inputs are actually boolean (either 0 or 1), as the xor operation works bitwise and isn't restricted to boolean values. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is confusing Solidity doesn't have a built-in operation for boolean values, as you can use && and || on booleans. Maybe solidity doesn't have built-in for boolean xor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good catch, this was a bit messed up due to my latest push.
learn_evm/arithmetic-checks.md
Outdated
| } | ||
| ``` | ||
|
|
||
| > It's important to note that the Solidity compiler always includes a division by zero check for all division and modulo operations, regardless of the presence of an unchecked block. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
division by zero and modulo by zero check maybe? saying "a division by zero" check on a modulo is a bit confusing ;) (even if its not false)
learn_evm/arithmetic-checks.md
Outdated
|
|
||
| ## Arithmetic checks for int256 multiplication | ||
|
|
||
| In older versions, the Solidity compiler uses four separate checks to detect integer multiplication overflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are the older versions? <0.8.17?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I included the exact versions in the code snippet, didn't want to clutter the text.
learn_evm/arithmetic-checks.md
Outdated
| } | ||
| ``` | ||
|
|
||
| Newer Solidity versions optimize the process by utilizing the computed product in the check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are newer versions? 0.8.17?
montyly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
review finished ;) great work overall
| We can simplify the expression to a single comparison if we're able to shift the disjointed number domain back so that it's connected. | ||
| To accomplish this, we subtract the smallest negative int64 `type(int64).min` from a value (or add the underlying unsigned value). | ||
| A better way to understand this is by visualizing the signed integer number domain in relation to the unsigned domain (which is demonstrated here using int128). | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not render well on github
learn_evm/arithmetic-checks.md
Outdated
| @@ -0,0 +1,717 @@ | |||
| # A Guide on Performing Arithmetic Checks in the EVM | |||
|
|
|||
| The Ethereum Virtual Machine (EVM) distinguishes itself from traditional computer systems and virtual machines through several unique aspects. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove "from traditional computer systems" - it sounds a bit weird
learn_evm/arithmetic-checks.md
Outdated
| One notable difference is its treatment of arithmetic checks. | ||
| While most architectures and virtual machines provide access to carry bits or an overflow flag, | ||
| these features are absent in the EVM. | ||
| Consequently, developers must manually incorporate these safeguards within the machine's constraints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
developers is a bit misleading here. In practice developers who use solidity don't need to care about it
learn_evm/arithmetic-checks.md
Outdated
|
|
||
| In conclusion, we hope this article has served as an informative guide on signed integer arithmetic within the EVM and the two's complement system. We have seen the added complexity that is introduced when performing checked arithmetic on signed integers compared to unsigned ones and when dealing with sub-32 byte types. | ||
|
|
||
| As the trend in Solidity smart contract development leans towards low-level optimizations, it is important to emphasize the diligence required when implementing these techniques. The aim of this article is to deepen one's understanding of low-level arithmetic, thereby improving the security of Solidity code by enabling developers to better assess and comprehend the assumptions present in these operations. However, it is crucial to remember that custom low-level optimizations should be integrated only after rigorous manual analysis, fuzzing, and symbolic verification. Additionally, any non-obvious assumptions should always be clearly documented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These points are important, but it feels like we could shorten this paragraph.
learn_evm/arithmetic-checks.md
Outdated
|
|
||
| ## Conclusion | ||
|
|
||
| In conclusion, we hope this article has served as an informative guide on signed integer arithmetic within the EVM and the two's complement system. We have seen the added complexity that is introduced when performing checked arithmetic on signed integers compared to unsigned ones and when dealing with sub-32 byte types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should use a bullet list here to highlight the lessons learned - I think it would make it more readable.
|
This isa big help for self taught |
This section is part of learn-evm. It includes a guide on how arithmetic checks can be performed in the EVM and how integers are handled in the EVM in general.