fix(server): allow Bulletin allocation on wasm - #449
Conversation
95c4a15 to
a10664a
Compare
| pub(super) fn current_unix_secs() -> Result<u64, AllowanceAllocationError> { | ||
| std::time::SystemTime::now() | ||
| .duration_since(std::time::UNIX_EPOCH) | ||
| #[cfg(not(target_arch = "wasm32"))] |
There was a problem hiding this comment.
The description says this takes wall clock from RuntimeServices::current_unix_seconds. That method does not exist, I grepped the whole branch. What is actually here is the same free function reading web_time::SystemTime::now(), so on wasm we read the browser clock directly rather than going through a host service.
Fine by me as behaviour. The pallet checks the period, so a bad clock gets rejected on chain rather than granting the wrong thing. But the description makes it sound like there is a seam we can inject or test, and there is not. Please fix the wording, otherwise someone approves this thinking the clock is host supplied.
| //! (needs the `verifiable` prover and live chain reads). | ||
| //! and submit the resulting unsigned General (v5) extrinsic. Compiles for | ||
| //! every target: the wasm host reaches both chains through its platform | ||
| //! connections, and the `verifiable` prover runs under wasm (the ring-VRF |
There was a problem hiding this comment.
This line says the verifiable prover runs under wasm, which it does, but the number that matters is buried in the verification list: the live claim took 25.3s. That is ring VRF proving in a browser.
Not a blocker and I am glad it works, but 25 seconds of a user interaction needs to be stated up front rather than found at the bottom, and someone has to decide whether this wants a worker so it is not sitting on the main thread.
TarikGul
left a comment
TarikGul
left a comment
There was a problem hiding this comment.
Verification only lists the wasm check with --no-default-features --features wasm-signing-host. CI's Rust workspace job runs the default features one, which is a different configuration, and this PR is entirely about target and feature conditional compilation. Both pass, I ran them, but both should be in the list.
This is on a10664ad and main is 9c3abffb. Main has taken #452 since, which reworked the callback and FFI surface, plus #431, #432, #434 and #400. Rebase and re-run before merging.
Worth knowing #453 is also queued against statement_allowance with its own cfg gating, so whichever lands second will need to reconcile the module split.
| #[cfg(not(target_arch = "wasm32"))] | ||
| use std::time::{Duration, Instant}; | ||
| #[cfg(target_arch = "wasm32")] | ||
| use web_time::{Duration, Instant}; |
There was a problem hiding this comment.
Nit. web_time re-exports core::time::Duration unchanged, only Instant and SystemTime are actually web backed. Importing Duration from there makes it look like it differs per target when it does not. Harmless either way.
TarikGul
left a comment
TarikGul
left a comment
There was a problem hiding this comment.
Approving to not block, but comments should be addressed
2 participants
Problem
truapi-servercompiles Bulletin allowance allocation out onwasm32, returningNativeOnlyeven when a browser host supplies the canonical People/Bulletin runtime services. This prevents the WASM signing host from proving a lite-person ring and claiming long-term Bulletin storage.Change
web-timefor the WASM wall clock while native hosts retainstd::time::SystemTime;@parity/truapi-host.Protocol ownership remains in TrUAPI. Browser hosts only provide the existing chain-service callbacks.
Verification
cargo +nightly fmt --checkRUSTFLAGS="-D warnings" cargo check --target wasm32-unknown-unknown -p truapi-servercargo build -p truapi-server --all-targets --all-featurescargo test -p truapi-server --all-features: 688 passed