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

Proper Revert reasons #342

Merged
merged 3 commits into from
Apr 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
11 changes: 8 additions & 3 deletions compiler/src/yul/mappers/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,15 @@ fn emit(context: &Context, stmt: &Node<fe::FuncStmt>) -> yul::Statement {
}

fn assert(context: &Context, stmt: &Node<fe::FuncStmt>) -> yul::Statement {
if let fe::FuncStmt::Assert { test, msg: _ } = &stmt.kind {
if let fe::FuncStmt::Assert { test, msg } = &stmt.kind {
let test = expressions::expr(context, test);

return statement! { if (iszero([test])) { (revert(0, 0)) } };
return match msg {
Some(val) => {
let msg = expressions::expr(context, val);
statement! { if (iszero([test])) { (revert_with_reason_string([msg])) } }
}
None => statement! { if (iszero([test])) { (revert(0, 0)) } },
};
}

unreachable!()
Expand Down
56 changes: 41 additions & 15 deletions compiler/src/yul/runtime/functions/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,31 @@ use yultsur::*;
/// Return all data runtime functions
pub fn all() -> Vec<yul::Statement> {
vec![
avail(),
alloc(),
alloc_mstoren(),
free(),
alloc(),
avail(),
bytes_mcopys(),
bytes_scopym(),
bytes_scopys(),
bytes_sloadn(),
bytes_sstoren(),
ccopym(),
ceil32(),
cloadn(),
free(),
load_data_string(),
map_value_ptr(),
mcopym(),
mcopys(),
mloadn(),
mstoren(),
revert_with_reason_string(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's the only new one. I just sorted them alphabetically

scopym(),
mcopym(),
scopys(),
mloadn(),
set_zero(),
sloadn(),
bytes_sloadn(),
cloadn(),
mstoren(),
sstoren(),
bytes_sstoren(),
bytes_mcopys(),
bytes_scopym(),
bytes_scopys(),
map_value_ptr(),
ceil32(),
ternary(),
set_zero(),
]
}

Expand Down Expand Up @@ -380,3 +381,28 @@ pub fn load_data_string() -> yul::Statement {
}
}
}

/// Revert with encoded reason string
pub fn revert_with_reason_string() -> yul::Statement {
function_definition! {
function revert_with_reason_string(reason) {
// Function selector for Error(string)
(let ptr := alloc_mstoren(0x08C379A0, 4))

// Write the (fixed) data offset into the next 32 bytes of memory
(pop((alloc_mstoren(0x0000000000000000000000000000000000000000000000000000000000000020, 32))))

// Read the size of the string
(let reason_size := mloadn(reason, 32))

//Copy the whole reason string (length + data) to the current segment of memory
(pop((mcopym(reason , (add(reason_size, 32))))))

// Right pad the reason bytes to a multiple of 32 bytes
(let padding := sub((ceil32(reason_size)), reason_size))
(pop((alloc(padding))))

(revert(ptr, (add(68, (add(reason_size, padding))))))
}
}
}
34 changes: 29 additions & 5 deletions compiler/tests/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,41 @@ fn test_assert() {

let exit1 = harness.capture_call(&mut executor, "bar", &[uint_token(4)]);

assert!(matches!(
exit1,
evm::Capture::Exit((evm::ExitReason::Revert(_), _))
));
match exit1 {
evm::Capture::Exit((evm::ExitReason::Revert(_), output)) => assert_eq!(output.len(), 0),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without a revert string the data will simply be empty (NOT the same as having a revert string of "")

_ => panic!("Did not revert correctly"),
}

let exit2 = harness.capture_call(&mut executor, "bar", &[uint_token(42)]);

assert!(matches!(
exit2,
evm::Capture::Exit((evm::ExitReason::Succeed(_), _))
))
));

let exit3 =
harness.capture_call(&mut executor, "revert_with_static_string", &[uint_token(4)]);

match exit3 {
evm::Capture::Exit((evm::ExitReason::Revert(_), output)) => {
assert_eq!(output, encode_error_reason("Must be greater than five"))
}
_ => panic!("Did not revert correctly"),
}

let reason = "A very looooooooooooooong reason that consumes multiple words";
let exit4 = harness.capture_call(
&mut executor,
"revert_with",
&[uint_token(4), string_token(&reason)],
);

match exit4 {
evm::Capture::Exit((evm::ExitReason::Revert(_), output)) => {
assert_eq!(output, encode_error_reason(reason))
}
_ => panic!("Did not revert correctly"),
}
})
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/tests/fixtures/features/assert.fe
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
contract Foo:
pub def bar(baz: u256):
assert baz > 5
assert baz > 5

pub def revert_with_static_string(baz: u256):
assert baz > 5, "Must be greater than five"

pub def revert_with(baz: u256, reason: string1000):
assert baz > 5, reason
13 changes: 13 additions & 0 deletions compiler/tests/fixtures/solidity/revert_test.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
contract Foo {
function revert_me() public pure returns(uint){
revert("Not enough Ether provided.");
}

function revert_with_long_string() public pure returns(uint){
revert("A muuuuuch longer reason string that consumes multiple words");
}

function revert_with_empty_string() public pure returns(uint){
revert("");
}
}
Loading