-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Core tracedb functionality. #996
Conversation
pub fn new(mut config: Config, path: &Path, extras: Arc<T>) -> Self { | ||
let mut tracedb_path = path.to_path_buf(); | ||
tracedb_path.push("tracedb"); | ||
let tracesdb = Database::open_default(tracedb_path.to_str().unwrap()).unwrap(); |
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.
Will path be automatically created? (path
and tracedb_path
)
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.
yes
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.
what about versioning?
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.
added here: ethcore/parity@0b2855d
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.
what's the idea behind having a separate DB? are we going to start having loads of different DBs now? VM tracing, and fat db (i.e. hash preimages) are both independent additions. is there a storage/memory/compute/file-handle cost to loads of open databases?
Had a quick look, but I need more time to dig into the logic. Would be cool for some more experienced guys to look into general design / logic. |
@@ -245,30 +246,51 @@ impl<'a> Executive<'a> { | |||
} | |||
trace!("Executive::call(params={:?}) self.env_info={:?}", params, self.info); | |||
|
|||
let delegate_call = params.code_address != params.address; |
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.
Is this correct? What if contract A
calls B
which calls back A
?
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.
Ahh... you are right. But this edge case is not trivial to fix. With our current code design Executive
does not know if A
callcode B
which callcode A
, because it's irrelevant to A
state (the result of this operation is the same as A
calling itself).
I suggest leaving this as it is and fixing later, cause it may require additional changes in evm (exposing extra data).
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.
delegate calls are not traced further - if A delegatecalls B delegatecalls A, then, correctly, nothing will be traced.
changes in last few commits
|
@@ -271,6 +268,8 @@ mod tests { | |||
input: vec![0x5], | |||
}), | |||
result: Res::FailedCall, | |||
trace_address: vec![0], |
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.
is this correct? or should it be vec![]
?
first |
you are right! 👍 |
separated core tracedb functionality from pr #924.
99% of this pr is a new code. Removed lines are mostly variables/functions renaming. Most of the new code is added to
ethcore/trace
module. Changes in this pr won't store traces in the database, but they provide everything that is required to do so.this pr includes:
0u8
(fixed encoding 0u8 #992 ,merged)trace::ExecutiveTracer
andtrace::NoopTracer
moved to separate fileshave_tracing
from blockchainthis pr does not include: