runtime api #3402
runtime api #3402
Conversation
|
I’m not yet allowed to request for reviews, but since you opened the issue, @jvff, a review would be appreciated. |
| #[cfg_attr(with_wasmer, test_case(WasmRuntime::Wasmer, 92357; "wasmer"))] | ||
| #[cfg_attr(with_wasmer, test_case(WasmRuntime::WasmerWithSanitizer, 92917; "wasmer_with_sanitizer"))] | ||
| #[cfg_attr(with_wasmtime, test_case(WasmRuntime::Wasmtime, 92749; "wasmtime"))] | ||
| #[cfg_attr(with_wasmtime, test_case(WasmRuntime::WasmtimeWithSanitizer, 92749; "wasmtime_with_sanitizer"))] |
There was a problem hiding this comment.
Nit: adding an underscore to make it easier to read.
| #[cfg_attr(with_wasmer, test_case(WasmRuntime::Wasmer, 92357; "wasmer"))] | |
| #[cfg_attr(with_wasmer, test_case(WasmRuntime::WasmerWithSanitizer, 92917; "wasmer_with_sanitizer"))] | |
| #[cfg_attr(with_wasmtime, test_case(WasmRuntime::Wasmtime, 92749; "wasmtime"))] | |
| #[cfg_attr(with_wasmtime, test_case(WasmRuntime::WasmtimeWithSanitizer, 92749; "wasmtime_with_sanitizer"))] | |
| #[cfg_attr(with_wasmer, test_case(WasmRuntime::Wasmer, 92_357; "wasmer"))] | |
| #[cfg_attr(with_wasmer, test_case(WasmRuntime::WasmerWithSanitizer, 92_917; "wasmer_with_sanitizer"))] | |
| #[cfg_attr(with_wasmtime, test_case(WasmRuntime::Wasmtime, 92_749; "wasmtime"))] | |
| #[cfg_attr(with_wasmtime, test_case(WasmRuntime::WasmtimeWithSanitizer, 92_749; "wasmtime_with_sanitizer"))] |
| @@ -44,13 +45,15 @@ pub mod views; | |||
|
|
|||
| use std::fmt::Debug; | |||
|
|
|||
| extern crate linera_base as linera_base_crate; | |||
There was a problem hiding this comment.
I don't think we need this. We can keep using linera_base for now.
| mod conversions_from_contract_wit; | ||
| mod conversions_from_service_wit; | ||
| mod conversions_to_contract_wit; | ||
| mod conversions_to_service_wit; |
There was a problem hiding this comment.
It might be possible to join the contract/service module pairs so that we only have conversions_from_wit and conversions_to_wit. We could get the common code and place it in a macro, and then use wit_base_api identifier as the macro's parameter. Then it could be called like:
conversions_to_wit(crate::contract::wit::base_runtime_api);
conversions_to_wit(crate::service::wit::base_runtime_api);
| @@ -31,6 +31,27 @@ where | |||
| balance_owners: Mutex<Option<Vec<AccountOwner>>>, | |||
| } | |||
|
|
|||
| impl<Application> ServiceRuntime<Application> | |||
There was a problem hiding this comment.
I think it would be best to move this to the end of the file. The public methods that users can call are more important than this helper method, so they should come first in the file.
| identifiers::{ | ||
| Account, AccountOwner, ApplicationId, BytecodeId, ChainId, ChannelName, Destination, | ||
| MessageId, Owner, StreamName, | ||
| }, | ||
| ownership::{ChainOwnership, TimeoutConfig}, | ||
| }; | ||
| use linera_base_crate::data_types::BlockHeight; |
There was a problem hiding this comment.
This should still be in the import above.
| pub fn new(runtime: Runtime) -> Self { | ||
| SystemApiData { | ||
| Self { |
There was a problem hiding this comment.
Nit: I usually use Self in the method signatures to keep them short, and the expanded name when constructing to avoid using Self too much and making it harder to find what it Self when glancing at the code.
| Self { | |
| RuntimeApiData { |
There was a problem hiding this comment.
I usually avoid using the non syntactic sugared full name because when refactoring, changing the type's name would require many consequent changes. But I guess this could be done for the constructor.
|
|
||
| #[wit_export(package = "linera:app")] | ||
| impl<Caller, Runtime> ContractSystemApi<Caller> | ||
| #[linera_witty::wit_export(package = "linera:app")] |
There was a problem hiding this comment.
Nit: The full path shouldn't be needed here.
| #[linera_witty::wit_export(package = "linera:app")] | |
| #[wit_export(package = "linera:app")] |
| impl WriteBatch for ContractSyncRuntimeHandle { | ||
| fn write_batch(&mut self, batch: Batch) -> Result<(), ExecutionError> { | ||
| ContractRuntime::write_batch(self, batch) | ||
| #[linera_witty::wit_export(package = "linera:app")] |
There was a problem hiding this comment.
Nit: Same here regarding the path prefix.
| #[linera_witty::wit_export(package = "linera:app")] | |
| #[wit_export(package = "linera:app")] |
addresses #1977