-
Notifications
You must be signed in to change notification settings - Fork 713
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
stream transfer history #4861
base: main_2_6
Are you sure you want to change the base?
stream transfer history #4861
Conversation
execution-trace = ["massa_execution_exports/execution-trace"] | ||
execution-trace = [ | ||
"massa_execution_exports/execution-trace", | ||
"schnellru" |
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.
LRU is not necessary anymore
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.
See comment above...
@@ -91,7 +96,7 @@ massa_event_cache = { workspace = true } | |||
tempfile = { workspace = true, optional = true } | |||
massa_wallet = { workspace = true } | |||
massa-proto-rs = { workspace = true } | |||
schnellru = { workspace = true } | |||
schnellru = { workspace = true, optional = true } |
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 necessary
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.
Removing "optional = true" here add a warning when compiling without any feature:
warning: extern crate `schnellru` is unused in crate `massa_execution_worker`
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.
I added some comments but coming a bit as an outsider on this work, don't hesitate to close irrelevant ones directly!
@@ -75,6 +77,9 @@ pub mod test_exports; | |||
/// types for execution-trace / execution-info | |||
pub mod types_trace_info; | |||
|
|||
/// types for execution-info | |||
pub mod execution_info; |
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 there a reason to separate types_trace_info
and execution_info
here?
@@ -11,6 +11,7 @@ documentation = "https://docs.massa.net/" | |||
execution-trace = ["serde_json"] | |||
dump-block = [] | |||
test-exports = [] | |||
execution-info = ["execution-trace"] |
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.
shouldn't we also add "massa_execution_exports/execution-info"? (or "massa_execution_exports/execution-trace", not sure).
Because in public.rs, we import types that are only valid if "massa_execution_exports/execution-trace" is set, so building this package separately fails (cargo check --package massa_grpc --features execution-info
)
None, | ||
None, | ||
None, | ||
None, |
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.
we don't provide other info here? (the operation_id where the register was done?)
(CoinOrigin::Slash as i32, None, None, None, None) | ||
} | ||
TransferContext::CreateSCStorage => { | ||
(CoinOrigin::CreateScStorage as i32, None, None, None, None) |
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.
same for here and below, can't we get the operation id?
resync_check
flag