Remove PVF memory reuse

.githooks/pre-commit

@@ -14,7 +14,7 @@ fi
 
 # check clang-format binary
 CLANG_FORMAT_ENABLED=1
-CLANG_FORMAT=$(which clang-format-15)
+CLANG_FORMAT=$(which clang-format-15 2>/dev/null)
 if [ -z "${CLANG_FORMAT}" ]; then
   CLANG_FORMAT=$(which clang-format)
   if [ -z "${CLANG_FORMAT}" ]; then

CMakePresets.json

@@ -3,7 +3,7 @@
     "configurePresets": [
         {
             "name": "base",
-            "toolchainFile": "${sourceDir}/cmake/toolchain/clang-15_cxx20.cmake",
+            "toolchainFile": "${sourceDir}/cmake/toolchain/clang-16_cxx20.cmake",
             "generator": "Ninja",
             "hidden": true
         },

core/parachain/pvf/kagome_pvf_worker.cpp

@@ -12,6 +12,7 @@
 #include <string>
 #include <system_error>
 #include <vector>
+#include "runtime/wabt/instrument.hpp"
 
 #ifdef __linux__
 #include <linux/landlock.h>

core/parachain/pvf/pool.cpp

@@ -44,7 +44,7 @@ namespace kagome::parachain {
 
   PvfPool::PvfPool(const application::AppConfiguration &app_config,
                    std::shared_ptr<runtime::ModuleFactory> module_factory,
-                   std::shared_ptr<runtime::InstrumentWasm> instrument)
+                   std::shared_ptr<runtime::WasmInstrumenter> instrument)
       : pool_{std::make_shared<runtime::RuntimeInstancesPoolImpl>(
           app_config,
           std::move(module_factory),

core/parachain/pvf/pool.hpp

@@ -15,7 +15,7 @@ namespace kagome::application {
 }  // namespace kagome::application
 
 namespace kagome::runtime {
-  class InstrumentWasm;
+  class WasmInstrumenter;
   class ModuleFactory;
   class RuntimeInstancesPoolImpl;
 }  // namespace kagome::runtime
@@ -28,7 +28,7 @@ namespace kagome::parachain {
    public:
     PvfPool(const application::AppConfiguration &app_config,
             std::shared_ptr<runtime::ModuleFactory> module_factory,
-            std::shared_ptr<runtime::InstrumentWasm> instrument);
+            std::shared_ptr<runtime::WasmInstrumenter> instrument);
 
     auto &pool() const {
       return pool_;

core/parachain/pvf/pvf_impl.cpp

@@ -12,6 +12,7 @@
 #include "application/app_state_manager.hpp"
 #include "blockchain/block_tree.hpp"
 #include "common/visitor.hpp"
+#include "log/profiling_logger.hpp"
 #include "metrics/histogram_timer.hpp"
 #include "parachain/pvf/module_precompiler.hpp"
 #include "parachain/pvf/pool.hpp"
@@ -26,6 +27,7 @@
 #include "runtime/module.hpp"
 #include "runtime/module_repository.hpp"
 #include "runtime/runtime_code_provider.hpp"
+#include "runtime/runtime_context.hpp"
 #include "runtime/runtime_instances_pool.hpp"
 #include "scale/std_variant.hpp"
 
@@ -44,7 +46,7 @@ OUTCOME_CPP_DEFINE_CATEGORY(kagome::parachain, PvfError, e) {
   using kagome::parachain::PvfError;
   switch (e) {
     case PvfError::PERSISTED_DATA_HASH:
-      return "Incorrect Perssted Data hash";
+      return "Incorrect Persisted Data hash";
     case PvfError::NO_CODE:
       return "No code";
     case PvfError::NO_PERSISTED_DATA:
@@ -63,6 +65,8 @@ OUTCOME_CPP_DEFINE_CATEGORY(kagome::parachain, PvfError, e) {
       return "Commitments hash mismatch";
     case PvfError::OUTPUTS:
       return "ValidationResult is invalid";
+    case PvfError::COMPILATION_ERROR:
+      return "PVF code failed to compile";
   }
   return "unknown error (kagome::parachain::PvfError)";
 }
@@ -145,6 +149,7 @@ namespace kagome::parachain {
       std::shared_ptr<crypto::Sr25519Provider> sr25519_provider,
       std::shared_ptr<runtime::ParachainHost> parachain_api,
       std::shared_ptr<runtime::Executor> executor,
+      std::shared_ptr<runtime::ModuleFactory> module_factory,
       std::shared_ptr<runtime::RuntimeContextFactory> ctx_factory,
       PvfThreadPool &pvf_thread_pool,
       std::shared_ptr<application::AppStateManager> app_state_manager,
@@ -159,6 +164,7 @@ namespace kagome::parachain {
         ctx_factory_{std::move(ctx_factory)},
         log_{log::createLogger("PVF Executor", "pvf_executor")},
         pvf_pool_{std::move(pvf_pool)},
+        module_factory_{std::move(module_factory)},
         precompiler_{std::make_shared<ModulePrecompiler>(
             ModulePrecompiler::Config{config_.precompile_threads_num},
             parachain_api_,
@@ -176,6 +182,7 @@ namespace kagome::parachain {
     SL_INFO(log_,
             "pvf runtime engine {}",
             engines[fmt::underlying(pvf_runtime_engine(*app_configuration_))]);
+    BOOST_ASSERT(module_factory_ != nullptr);
   }
 
   PvfImpl::~PvfImpl() {
@@ -318,11 +325,24 @@ namespace kagome::parachain {
     constexpr auto name = "validate_block";
     CB_TRYV(pvf_pool_->precompile(code_hash, code_zstd, executor_params));
     if (not app_configuration_->usePvfSubprocess()) {
-      CB_TRY(
-          auto instance,
-          pvf_pool_->pool()->instantiateFromCode(
-              code_hash, [&] { return PvfError::NO_CODE; }, executor_params));
-      CB_TRY(auto ctx, ctx_factory_->stateless(instance));
+      // Reusing instances for PVF calls doesn't work, runtime calls start to
+      // crash on access out of memory bounds
+      KAGOME_PROFILE_START_L(log_, single_process_runtime_instantitation);
+      auto res = module_factory_->loadCompiled(
+          pvf_pool_->pool()->cachePath(code_hash, executor_params));
+      if (!res) {
+        SL_WARN(log_,
+                "Failed to compile PVF code for parachain ID {}: {}",
+                receipt.descriptor.para_id,
+                res.error().message());
+        cb(PvfError::COMPILATION_ERROR);
+        return;
+      }
+      auto &wasm_module = res.value();
+      CB_TRY(auto instance, wasm_module->instantiate());
+      CB_TRY(auto ctx, runtime::RuntimeContextFactory::stateless(instance));
+      KAGOME_PROFILE_END_L(log_, single_process_runtime_instantitation);
+      KAGOME_PROFILE_START_L(log_, single_process_runtime_call);
       return cb(executor_->call<ValidationResult>(ctx, name, params));
     }
     workers_->execute({

core/parachain/pvf/pvf_impl.hpp

@@ -13,6 +13,7 @@
 #include "crypto/sr25519_provider.hpp"
 #include "log/logger.hpp"
 #include "runtime/runtime_api/parachain_host.hpp"
+#include "runtime/wabt/instrument.hpp"
 
 namespace kagome {
   class PoolHandler;
@@ -31,6 +32,7 @@ namespace kagome::runtime {
   class Executor;
   class RuntimeContextFactory;
   class RuntimeInstancesPool;
+  class ModuleFactory;
 }  // namespace kagome::runtime
 
 namespace kagome::parachain {
@@ -46,6 +48,7 @@ namespace kagome::parachain {
     OUTPUTS,
     PERSISTED_DATA_HASH,
     NO_CODE,
+    COMPILATION_ERROR
   };
 }  // namespace kagome::parachain
 
@@ -86,6 +89,7 @@ namespace kagome::parachain {
             std::shared_ptr<crypto::Sr25519Provider> sr25519_provider,
             std::shared_ptr<runtime::ParachainHost> parachain_api,
             std::shared_ptr<runtime::Executor> executor,
+            std::shared_ptr<runtime::ModuleFactory> module_factory,
             std::shared_ptr<runtime::RuntimeContextFactory> ctx_factory,
             PvfThreadPool &pvf_thread_pool,
             std::shared_ptr<application::AppStateManager> app_state_manager,
@@ -132,6 +136,7 @@ namespace kagome::parachain {
     log::Logger log_;
 
     std::shared_ptr<PvfPool> pvf_pool_;
+    std::shared_ptr<runtime::ModuleFactory> module_factory_;
     std::shared_ptr<ModulePrecompiler> precompiler_;
     std::shared_ptr<PoolHandler> pvf_thread_handler_;
     std::shared_ptr<application::AppConfiguration> app_configuration_;

core/parachain/pvf/workers.cpp

@@ -145,7 +145,9 @@ namespace kagome::parachain {
               return job.cb(r.error());
             }
             self->writeCode(
-                std::move(job), {std::move(process)}, std::move(used));
+                std::move(job),
+                {.process = std::move(process), .code = std::nullopt},
+                std::move(used));
           });
       return;
     }

core/runtime/binaryen/instance_environment_factory.cpp

@@ -9,6 +9,7 @@
 #include "host_api/host_api_factory.hpp"
 #include "runtime/binaryen/binaryen_memory_provider.hpp"
 #include "runtime/common/trie_storage_provider_impl.hpp"
+#include "runtime/wabt/instrument.hpp"
 
 namespace kagome::runtime::binaryen {
 

core/runtime/common/core_api_factory_impl.cpp

@@ -30,9 +30,8 @@ namespace kagome::runtime {
 
   CoreApiFactoryImpl::CoreApiFactoryImpl(
       std::shared_ptr<crypto::Hasher> hasher,
-      LazySPtr<RuntimeInstancesPool> module_factory)
-      : hasher_{std::move(hasher)},
-        module_factory_{std::move(module_factory)} {}
+      LazySPtr<RuntimeInstancesPool> instance_pool)
+      : hasher_{std::move(hasher)}, instance_pool_{std::move(instance_pool)} {}
 
   outcome::result<std::unique_ptr<RestrictedCore>> CoreApiFactoryImpl::make(
       BufferView code_zstd,
@@ -43,15 +42,15 @@ namespace kagome::runtime {
     if (version) {
       return std::make_unique<GetVersion>(*version);
     }
-    if (not module_factory_.get()) {
+    if (not instance_pool_.get()) {
       return std::errc::not_supported;
     }
     MemoryLimits config;
     BOOST_OUTCOME_TRY(config.heap_alloc_strategy,
                       heapAllocStrategyHeappagesDefault(
                           *storage_provider->getCurrentBatch()));
     OUTCOME_TRY(instance,
-                module_factory_.get()->instantiateFromCode(
+                instance_pool_.get()->instantiateFromCode(
                     code_hash,
                     [&] { return std::make_shared<Buffer>(code); },
                     {config}));

core/runtime/common/core_api_factory_impl.hpp

@@ -35,15 +35,15 @@ namespace kagome::runtime {
         public std::enable_shared_from_this<CoreApiFactoryImpl> {
    public:
     explicit CoreApiFactoryImpl(std::shared_ptr<crypto::Hasher> hasher,
-                                LazySPtr<RuntimeInstancesPool> module_factory);
+                                LazySPtr<RuntimeInstancesPool> instance_pool);
 
     outcome::result<std::unique_ptr<RestrictedCore>> make(
         BufferView code,
         std::shared_ptr<TrieStorageProvider> storage_provider) const override;
 
    private:
     std::shared_ptr<crypto::Hasher> hasher_;
-    LazySPtr<RuntimeInstancesPool> module_factory_;
+    LazySPtr<RuntimeInstancesPool> instance_pool_;
   };
 
 }  // namespace kagome::runtime

core/runtime/common/runtime_instances_pool.cpp

@@ -83,7 +83,7 @@ namespace kagome::runtime {
   RuntimeInstancesPoolImpl::RuntimeInstancesPoolImpl(
       const application::AppConfiguration &app_config,
       std::shared_ptr<ModuleFactory> module_factory,
-      std::shared_ptr<InstrumentWasm> instrument,
+      std::shared_ptr<WasmInstrumenter> instrument,
       size_t capacity)
       : cache_dir_{app_config.runtimeCacheDirPath()},
         module_factory_{std::move(module_factory)},

core/runtime/common/runtime_instances_pool.hpp

@@ -22,7 +22,7 @@ namespace kagome::application {
 }  // namespace kagome::application
 
 namespace kagome::runtime {
-  class InstrumentWasm;
+  class WasmInstrumenter;
 
   /**
    * @brief Pool of runtime instances - per state. Encapsulates modules cache.
@@ -34,7 +34,7 @@ namespace kagome::runtime {
     explicit RuntimeInstancesPoolImpl(
         const application::AppConfiguration &app_config,
         std::shared_ptr<ModuleFactory> module_factory,
-        std::shared_ptr<InstrumentWasm> instrument,
+        std::shared_ptr<WasmInstrumenter> instrument,
         size_t capacity = DEFAULT_MODULES_CACHE_SIZE);
 
     outcome::result<std::shared_ptr<ModuleInstance>> instantiateFromCode(
@@ -86,7 +86,7 @@ namespace kagome::runtime {
 
     std::filesystem::path cache_dir_;
     std::shared_ptr<ModuleFactory> module_factory_;
-    std::shared_ptr<InstrumentWasm> instrument_;
+    std::shared_ptr<WasmInstrumenter> instrument_;
 
     std::mutex pools_mtx_;
     Lru<Key, InstancePool> pools_;
@@ -102,5 +102,5 @@ template <>
 struct boost::di::ctor_traits<kagome::runtime::RuntimeInstancesPoolImpl> {
   BOOST_DI_INJECT_TRAITS(const kagome::application::AppConfiguration &,
                          std::shared_ptr<kagome::runtime::ModuleFactory>,
-                         std::shared_ptr<kagome::runtime::InstrumentWasm>);
+                         std::shared_ptr<kagome::runtime::WasmInstrumenter>);
 };

core/runtime/wabt/instrument.cpp

@@ -80,7 +80,7 @@ namespace kagome::runtime {
     return outcome::success();
   }
 
-  WabtOutcome<common::Buffer> prepareBlobForCompilation(
+  WabtOutcome<common::Buffer> instrumentCodeForCompilation(
       common::BufferView code, const MemoryLimits &config) {
     OUTCOME_TRY(module, wabtDecode(code));
     if (config.max_stack_values_num) {
@@ -94,8 +94,8 @@ namespace kagome::runtime {
     return wabtEncode(module);
   }
 
-  WabtOutcome<common::Buffer> InstrumentWasm::instrument(
+  WabtOutcome<common::Buffer> WasmInstrumenter::instrument(
       common::BufferView code, const MemoryLimits &config) const {
-    return prepareBlobForCompilation(code, config);
+    return instrumentCodeForCompilation(code, config);
   }
 }  // namespace kagome::runtime

core/runtime/wabt/instrument.hpp

@@ -28,12 +28,12 @@ namespace kagome::runtime {
    * - set memory limit
    * https://github.com/paritytech/polkadot-sdk/blob/11831df8e709061e9c6b3292facb5d7d9709f151/substrate/client/executor/wasmtime/src/runtime.rs#L651
    */
-  WabtOutcome<common::Buffer> prepareBlobForCompilation(
+  WabtOutcome<common::Buffer> instrumentCodeForCompilation(
       common::BufferView code, const MemoryLimits &config);
 
-  class InstrumentWasm {
+  class WasmInstrumenter {
    public:
-    virtual ~InstrumentWasm() = default;
+    virtual ~WasmInstrumenter() = default;
 
     virtual WabtOutcome<common::Buffer> instrument(
         common::BufferView code, const MemoryLimits &config) const;

core/runtime/wasm_edge/module_factory_impl.cpp

@@ -400,8 +400,12 @@ namespace kagome::runtime::wasm_edge {
   CompilationOutcome<std::shared_ptr<Module>> ModuleFactoryImpl::loadCompiled(
       std::filesystem::path path_compiled) const {
     Buffer code;
-    if (not readFile(code, path_compiled)) {
-      return CompilationError{"read file failed"};
+    std::error_code ec;
+    if (not readFile(code, path_compiled, ec)) {
+      return CompilationError{
+          fmt::format("Failed to read compiled wasm module from '{}': {}",
+                      path_compiled,
+                      ec.message())};
     }
     auto code_hash = hasher_->blake2b_256(code);
     OUTCOME_TRY(configure_ctx, configureCtx());

core/utils/read_file.hpp

@@ -8,6 +8,7 @@
 
 #include <filesystem>
 #include <fstream>
+#include <system_error>
 
 namespace kagome {
 
@@ -25,19 +26,31 @@ namespace kagome {
       };
 
   template <ByteContainer Out>
-  bool readFile(Out &out, const std::filesystem::path &path) {
+  bool readFile(Out &out,
+                const std::filesystem::path &path,
+                std::error_code &ec) {
     std::ifstream file{path, std::ios::binary | std::ios::ate};
     if (not file.good()) {
+      ec = std::error_code{errno, std::system_category()};
       out.clear();
       return false;
     }
     out.resize(file.tellg());
     file.seekg(0);
     file.read(reinterpret_cast<char *>(out.data()), out.size());
     if (not file.good()) {
+      ec = std::error_code{errno, std::system_category()};
       out.clear();
       return false;
     }
+    ec = {};
     return true;
   }
+
+  template <ByteContainer Out>
+  bool readFile(Out &out, const std::filesystem::path &path) {
+    [[maybe_unused]] std::error_code ec;
+    return readFile(out, path, ec);
+  }
+
 }  // namespace kagome