Skip to content

Commit c2fed32

Browse files
authored
Merge pull request #5336 from igor-casper/core-198
[VM2] Support ret, revert, rollback
2 parents 27ccb7d + 48c2e75 commit c2fed32

File tree

19 files changed

+302
-62
lines changed

19 files changed

+302
-62
lines changed

binary_port/src/sandboxed_execution.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use casper_types::{
88
/// Errors that can occur during sandboxed execution.
99
#[derive(Debug, Clone, PartialEq, Eq)]
1010
pub enum SandboxedExecutionError {
11-
/// The contract reverted execution.
12-
CalleeReverted,
11+
/// The contract rolled back execution.
12+
CalleeRolledBack,
1313
/// The contract trapped during execution.
1414
CalleeTrapped,
1515
/// The contract ran out of gas.
@@ -27,7 +27,7 @@ pub enum SandboxedExecutionError {
2727
impl core::fmt::Display for SandboxedExecutionError {
2828
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
2929
match self {
30-
SandboxedExecutionError::CalleeReverted => write!(f, "contract reverted"),
30+
SandboxedExecutionError::CalleeRolledBack => write!(f, "contract rolled back"),
3131
SandboxedExecutionError::CalleeTrapped => write!(f, "contract trapped"),
3232
SandboxedExecutionError::CalleeGasDepleted => write!(f, "contract gas depleted"),
3333
SandboxedExecutionError::NotCallable => write!(f, "contract not callable"),

executor/wasm/src/lib.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -988,9 +988,24 @@ impl ExecutorV2 {
988988
messages: final_tracking_copy.messages(),
989989
}),
990990
Err(VMError::Return { flags, data }) => {
991-
let host_error = if flags.contains(ReturnFlags::REVERT) {
992-
// The contract has reverted.
993-
Some(CallError::CalleeReverted)
991+
if flags.contains(ReturnFlags::REVERT) {
992+
let message = data
993+
.as_ref()
994+
.map(|b| String::from_utf8_lossy(b).into_owned())
995+
.unwrap_or_default();
996+
return Ok(ExecuteResult {
997+
host_error: Some(CallError::Api(message)),
998+
output: None,
999+
gas_usage,
1000+
effects: initial_tracking_copy.effects(),
1001+
cache: initial_tracking_copy.cache(),
1002+
messages: initial_tracking_copy.messages(),
1003+
});
1004+
}
1005+
1006+
let host_error = if flags.contains(ReturnFlags::ROLLBACK) {
1007+
// The contract has rolled back.
1008+
Some(CallError::CalleeRolledBack)
9941009
} else {
9951010
// Merge the tracking copy parts since the execution has succeeded.
9961011
initial_tracking_copy.apply_changes(
@@ -1147,7 +1162,7 @@ impl ExecutorV2 {
11471162
}
11481163
let revert_code: u32 = (*revert_code).into();
11491164
output = Some(revert_code.to_le_bytes().to_vec().into()); // Pass serialized revert code as output.
1150-
Some(CallError::CalleeReverted)
1165+
Some(CallError::CalleeRolledBack)
11511166
}
11521167
Some(_) => Some(CallError::CalleeTrapped(TrapCode::UnreachableCodeReached)),
11531168
None => None,
@@ -1309,7 +1324,7 @@ impl Executor for ExecutorV2 {
13091324
error: execute_result
13101325
.host_error
13111326
.map(|call_error| match call_error {
1312-
CallError::CalleeReverted => SandboxedExecutionError::CalleeReverted,
1327+
CallError::CalleeRolledBack => SandboxedExecutionError::CalleeRolledBack,
13131328
CallError::CalleeTrapped(_) => SandboxedExecutionError::CalleeTrapped,
13141329
CallError::CalleeGasDepleted => SandboxedExecutionError::CalleeGasDepleted,
13151330
CallError::NotCallable => SandboxedExecutionError::NotCallable,

executor/wasm/tests/ee_966.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ fn should_run_ee_966_regression_fail_when_growing_mem_past_max() {
458458
assert!(matches!(
459459
result,
460460
Ok(ExecuteWithProviderResult {
461-
host_error: Some(CallError::CalleeReverted),
461+
host_error: Some(CallError::CalleeRolledBack),
462462
..
463463
})
464464
));

executor/wasm/tests/integration.rs

Lines changed: 194 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,192 @@ pub static CHAINSPEC_SYMLINK: Lazy<PathBuf> = Lazy::new(|| {
6464
});
6565

6666
#[test]
67+
fn vm2_should_return_output_to_caller() {
68+
let chainspec_config = ChainspecConfig::from_chainspec_path(&*CHAINSPEC_SYMLINK)
69+
.expect("must get chainspec config");
70+
71+
let mut executor = make_executor(&chainspec_config);
72+
let (global_state, mut state_root_hash, _tempdir) = make_global_state_with_genesis();
73+
let address_generator = make_address_generator();
74+
75+
let install_request = base_install_request_builder(&chainspec_config)
76+
.with_wasm_bytes(read_wasm("vm2-harness.wasm"))
77+
.with_shared_address_generator(Arc::clone(&address_generator))
78+
.with_transferred_value(0)
79+
.with_entry_point("initialize".to_string())
80+
.with_input(Bytes::from(b"".as_slice()))
81+
.build()
82+
.expect("should build");
83+
84+
let create_result = run_create_contract(
85+
&mut executor,
86+
&global_state,
87+
state_root_hash,
88+
install_request,
89+
);
90+
91+
let contract_hash = EntityAddr::SmartContract(*create_result.smart_contract_addr());
92+
state_root_hash = global_state
93+
.commit_effects(state_root_hash, create_result.effects().clone())
94+
.expect("Should commit");
95+
96+
let input = borsh::to_vec(&(String::from("hi"),))
97+
.map(Bytes::from)
98+
.unwrap();
99+
100+
let execute_request = base_execute_builder(&chainspec_config)
101+
.with_initiator(*DEFAULT_ACCOUNT_HASH)
102+
.with_caller_key(Key::Account(*DEFAULT_ACCOUNT_HASH))
103+
.with_gas_limit(DEFAULT_GAS_LIMIT)
104+
.with_transaction_hash(TRANSACTION_HASH)
105+
.with_execution_kind(ExecutionKind::Stored {
106+
address: contract_hash.value(),
107+
entry_point: "entry_point_without_state_with_args_and_output".to_string(),
108+
})
109+
.with_transferred_value(0)
110+
.with_shared_address_generator(Arc::clone(&address_generator))
111+
.with_block_time(Timestamp::now().into())
112+
.with_state_hash(state_root_hash)
113+
.with_block_height(2)
114+
.with_parent_block_hash(BlockHash::new(Digest::hash(b"block")))
115+
.with_input(input)
116+
.with_runtime_native_config(make_runtime_config(&chainspec_config))
117+
.build()
118+
.expect("should build");
119+
120+
let result = expect_successful_execution(
121+
&mut executor,
122+
&global_state,
123+
state_root_hash,
124+
execute_request,
125+
);
126+
assert!(result.host_error.is_none(), "should be success");
127+
let out = result.output().expect("should have output");
128+
let returned: String = borsh::from_slice(out).expect("borsh string");
129+
assert_eq!(returned, "hiextra");
130+
}
131+
132+
#[test]
133+
fn vm2_rollback_should_return_to_caller_with_data() {
134+
let chainspec_config = ChainspecConfig::from_chainspec_path(&*CHAINSPEC_SYMLINK)
135+
.expect("must get chainspec config");
136+
137+
let mut executor = make_executor(&chainspec_config);
138+
let (global_state, mut state_root_hash, _tempdir) = make_global_state_with_genesis();
139+
let address_generator = make_address_generator();
140+
141+
let install_request = base_install_request_builder(&chainspec_config)
142+
.with_wasm_bytes(read_wasm("vm2-harness.wasm"))
143+
.with_shared_address_generator(Arc::clone(&address_generator))
144+
.with_transferred_value(0)
145+
.with_entry_point("initialize".to_string())
146+
.with_input(Bytes::from(b"".as_slice()))
147+
.build()
148+
.expect("should build");
149+
let create_result = run_create_contract(
150+
&mut executor,
151+
&global_state,
152+
state_root_hash,
153+
install_request,
154+
);
155+
let contract_hash = EntityAddr::SmartContract(*create_result.smart_contract_addr());
156+
state_root_hash = global_state
157+
.commit_effects(state_root_hash, create_result.effects().clone())
158+
.expect("Should commit");
159+
160+
let input = borsh::to_vec(&()).map(Bytes::from).unwrap();
161+
let execute_request = base_execute_builder(&chainspec_config)
162+
.with_initiator(*DEFAULT_ACCOUNT_HASH)
163+
.with_caller_key(Key::Account(*DEFAULT_ACCOUNT_HASH))
164+
.with_gas_limit(DEFAULT_GAS_LIMIT)
165+
.with_transaction_hash(TRANSACTION_HASH)
166+
.with_execution_kind(ExecutionKind::Stored {
167+
address: contract_hash.value(),
168+
entry_point: "emit_revert_with_data".to_string(),
169+
})
170+
.with_transferred_value(0)
171+
.with_shared_address_generator(Arc::clone(&address_generator))
172+
.with_block_time(Timestamp::now().into())
173+
.with_state_hash(state_root_hash)
174+
.with_block_height(2)
175+
.with_parent_block_hash(BlockHash::new(Digest::hash(b"bl0ck")))
176+
.with_input(input)
177+
.with_runtime_native_config(make_runtime_config(&chainspec_config))
178+
.build()
179+
.expect("should build");
180+
181+
let result = executor
182+
.execute_with_provider(state_root_hash, &global_state, execute_request)
183+
.expect("exec ok");
184+
match result.host_error {
185+
Some(CallError::CalleeRolledBack) => {}
186+
Some(other) => panic!("expected CalleeRolledBack got {other:?}"),
187+
None => panic!("expected error"),
188+
}
189+
190+
let out = result.output().expect("should carry rollback data");
191+
assert!(!out.is_empty(), "rollback should return some data");
192+
}
193+
194+
#[test]
195+
fn vm2_revert_should_abort_whole_stack() {
196+
let chainspec_config = ChainspecConfig::from_chainspec_path(&*CHAINSPEC_SYMLINK)
197+
.expect("must get chainspec config");
198+
199+
let mut executor = make_executor(&chainspec_config);
200+
let (global_state, mut state_root_hash, _tempdir) = make_global_state_with_genesis();
201+
let address_generator = make_address_generator();
202+
203+
let install_request = base_install_request_builder(&chainspec_config)
204+
.with_wasm_bytes(read_wasm("vm2-harness.wasm"))
205+
.with_shared_address_generator(Arc::clone(&address_generator))
206+
.with_transferred_value(0)
207+
.with_entry_point("initialize".to_string())
208+
.with_input(Bytes::from(b"".as_slice()))
209+
.build()
210+
.expect("should build");
211+
let create_result = run_create_contract(
212+
&mut executor,
213+
&global_state,
214+
state_root_hash,
215+
install_request,
216+
);
217+
let contract_hash = EntityAddr::SmartContract(*create_result.smart_contract_addr());
218+
state_root_hash = global_state
219+
.commit_effects(state_root_hash, create_result.effects().clone())
220+
.expect("Should commit");
221+
222+
let input = borsh::to_vec(&()).map(Bytes::from).unwrap();
223+
let execute_request = base_execute_builder(&chainspec_config)
224+
.with_initiator(*DEFAULT_ACCOUNT_HASH)
225+
.with_caller_key(Key::Account(*DEFAULT_ACCOUNT_HASH))
226+
.with_gas_limit(DEFAULT_GAS_LIMIT)
227+
.with_transaction_hash(TRANSACTION_HASH)
228+
.with_execution_kind(ExecutionKind::Stored {
229+
address: contract_hash.value(),
230+
entry_point: "emit_revert_without_data".to_string(),
231+
})
232+
.with_transferred_value(0)
233+
.with_shared_address_generator(Arc::clone(&address_generator))
234+
.with_block_time(Timestamp::now().into())
235+
.with_state_hash(state_root_hash)
236+
.with_block_height(2)
237+
.with_parent_block_hash(BlockHash::new(Digest::hash(b"bl0ck")))
238+
.with_input(input)
239+
.with_runtime_native_config(make_runtime_config(&chainspec_config))
240+
.build()
241+
.expect("should build");
242+
243+
let result = executor
244+
.execute_with_provider(state_root_hash, &global_state, execute_request)
245+
.expect("exec ok");
246+
match result.host_error {
247+
Some(CallError::Api(_)) => {}
248+
Some(other) => panic!("expected Api(_) got {other:?}"),
249+
None => panic!("expected error"),
250+
}
251+
}
252+
67253
fn harness() {
68254
let chainspec_config = ChainspecConfig::from_chainspec_path(&*CHAINSPEC_SYMLINK)
69255
.expect("must get chainspec config");
@@ -257,11 +443,11 @@ fn should_revert_invalid_system_option() {
257443
if let Ok(exec_result) = result {
258444
assert!(exec_result.host_error.is_some(), "should have error");
259445
match exec_result.host_error {
260-
Some(CallError::CalleeReverted) => {
446+
Some(CallError::CalleeRolledBack) => {
261447
// noop, expected outcome
262448
}
263449
Some(err) => {
264-
panic!("expected: CalleeReverted actual: {}", err);
450+
panic!("expected: CalleeRolledBack actual: {}", err);
265451
}
266452
None => {
267453
panic!("should have error")
@@ -1321,10 +1507,12 @@ fn casper_return_fails_if_contract_uses_unsupported_flags() {
13211507
let result = executor.execute_with_provider(state_root_hash, &global_state, execute_request);
13221508
assert!(result.is_err());
13231509
let err: ExecuteWithProviderError = result.expect_err("should have error details");
1324-
assert!(matches!(
1325-
err,
1326-
ExecuteWithProviderError::Execute(ExecuteError::ReturnFlagsNotSupported(2))
1327-
));
1510+
match err {
1511+
ExecuteWithProviderError::Execute(ExecuteError::ReturnFlagsNotSupported(v)) => {
1512+
assert!(v != 0, "invalid flags should be non-zero");
1513+
}
1514+
other => panic!("expected ReturnFlagsNotSupported, got {other:?}"),
1515+
}
13281516
}
13291517

13301518
#[test]

executor/wasm_common/src/error.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ pub enum TrapCode {
124124
}
125125

126126
pub const CALLEE_SUCCEEDED: u32 = 0;
127-
pub const CALLEE_REVERTED: u32 = 1;
127+
pub const CALLEE_ROLLED_BACK: u32 = 1;
128128
pub const CALLEE_TRAPPED: u32 = 2;
129129
pub const CALLEE_GAS_DEPLETED: u32 = 3;
130130
pub const CALLEE_NOT_CALLABLE: u32 = 4;
@@ -136,7 +136,7 @@ pub const CALLEE_API_ERROR: u32 = 5;
136136
pub enum CallError {
137137
/// Callee contract reverted.
138138
#[error("callee reverted")]
139-
CalleeReverted,
139+
CalleeRolledBack,
140140
/// Called contract trapped.
141141
#[error("callee trapped: {0}")]
142142
CalleeTrapped(TrapCode),
@@ -156,7 +156,7 @@ impl CallError {
156156
#[must_use]
157157
pub fn into_u32(self) -> u32 {
158158
match self {
159-
Self::CalleeReverted => CALLEE_REVERTED,
159+
Self::CalleeRolledBack => CALLEE_ROLLED_BACK,
160160
Self::CalleeTrapped(_) => CALLEE_TRAPPED,
161161
Self::CalleeGasDepleted => CALLEE_GAS_DEPLETED,
162162
Self::NotCallable => CALLEE_NOT_CALLABLE,

executor/wasm_common/src/flags.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ bitflags! {
77
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
88
pub struct ReturnFlags: u32 {
99
/// If this bit is set, the host should return the value to the caller and all the execution effects are reverted.
10-
const REVERT = 0x0000_0001;
10+
const ROLLBACK = 0x0000_0001;
11+
/// If this bit is set, the host should abort the entire call stack.
12+
/// The optional return data is interpreted as a UTF-8 message.
13+
const REVERT = 0x0000_0002;
1114
}
1215

1316
#[repr(transparent)]
@@ -30,16 +33,16 @@ mod tests {
3033
#[test]
3134
fn test_return_flags() {
3235
assert_eq!(ReturnFlags::empty().bits(), 0x0000_0000);
33-
assert_eq!(ReturnFlags::REVERT.bits(), 0x0000_0001);
36+
assert_eq!(ReturnFlags::ROLLBACK.bits(), 0x0000_0001);
3437
}
3538

3639
#[test]
3740
fn creating_from_invalid_bit_flags_does_not_fail() {
3841
let return_flags_ret_1 = ReturnFlags::from_bits(u32::MAX);
3942
assert_eq!(return_flags_ret_1, None);
4043

41-
let maybe_revert = ReturnFlags::from_bits(0x0000_0001);
42-
assert_eq!(maybe_revert, Some(ReturnFlags::REVERT));
44+
let maybe_rollback = ReturnFlags::from_bits(0x0000_0001);
45+
assert_eq!(maybe_rollback, Some(ReturnFlags::ROLLBACK));
4346

4447
let maybe_empty = ReturnFlags::from_bits(0x0000_0000);
4548
assert_eq!(maybe_empty, Some(ReturnFlags::empty()));

executor/wasm_host/src/system/transfer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ pub fn transfer<R: GlobalStateReader>(
100100
match transfer_result {
101101
Ok(()) => Ok(()),
102102
Err(casper_types::system::mint::Error::InsufficientFunds) => {
103-
Err(DispatchError::Call(CallError::CalleeReverted))
103+
Err(DispatchError::Call(CallError::CalleeRolledBack))
104104
}
105105
Err(casper_types::system::mint::Error::GasLimit) => {
106106
Err(DispatchError::Call(CallError::CalleeGasDepleted))

executor/wasm_interface/src/sandboxed_execution.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use casper_types::{
88
/// Errors that can occur during sandboxed execution.
99
#[derive(Debug, Clone, PartialEq, Eq)]
1010
pub enum SandboxedExecutionError {
11-
/// The contract reverted execution.
12-
CalleeReverted,
11+
/// The contract rolled back execution for the callee.
12+
CalleeRolledBack,
1313
/// The contract trapped during execution.
1414
CalleeTrapped,
1515
/// The contract ran out of gas.
@@ -27,7 +27,7 @@ pub enum SandboxedExecutionError {
2727
impl core::fmt::Display for SandboxedExecutionError {
2828
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
2929
match self {
30-
SandboxedExecutionError::CalleeReverted => write!(f, "contract reverted"),
30+
SandboxedExecutionError::CalleeRolledBack => write!(f, "contract rolled back"),
3131
SandboxedExecutionError::CalleeTrapped => write!(f, "contract trapped"),
3232
SandboxedExecutionError::CalleeGasDepleted => write!(f, "contract gas depleted"),
3333
SandboxedExecutionError::NotCallable => write!(f, "contract not callable"),

node/src/components/binary_port/utils.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ pub(super) fn map_sandbox_error(
3232
match maybe_error {
3333
Some(error) => {
3434
let ret = match error {
35-
InnerSandboxedExecutionError::CalleeReverted => {
36-
SandboxedExecutionError::CalleeReverted
35+
InnerSandboxedExecutionError::CalleeRolledBack => {
36+
SandboxedExecutionError::CalleeRolledBack
3737
}
3838
InnerSandboxedExecutionError::CalleeTrapped => {
3939
SandboxedExecutionError::CalleeTrapped

0 commit comments

Comments
 (0)