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

Change effect of assert to invalid opcode. #1702

Merged
merged 2 commits into from
Mar 3, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions docs/control-structures.rst
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,13 @@ Currently, Solidity automatically generates a runtime exception in the following

While a user-provided exception is generated in the following situations:
#. Calling ``throw``.
#. The condition of ``assert(condition)`` is not met.

Internally, Solidity performs a revert operation (instruction ``0xfd``) when a user-provided exception is thrown. In contrast, it performs an invalid operation
(instruction ``0xfe``) if a runtime exception is encountered. In both cases, this causes
(instruction ``0xfe``) if a runtime exception is encountered or the condition of an ``assert`` call is not met. In both cases, this causes
the EVM to revert all changes made to the state. The reason for this is that there is no safe way to continue execution, because an expected effect
did not occur. Because we want to retain the atomicity of transactions, the safest thing to do is to revert all changes and make the whole transaction
(or at least call) without effect.
(or at least call) without effect.

If contracts are written so that ``assert`` is only used to test internal conditions and ``throw`` or
Copy link
Member

Choose a reason for hiding this comment

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

The phrase "internal conditions" might not make sense to everyone. What about "If contracts are written so that assert should never fail at runtime."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to convey when to use assert and when to use throw. How would you write that?

Copy link
Member

Choose a reason for hiding this comment

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

An assert should be a sanity check that is expected to pass always. An assert failure should detect coding mistakes (e.g. when the owner of something has changed without the owners' permission).

throw should catch everything else that goes wrong at runtime (e.g. when the caller supplies wrong data; it's not time yet to call this function).

Copy link
Contributor

Choose a reason for hiding this comment

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

@chriseth @pirapira Maybe it would be helpful to explain who is responsible for ensuring that there are no such exceptions: "assert is used to express safety conditions that the contract should ensure. Consequently, the author of the contract is responsible for avoiding any assertion violations. In contrast, throw can be used to express that some input is not considered valid. It is the responsibilty of the client/caller, as opposed to the contract's author, to ensure that this does not happen. Formal analysis tools can be used to verify that no execution of a contract leads to an assertion violation assuming the client/caller provided valid inputs.".

Does that capture what you want to express?

Copy link
Member

Choose a reason for hiding this comment

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

@wuestholz yes, that sounds accurate. Thank you.

``revert`` is used in case of malformed input, a formal analysis tool that verifies that the invalid
opcode can never be reached can be used to check for the absence of errors assuming valid inputs.
2 changes: 1 addition & 1 deletion docs/miscellaneous.rst
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ Global Variables
- ``ecrecover(bytes32 hash, uint8 v, bytes32 r, bytes32 s) returns (address)``: recover address associated with the public key from elliptic curve signature, return zero on error
- ``addmod(uint x, uint y, uint k) returns (uint)``: compute ``(x + y) % k`` where the addition is performed with arbitrary precision and does not wrap around at ``2**256``
- ``mulmod(uint x, uint y, uint k) returns (uint)``: compute ``(x * y) % k`` where the multiplication is performed with arbitrary precision and does not wrap around at ``2**256``
- ``assert(bool condition)``: throws if the condition is false
- ``assert(bool condition)``: throws if the condition is false (using an invalid opcode)
- ``this`` (current contract's type): the current contract, explicitly convertible to ``address``
- ``super``: the contract one level higher in the inheritance hierarchy
- ``selfdestruct(address recipient)``: destroy the current contract, sending its funds to the given address
Expand Down
5 changes: 2 additions & 3 deletions libsolidity/codegen/ExpressionCompiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -875,9 +875,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
// jump if condition was met
m_context << Instruction::ISZERO << Instruction::ISZERO;
auto success = m_context.appendConditionalJump();
// condition was not met, abort
m_context << u256(0) << u256(0);
m_context << Instruction::REVERT;
// condition was not met, flag an error
m_context << Instruction::INVALID;
// the success branch
m_context << success;
break;
Expand Down