Skip to content
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

2221 feature request handle pvf execution parameters #2227

Merged
merged 17 commits into from
Oct 22, 2024

Conversation

ErakhtinB
Copy link
Contributor

Referenced issues

Closes #2221

Description of the Change

Both kinds (approval and backing) of pvf execution timeout are handled right. WasmExtBulkMemory parameter for WasmEdge is parsed and used for parachain compilation

Possible Drawbacks

Checklist Before Opening a PR

Before you open a Pull Request (PR), please make sure you've completed the following steps and confirm by answering 'Yes' to each item:

  1. Code is formatted: Have you run your code through clang-format to ensure it adheres to the project's coding standards? [Yes|No]
  2. Code is documented: Have you added comments and documentation to your code according to the guidelines in the project's contributing guidelines? [Yes|No]
  3. Self-review: Have you reviewed your own code to ensure it is free of typos, syntax errors, logical errors, and unresolved TODOs or FIXME without linking to an issue? [Yes|No]
  4. Zombienet Tests: Have you ensured that the zombienet tests are passing? Zombienet is a network simulation and testing tool used in this project. It's important to ensure that these tests pass to maintain the stability and reliability of the project. [Yes|No]

@ErakhtinB ErakhtinB linked an issue Oct 2, 2024 that may be closed by this pull request
2 tasks
@ErakhtinB ErakhtinB force-pushed the 2221-feature-request-handle-pvf-execution-parameters branch from 82eaab2 to 5e26dab Compare October 2, 2024 11:00
@ErakhtinB ErakhtinB requested review from turuslan and Harrm October 3, 2024 03:30
turuslan
turuslan previously approved these changes Oct 3, 2024
@ErakhtinB ErakhtinB force-pushed the 2221-feature-request-handle-pvf-execution-parameters branch from 5e26dab to 140b775 Compare October 14, 2024 13:51
@turuslan turuslan dismissed their stale review October 16, 2024 02:56

pending changes

@ErakhtinB ErakhtinB force-pushed the 2221-feature-request-handle-pvf-execution-parameters branch from 0e6c2a1 to 9a2ddde Compare October 17, 2024 12:01
@ErakhtinB ErakhtinB force-pushed the 2221-feature-request-handle-pvf-execution-parameters branch from 9a2ddde to d1a997e Compare October 17, 2024 13:09
Comment on lines 78 to 89
std::vector<uint8_t> wat_to_wasm(std::string_view wat) {
auto module = wat_to_module(kagome::str2byte(wat));
wabt::MemoryStream stream;
if (wabt::Failed(wabt::WriteBinaryModule(
&stream,
module.get(),
wabt::WriteBinaryOptions{{}, true, false, true}))) {
throw std::runtime_error{"Failed to write binary wasm"};
}
return std::move(stream.output_buffer().data);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wabtEncode(watDecodeWat(wat))

Suggested change
std::vector<uint8_t> wat_to_wasm(std::string_view wat) {
auto module = wat_to_module(kagome::str2byte(wat));
wabt::MemoryStream stream;
if (wabt::Failed(wabt::WriteBinaryModule(
&stream,
module.get(),
wabt::WriteBinaryOptions{{}, true, false, true}))) {
throw std::runtime_error{"Failed to write binary wasm"};
}
return std::move(stream.output_buffer().data);
}

Buffer code;
if (not readFile(code, path_compiled)) {
return CompilationError{"read file failed"};
}
/// TODO(erakhtinb) handle wasm bulk memory flag if Binaryen is keeped
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loadCompiled should check flag too

    if (config.wasm_ext_bulk_memory) {
      return CompilationError{"bulk memory is not supported"};
    }

Comment on lines 89 to 97
wabt::MemoryStream stream;
if (wabt::Failed(wabt::WriteBinaryModule(
&stream,
module.get(),
wabt::WriteBinaryOptions{{}, true, false, true}))) {
throw std::runtime_error{"Failed to write binary wasm"};
}
return std::move(stream.output_buffer().data);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
wabt::MemoryStream stream;
if (wabt::Failed(wabt::WriteBinaryModule(
&stream,
module.get(),
wabt::WriteBinaryOptions{{}, true, false, true}))) {
throw std::runtime_error{"Failed to write binary wasm"};
}
return std::move(stream.output_buffer().data);
}
return wabtEncode(module).value();
}

@@ -52,4 +63,41 @@ namespace kagome::runtime {
}
return common::Buffer{std::move(s.output_buffer().data)};
}

inline std::unique_ptr<wabt::Module> watToModule(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May rename to wabtDecodeWat(string_view)

Comment on lines 77 to 83
if (bulk) {
EXPECT_TRUE(instrumented);
code = std::move(instrumented.value());
} else {
EXPECT_FALSE(instrumented);
return nullptr;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (bulk) {
EXPECT_TRUE(instrumented);
code = std::move(instrumented.value());
} else {
EXPECT_FALSE(instrumented);
return nullptr;
}
if (bulk) {
EXPECT_TRUE(instrumented);
} else {
EXPECT_FALSE(instrumented);
}
if (not instrumented) {
return nullptr;
}
code = std::move(instrumented.value());

@ErakhtinB ErakhtinB enabled auto-merge (squash) October 22, 2024 12:10
@ErakhtinB ErakhtinB merged commit f49f45a into master Oct 22, 2024
10 of 11 checks passed
@ErakhtinB ErakhtinB deleted the 2221-feature-request-handle-pvf-execution-parameters branch October 22, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Handle PVF execution parameters
3 participants