-
Notifications
You must be signed in to change notification settings - Fork 141
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
Implement execution traces for the FVM #412
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,17 +4,19 @@ use fvm_ipld_encoding::{RawBytes, DAG_CBOR}; | |
use fvm_shared::actor::builtin::Type; | ||
use fvm_shared::address::{Address, Protocol}; | ||
use fvm_shared::econ::TokenAmount; | ||
use fvm_shared::error::ExitCode; | ||
use fvm_shared::error::{ErrorNumber, ExitCode}; | ||
use fvm_shared::version::NetworkVersion; | ||
use fvm_shared::{ActorID, MethodNum, METHOD_SEND}; | ||
use num_traits::Zero; | ||
|
||
use super::{Backtrace, CallManager, InvocationResult, NO_DATA_BLOCK_ID}; | ||
use crate::call_manager::backtrace::Frame; | ||
use crate::call_manager::FinishRet; | ||
use crate::gas::GasTracker; | ||
use crate::kernel::{ClassifyResult, ExecutionError, Kernel, Result}; | ||
use crate::kernel::{ClassifyResult, ExecutionError, Kernel, Result, SyscallError}; | ||
use crate::machine::Machine; | ||
use crate::syscalls::error::Abort; | ||
use crate::trace::{ExecutionEvent, ExecutionTrace, SendParams}; | ||
use crate::{account_actor, syscall_error}; | ||
|
||
/// The default [`CallManager`] implementation. | ||
|
@@ -40,6 +42,8 @@ pub struct InnerDefaultCallManager<M> { | |
call_stack_depth: u32, | ||
/// The current chain of errors, if any. | ||
backtrace: Backtrace, | ||
/// The current execution trace. | ||
exec_trace: ExecutionTrace, | ||
} | ||
|
||
#[doc(hidden)] | ||
|
@@ -73,6 +77,7 @@ where | |
num_actors_created: 0, | ||
call_stack_depth: 0, | ||
backtrace: Backtrace::default(), | ||
exec_trace: vec![], | ||
}))) | ||
} | ||
|
||
|
@@ -87,6 +92,16 @@ where | |
where | ||
K: Kernel<CallManager = Self>, | ||
{ | ||
if self.machine.context().tracing { | ||
self.exec_trace.push(ExecutionEvent::Call(SendParams { | ||
from, | ||
to, | ||
method, | ||
params: params.clone(), | ||
value: value.clone(), | ||
})); | ||
} | ||
|
||
// We check _then_ set because we don't count the top call. This effectivly allows a | ||
// call-stack depth of `max_call_depth + 1` (or `max_call_depth` sub-calls). While this is | ||
// likely a bug, this is how NV15 behaves so we mimic that behavior here. | ||
|
@@ -108,6 +123,20 @@ where | |
self.call_stack_depth += 1; | ||
let result = self.send_unchecked::<K>(from, to, method, params, value); | ||
self.call_stack_depth -= 1; | ||
|
||
if self.machine.context().tracing { | ||
self.exec_trace.push(ExecutionEvent::Return(match result { | ||
Err(ref e) => Err(match e { | ||
ExecutionError::OutOfGas => { | ||
SyscallError::new(ErrorNumber::Forbidden, "out of gas") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these syscall errors numbers the correct ones to use here? |
||
} | ||
ExecutionError::Fatal(_) => SyscallError::new(ErrorNumber::Forbidden, "fatal"), | ||
ExecutionError::Syscall(s) => s.clone(), | ||
}), | ||
Ok(ref v) => Ok(v.clone()), | ||
})); | ||
} | ||
|
||
result | ||
} | ||
|
||
|
@@ -124,12 +153,19 @@ where | |
res | ||
} | ||
|
||
fn finish(mut self) -> (i64, Backtrace, Self::Machine) { | ||
fn finish(mut self) -> (FinishRet, Self::Machine) { | ||
let gas_used = self.gas_tracker.gas_used().max(0); | ||
|
||
let inner = self.0.take().expect("call manager is poisoned"); | ||
// TODO: Having to check against zero here is fishy, but this is what lotus does. | ||
(gas_used, inner.backtrace, inner.machine) | ||
( | ||
FinishRet { | ||
gas_used, | ||
backtrace: inner.backtrace, | ||
exec_trace: inner.exec_trace, | ||
}, | ||
inner.machine, | ||
) | ||
} | ||
|
||
// Accessor methods so the trait can implement some common methods by default. | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -59,12 +59,12 @@ where | |||||
}; | ||||||
|
||||||
// Apply the message. | ||||||
let (res, gas_used, mut backtrace) = self.map_machine(|machine| { | ||||||
let (res, gas_used, mut backtrace, exec_trace) = self.map_machine(|machine| { | ||||||
let mut cm = K::CallManager::new(machine, msg.gas_limit, msg.from, msg.sequence); | ||||||
// This error is fatal because it should have already been acounted for inside | ||||||
// This error is fatal because it should have already been accounted for inside | ||||||
// preflight_message. | ||||||
if let Err(e) = cm.charge_gas(inclusion_cost) { | ||||||
return (Err(e), cm.finish().2); | ||||||
return (Err(e), cm.finish().1); | ||||||
} | ||||||
|
||||||
let result = cm.with_transaction(|cm| { | ||||||
|
@@ -79,8 +79,11 @@ where | |||||
|
||||||
Ok(ret) | ||||||
}); | ||||||
let (gas_used, backtrace, machine) = cm.finish(); | ||||||
(Ok((result, gas_used, backtrace)), machine) | ||||||
let (res, machine) = cm.finish(); | ||||||
( | ||||||
Ok((result, res.gas_used, res.backtrace, res.exec_trace)), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eh, but we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can destructure this on the outside. I'm just suggesting a way to slightly reduce the amount of code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (but it doesn't really matter) |
||||||
machine, | ||||||
) | ||||||
})?; | ||||||
|
||||||
// Extract the exit code and build the result of the message application. | ||||||
|
@@ -142,7 +145,7 @@ where | |||||
msg.sequence, | ||||||
msg.method_num, | ||||||
self.context().epoch | ||||||
))) | ||||||
))); | ||||||
} | ||||||
}; | ||||||
|
||||||
|
@@ -153,12 +156,18 @@ where | |||||
}; | ||||||
|
||||||
match apply_kind { | ||||||
ApplyKind::Explicit => self.finish_message(msg, receipt, failure_info, gas_cost), | ||||||
ApplyKind::Explicit => self | ||||||
.finish_message(msg, receipt, failure_info, gas_cost) | ||||||
.map(|mut apply_ret| { | ||||||
apply_ret.exec_trace = exec_trace; | ||||||
apply_ret | ||||||
}), | ||||||
ApplyKind::Implicit => Ok(ApplyRet { | ||||||
msg_receipt: receipt, | ||||||
failure_info, | ||||||
penalty: TokenAmount::zero(), | ||||||
miner_tip: TokenAmount::zero(), | ||||||
exec_trace, | ||||||
}), | ||||||
} | ||||||
} | ||||||
|
@@ -234,7 +243,7 @@ where | |||||
ExitCode::SYS_SENDER_INVALID, | ||||||
"Sender invalid", | ||||||
miner_penalty_amount, | ||||||
))) | ||||||
))); | ||||||
} | ||||||
}; | ||||||
|
||||||
|
@@ -253,7 +262,7 @@ where | |||||
ExitCode::SYS_SENDER_INVALID, | ||||||
"Sender invalid", | ||||||
miner_penalty_amount, | ||||||
))) | ||||||
))); | ||||||
} | ||||||
}; | ||||||
|
||||||
|
@@ -365,6 +374,7 @@ where | |||||
failure_info, | ||||||
penalty: miner_penalty, | ||||||
miner_tip, | ||||||
exec_trace: vec![], | ||||||
}) | ||||||
} | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
use fvm_ipld_encoding::RawBytes; | ||
use fvm_shared::address::Address; | ||
use fvm_shared::econ::TokenAmount; | ||
use fvm_shared::{ActorID, MethodNum}; | ||
|
||
use crate::call_manager::InvocationResult; | ||
use crate::kernel::SyscallError; | ||
|
||
/// Execution Trace, only for informational and debugging purposes. | ||
pub type ExecutionTrace = Vec<ExecutionEvent>; | ||
|
||
#[derive(Clone, Debug)] | ||
pub enum ExecutionEvent { | ||
Stebalien marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Call(SendParams), | ||
Return(Result<InvocationResult, SyscallError>), | ||
} | ||
|
||
#[derive(Clone, Debug)] | ||
pub struct SendParams { | ||
pub from: ActorID, | ||
pub to: Address, | ||
pub method: MethodNum, | ||
pub params: RawBytes, | ||
pub value: TokenAmount, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This combination of
ref
s andclone
s appeased the checker, but I feel like there's something better that can be done?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, you can use
&result
and remove theref e
and friends, but you still need to clone. On the other hand, this will only happen when tracing is enabled.