Skip to content

Commit

Permalink
Testing and fixing CEI pattern analysis (follow up for FuelLabs#3168) (
Browse files Browse the repository at this point in the history
…FuelLabs#3356)

## Test cases

- [x] `if` with interaction in the condition and storage in a branch
- [x] `match` with interaction in the condition and storage in branches
- [x] Pair or struct with interaction in one component and storage in a
subsequent component


### `while`-loops tests

- [x] `while` loop with effects in the condition or the body
- [x] `while` loop with interaction-then-storage
- [x] `while` loop with storage-then-interaction (this is a violation
too because on the next loop iteration it will be
interaction-then-storage) -- this requires a fix implemented in this PR

### Function application tests

- [x] Pure function + argument is a code block with CEI violation
- [x] Pure function + first argument does interaction + second argument
does storage effect
- [x] Storage reading/writing function + argument does interaction
- [x] Improved error reporting precision for function application
- [x] Left-to-right arguments evaluation

Closes FuelLabs#3342
anton-trunov authored Nov 16, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 4fe5a38 commit 88b4cc5
Showing 54 changed files with 725 additions and 19 deletions.
47 changes: 28 additions & 19 deletions sway-core/src/semantic_analysis/cei_pattern_analysis.rs
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ use crate::{
};
use std::collections::HashSet;
use sway_error::warning::{CompileWarning, Warning};
use sway_types::{Ident, Span};
use sway_types::{Ident, Span, Spanned};

#[derive(PartialEq, Eq, Hash, Clone)]
enum Effect {
@@ -196,6 +196,7 @@ fn analyze_expression(
arguments,
function_decl_id,
selector,
call_path,
..
} => {
use crate::declaration_engine::de_get_function;
@@ -215,7 +216,14 @@ fn analyze_expression(

if args_effs.contains(&Effect::Interaction) {
// TODO: interaction span has to be more precise and point to an argument which performs interaction
warn_after_interaction(&fn_effs, &expr.span, &func.span, block_name, warnings)
let last_arg_span = &arguments.last().unwrap().1.span;
warn_after_interaction(
&fn_effs,
&call_path.span(),
last_arg_span,
block_name,
warnings,
)
}

let mut result_effs = set_union(fn_effs, args_effs);
@@ -273,27 +281,28 @@ fn analyze_expression(
set_union(cond_then_effs, cond_else_effs)
}
WhileLoop { condition, body } => {
// if the loop (condition + body) contains both interaction and storage operations
// in _any_ order, we report CEI pattern violation
let cond_effs = analyze_expression(condition, block_name, warnings);
let body_effs = analyze_code_block(body, block_name, warnings);
if cond_effs.contains(&Effect::Interaction) {
warn_after_interaction(
&body_effs,
&condition.span,
&expr.span,
block_name,
warnings,
);
let res_effs = set_union(cond_effs, body_effs);
if res_effs.is_superset(&HashSet::from([Effect::Interaction, Effect::StorageRead])) {
warnings.push(CompileWarning {
span: expr.span.clone(),
warning_content: Warning::StorageReadAfterInteraction {
block_name: block_name.clone(),
},
});
};
if body_effs.contains(&Effect::Interaction) {
warn_after_interaction(
&cond_effs,
&expr.span,
&condition.span,
block_name,
warnings,
);
if res_effs.is_superset(&HashSet::from([Effect::Interaction, Effect::StorageWrite])) {
warnings.push(CompileWarning {
span: expr.span.clone(),
warning_content: Warning::StorageWriteAfterInteraction {
block_name: block_name.clone(),
},
});
};
set_union(cond_effs, body_effs)
res_effs
}
AsmExpression {
registers, body, ..
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[[package]]
name = 'core'
source = 'path+from-root-88DBE1D69186ED7B'

[[package]]
name = 'left_to_right_arg_evaluation'
source = 'member'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
license = "Apache-2.0"
name = "left_to_right_arg_evaluation"
entry = "main.sw"

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"functions": [
{
"inputs": [],
"name": "main",
"output": {
"name": "",
"type": 0,
"typeArguments": null
}
}
],
"loggedTypes": [],
"types": [
{
"components": null,
"type": "bool",
"typeId": 0,
"typeParameters": null
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
script;
// This tests function, tuple, struct arguments are evaluated from left to right

fn func(a: u64, b: u64, c: u64, d: u64) -> u64 {
d
}

struct MyStruct {
a: u64,
b: u64,
c: u64,
d: u64,
}

fn main() -> bool {
let mut x: u64 = 0;

// function arguments evaluation
let func_res =
func(
{
x = 1;
0
},
{
x = 2;
0
},
{
x = 3;
0
},
x
);

// tuple evaluation
x = 0;
let tuple_res =
(
{
x = 1;
0
},
{
x = 2;
1
},
{
x = 3;
2
},
x
);

// struct evaluation
x = 0;
let struct_res =
MyStruct {
a: {
x = 1;
0
},
b: {
x = 2;
1
},
c: {
x = 3;
2
},
d: x
};

return (func_res == 3) && (tuple_res.3 == 3) && (struct_res.d == 3);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "run"
expected_result = { action = "return", value = 1 }
validate_abi = true
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'cei_pattern_violation_in_func_app-1'
source = 'member'
dependencies = ['std']

[[package]]
name = 'core'
source = 'path+from-root-CC4CE5C392D95953'

[[package]]
name = 'std'
source = 'path+from-root-CC4CE5C392D95953'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
name = "cei_pattern_violation_in_func_app-1"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
contract;

use std::storage::store;

abi TestAbi {
#[storage(write)]
fn deposit(amount: u64);
}

fn pure_function(x: u64) -> u64 {
x
}

impl TestAbi for Contract {
#[storage(write)]
fn deposit(amount: u64) {
// the function argument is a code block with CEI pattern violation
pure_function(
{
// interaction
abi(TestAbi, 0x3dba0a4455b598b7655a7fb430883d96c9527ef275b49739e7b0ad12f8280eae).deposit(amount);
// effect -- therefore violation of CEI where effect should go before interaction
store(0x3dba0a4455b598b7655a7fb430883d96c9527ef275b49739e7b0ad12f8280eae, ());
42
}
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'cei_pattern_violation_in_func_app-2'
source = 'member'
dependencies = ['std']

[[package]]
name = 'core'
source = 'path+from-root-3351E955C8744D1C'

[[package]]
name = 'std'
source = 'path+from-root-3351E955C8744D1C'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
name = "cei_pattern_violation_in_func_app-2"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
contract;

use std::storage::store;

abi TestAbi {
#[storage(write)]
fn deposit(amount: u64);
}

fn pure_function(x: u64, y: u64) -> u64 {
x
}

impl TestAbi for Contract {
#[storage(write)]
fn deposit(amount: u64) {
// 1st function argument is a code block with interaction
// 2nd function argument is a code block with effect
pure_function(
{
// interaction
abi(TestAbi, 0x3dba0a4455b598b7655a7fb430883d96c9527ef275b49739e7b0ad12f8280eae).deposit(amount);
42
},
{
// effect -- therefore violation of CEI where effect should go before interaction
// (assuming left-to-right function argument evaluation)
store(0x3dba0a4455b598b7655a7fb430883d96c9527ef275b49739e7b0ad12f8280eae, ());
43
}
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "compile"

# check: $()Storage modification after external contract interaction in function or method "deposit". Consider making all storage writes before calling another contract
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'cei_pattern_violation_in_func_app-3'
source = 'member'
dependencies = ['std']

[[package]]
name = 'core'
source = 'path+from-root-EAF483A8C0425562'

[[package]]
name = 'std'
source = 'path+from-root-EAF483A8C0425562'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
name = "cei_pattern_violation_in_func_app-3"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
contract;

use std::storage::store;

abi TestAbi {
#[storage(write)]
fn deposit(amount: u64);
}

#[storage(write)]
fn do_something(x: u64) {
// effect
store(0x3dba0a4455b598b7655a7fb430883d96c9527ef275b49739e7b0ad12f8280eae, ());
}

impl TestAbi for Contract {
#[storage(write)]
fn deposit(amount: u64) {
// function's argument is a code block with interaction, function does storage write
do_something(
{
// interaction
abi(TestAbi, 0x3dba0a4455b598b7655a7fb430883d96c9527ef275b49739e7b0ad12f8280eae).deposit(amount);
42
},
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "compile"

# check: $()Storage modification after external contract interaction in function or method "deposit". Consider making all storage writes before calling another contract
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "compile"

# check: $()Storage modification after external contract interaction in function or method "deposit". Consider making all storage writes before calling another contract
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'cei_pattern_violation_in_if_statement-2'
source = 'member'
dependencies = ['std']

[[package]]
name = 'core'
source = 'path+from-root-CFB5A7C521AD2AFD'

[[package]]
name = 'std'
source = 'path+from-root-CFB5A7C521AD2AFD'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
name = "cei_pattern_violation_in_if_statement-2"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Loading

0 comments on commit 88b4cc5

Please sign in to comment.