-
Notifications
You must be signed in to change notification settings - Fork 108
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
refactor(core-code) Optimisation of read-write structures for code instrumentation #4274
base: master
Are you sure you want to change the base?
Conversation
…or better consistency
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 be refactored as discussed in dm
…harge architecture was changed to type state pattern
…n-refactoring # Conflicts: # gclient/src/api/calls.rs
…n-refactoring # Conflicts: # ethexe/runtime/common/src/lib.rs # gtest/src/manager.rs # gtest/src/manager/block_exec.rs # pallets/gear-program/src/migrations/add_section_sizes.rs # pallets/gear-program/src/migrations/v12_program_code_id_migration.rs # pallets/gear/src/tests.rs # runtime/vara/src/migrations.rs
# Conflicts: # core-processor/src/lib.rs # ethexe/runtime/common/src/lib.rs # pallets/gear-builtin/src/lib.rs # pallets/gear-program/src/migrations/add_section_sizes.rs # pallets/gear-program/src/migrations/v12_program_code_id_migration.rs
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.
Remove the migration. It was already executed on chain
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 don't think it was executed, it's about transforming H256 into CodeId
Line 101 in 4cbcece
pub code_hash: H256, |
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.
Migration is removed from the master branch
pallets/gear-program/src/migrations/v11_code_metadata_delete_migration.rs
Outdated
Show resolved
Hide resolved
pallets/gear-program/src/migrations/v13_split_instrumented_code_migration.rs
Outdated
Show resolved
Hide resolved
if !code_metadata.exports().contains(&dispatch_kind) { | ||
let (destination_id, dispatch, gas_counter, _) = context.into_parts(); | ||
|
||
// Load instrumented binary code from storage. | ||
let code = T::CodeStorage::get_code(code_id).unwrap_or_else(|| { | ||
// `Program` exists, so do code and code len. | ||
let err_msg = format!( | ||
"run_queue_step: failed to get code for the existing program. \ | ||
Program id -'{destination_id:?}', Code id - '{code_id:?}'." | ||
return core_processor::process_success( | ||
SuccessfulDispatchResultKind::Success, | ||
DispatchResult::success(dispatch, destination_id, gas_counter.to_amount()), | ||
); | ||
} |
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.
Should be executed after reinstrumentation
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.
hmm, is it really possible for exports to change after re-instrumentation?
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 a note to rewrite in case re-instrumentation will modify exports
# Conflicts: # core/src/code/mod.rs # ethexe/common/src/db.rs # ethexe/db/src/database.rs # ethexe/processor/src/host/mod.rs # pallets/gear-program/src/migrations/mod.rs # pallets/gear-program/src/migrations/v12_program_code_id_migration.rs # pallets/gear/src/lib.rs # pallets/gear/src/tests.rs # runtime/vara/src/migrations.rs # utils/calc-stack-height/src/main.rs # utils/wasm-builder/src/code_validator.rs
…n-refactoring # Conflicts: # gtest/src/manager.rs # gtest/src/manager/memory.rs
…n-refactoring # Conflicts: # ethexe/processor/src/host/mod.rs # ethexe/runtime/common/src/lib.rs # ethexe/runtime/src/wasm/api/run.rs # gcli/src/meta/mod.rs
…n-refactoring # Conflicts: # ethexe/db/src/database.rs # runtime/vara/src/migrations.rs
Code structure constructor now returns 3 parameters: InstrumentedCode, OriginalCode and CodeMetadata
CodeMetadata contains all the necessary data to infer the need for code instrumentation
Rewritten precharge to type-state pattern