crytic / crytic/slither

confused about position of modifiers in Function.nodes and Function.slithir_operations

Open
#751 0 comments 0 reactions 0 assignees View on GitHub
question
Dominant language
Python
Stars
6.4k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

A solidity modifer that adds `require` before the modified function will be the last element of the list `function.nodes` and `function.slithir_operations` of a `function` having that modifier.
I found this unintuitive.
In my case I wanted to print out the requires of a given function and was surprised that the require that gets executed first was listed last.
I imagine this can cause other problems as well.
Maybe there's a good reason for this that I'm missing.

Sidenote: `function.all_slither_operations()` uses `set(...)` to remove duplicates and in the process looses order entirely. This leads to surprising results as well.

## Example

Given `example.sol`:
```solidity
pragma solidity ^0.6.12;

contract Example {
modifier onlyOwner() {
require(msg.sender == 0x1494CA1F11D487c2bBe4543E90080AeBa4BA3C2b);
_;
}

function example() onlyOwner payable external {
require(msg.value == 12345);
}
}
```

then

```python
from slither import Slither

slither = Slither("example.sol")

for contract in slither.contracts:
for function in slither.functions:
print(f"\n# {function.name}")
print("\n## nodes\n")
for node in function.nodes:
print(node)

print("\n## slithir\n")
for ir in function.slithir_operations:
print(ir)

print("\n## all slithir\n")
for ir in function.all_slithir_operations():
print(ir)
```

outputs

```markdown
# example

## nodes

ENTRY_POINT
EXPRESSION require(bool)(msg.value == 12345)
EXPRESSION onlyOwner()

## slithir

TMP_0(bool) = msg.value == 12345
TMP_1(None) = SOLIDITY_CALL require(bool)(TMP_0)
MODIFIER_CALL, Example.onlyOwner()()

## all slithir

MODIFIER_CALL, Example.onlyOwner()()
TMP_4(None) = SOLIDITY_CALL require(bool)(TMP_3)
TMP_1(None) = SOLIDITY_CALL require(bool)(TMP_0)
TMP_0(bool) = msg.value == 12345
TMP_3(bool) = msg.sender == 117497932960071309244587853048350735941110479915
```

## Execution order

```
solc example.sol -o . --bin-runtime --overwrite
evmasm -d -i Example.bin-runtime
```

output excerpt of `JUMPDEST` for `0x54353f2f` which is the function id of `example()`:
```
00000021: JUMPDEST
00000022: PUSH1 0x27
00000024: PUSH1 0x29
00000026: JUMP
00000027: JUMPDEST
00000028: STOP
00000029: JUMPDEST
0000002a: PUSH20 0x1494ca1f11d487c2bbe4543e90080aeba4ba3c2b
0000003f: PUSH20 0xffffffffffffffffffffffffffffffffffffffff
00000054: AND
00000055: CALLER
00000056: PUSH20 0xffffffffffffffffffffffffffffffffffffffff
0000006b: AND
0000006c: EQ
```

the require in `onlyOwner` is executed first.

Contributor guide

Open the contributing guide

Research direction

Reproduce the behavior with the issue's example.sol and inspect Function.nodes, Function.slithir_operations, and all_slithir_operations(). Compare each collection with the Solidity execution order, including the effect of set(...). Done should establish whether these APIs are intended to preserve execution order and make their behavior consistent or explicit.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, solidity
Domain
blockchain, devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.