[Feature] Add dynamic dispatch import resolution across WASM entry points#1208
[Feature] Add dynamic dispatch import resolution across WASM entry points#1208marshacb wants to merge 17 commits into
Conversation
4c5615d to
ae46449
Compare
ae46449 to
15acf8e
Compare
15acf8e to
af672cd
Compare
|
@cursor review |
There was a problem hiding this comment.
Pull request overview
This PR adds support for dynamic dispatch (call.dynamic) import resolution in WASM entry points. Programs using call.dynamic reference targets via field-encoded identifiers at runtime rather than static import declarations, so the existing resolve_imports function did not load these dynamically dispatched targets. This PR introduces resolve_dynamic_imports to load all programs from the user-provided imports object that aren't already in the process, enabling authorization and execution to succeed for programs using call.dynamic.
Changes:
- Adds
resolve_dynamic_importsfunction to load all programs from imports object not already in the process - Integrates dynamic import resolution into 14 WASM entry points across execute, authorize, deploy, and proving request paths
- Adds comprehensive test coverage with simple dynamic dispatch programs and multi-target scenarios
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| wasm/src/programs/manager/mod.rs | Implements resolve_dynamic_imports function and adds test programs and unit tests |
| wasm/src/programs/manager/proving_request.rs | Adds dynamic import resolution to proving request path |
| wasm/src/programs/manager/execute.rs | Adds dynamic import resolution to 6 execution entry points and authorization test |
| wasm/src/programs/manager/deploy.rs | Adds dynamic import resolution to 5 deployment/upgrade entry points |
| wasm/src/programs/manager/authorize.rs | Adds dynamic import resolution to authorization entry points |
| sdk/tests/dynamic-dispatch.test.ts | Adds SDK tests for authorization and proving request with dynamic dispatch |
| sdk/tests/data/dynamic-dispatch.ts | Adds test program constants and field-encoded identifiers |
| e2e/dynamic/index.js | Adds e2e execution tests for single and multiple dynamic imports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
iamalwaysuncomfortable
left a comment
There was a problem hiding this comment.
This is a great first start, but I believe we need to add a few more things here.
Namely:
- We want to add an option at the top level for people to send in program imports that include the proving and verifying keys into the top level of calls of the program manager.
- We want to add the ability for keys to be resolved via any object implementing the
KeyProviderinterface so if keys have been stored, it can retrieve them prior to any call. - In resolve_program imports recursion we probably want to ensure we're adding in any proving and verifying keys stored.
| #[wasm_bindgen] | ||
| #[derive(Clone)] | ||
| pub struct ProgramImports { | ||
| entries: HashMap<String, ProgramEntry>, |
There was a problem hiding this comment.
| entries: HashMap<String, ProgramEntry>, | |
| entries: HashMap<ProgramIDNative, ProgramEntry>, |
We want to store the ProgramID here instead I think.
| program?: string | Program; | ||
| imports?: ProgramImports; | ||
| edition?: number, | ||
| programImportsBuilder?: ProgramImportsBuilder; |
There was a problem hiding this comment.
We could allow this as an option parameter for flexibility, but MOSTLY construct keys inside the ProgramManager functions.
| /// Check whether a specific program has been added. | ||
| /// | ||
| /// @param {string} name The program name. | ||
| /// @returns {boolean} | ||
| #[wasm_bindgen(js_name = "hasProgram")] | ||
| pub fn has_program(&self, name: &str) -> bool { | ||
| self.entries.contains_key(name) | ||
| } | ||
| } |
There was a problem hiding this comment.
Similarly here
| /// Check whether a specific program has been added. | |
| /// | |
| /// @param {string} name The program name. | |
| /// @returns {boolean} | |
| #[wasm_bindgen(js_name = "hasProgram")] | |
| pub fn has_program(&self, name: &str) -> bool { | |
| self.entries.contains_key(name) | |
| } | |
| } | |
| /// Check whether a specific program has been added. | |
| /// | |
| /// @param {string} name The program name. | |
| /// @returns {boolean} | |
| #[wasm_bindgen(js_name = "contains")] | |
| pub fn contains(&self, name: &str) -> bool { | |
| self.entries.contains_key(name) | |
| } | |
| } |
| /// Extract source code from a JS value (plain string or object with `.source`). | ||
| /// | ||
| /// Supports both legacy format (`"source code"`) and structured format | ||
| /// (`{ source: "source code", ... }`). | ||
| pub(crate) fn extract_source(value: &Option<wasm_bindgen::JsValue>) -> Option<String> { | ||
| let value = value.as_ref()?; | ||
| // Plain string format: "source code" | ||
| if let Some(source) = value.as_string() { | ||
| return Some(source); | ||
| } | ||
| // Structured object format: { source: "source code", ... } | ||
| Reflect::get(value, &"source".into()).ok().and_then(|v| v.as_string()) | ||
| } |
There was a problem hiding this comment.
Rename to program just to stick with existing conventions
| /// Extract source code from a JS value (plain string or object with `.source`). | |
| /// | |
| /// Supports both legacy format (`"source code"`) and structured format | |
| /// (`{ source: "source code", ... }`). | |
| pub(crate) fn extract_source(value: &Option<wasm_bindgen::JsValue>) -> Option<String> { | |
| let value = value.as_ref()?; | |
| // Plain string format: "source code" | |
| if let Some(source) = value.as_string() { | |
| return Some(source); | |
| } | |
| // Structured object format: { source: "source code", ... } | |
| Reflect::get(value, &"source".into()).ok().and_then(|v| v.as_string()) | |
| } | |
| /// Extract source code from a JS value (plain string or object with `.program`). | |
| /// | |
| /// Supports both legacy format (`"source code"`) and structured format | |
| /// (`{ program: "source code", ... }`). | |
| pub(crate) fn extract_source(value: &Option<wasm_bindgen::JsValue>) -> Option<String> { | |
| let value = value.as_ref()?; | |
| // Plain string format: "source code" | |
| if let Some(program) = value.as_string() { | |
| return Some(program); | |
| } | |
| // Structured object format: { program: "source code", ... } | |
| Reflect::get(value, &"source".into()).ok().and_then(|v| v.as_string()) | |
| } |
| /// (respecting transitive static imports via depth-first resolution). | ||
| /// 2. Inserts any pre-provided proving and verifying keys directly into | ||
| /// the process, avoiding expensive on-demand key synthesis. | ||
| pub(crate) fn resolve_into(&self, process: &mut ProcessNative) -> Result<(), String> { |
There was a problem hiding this comment.
Maybe we rename this as load_programs?
| pub(crate) fn resolve_into(&self, process: &mut ProcessNative) -> Result<(), String> { | |
| pub(crate) fn load_programs(&self, process: &mut ProcessNative) -> Result<(), String> { |
| if !process.contains_program(program.id()) { | ||
| log(&format!("Importing program: {name}")); | ||
| // Resolve transitive static imports first. | ||
| self.resolve_program_imports(process, &program)?; |
There was a problem hiding this comment.
It looks like this method might skip adding the program keys, perhaps we want to ensure that when we're calling this, any keys stored are also added?
| } | ||
|
|
||
| /// Recursively resolve a program's static imports in depth-first order. | ||
| fn resolve_program_imports(&self, process: &mut ProcessNative, program: &ProgramNative) -> Result<(), String> { |
There was a problem hiding this comment.
We might want to add any program proving and verifying keys here or consider just unifying this method with the method that calls it, because I believe as is, if programs get added here, it wouldn't add any of the proving or verifying keys.
0f51672 to
ad9220e
Compare
54b34eb to
b602f73
Compare
b602f73 to
50962b4
Compare
50962b4 to
dcc4c69
Compare
…namic-dispatch branch
* Add WASM support for dynamic dispatch variant types (DynamicRecord, DynamicFuture, RecordWithDynamicID, ExternalRecordWithDynamicID) * fmt * fix(wasm): normalize dynamic dispatch type strings to match snarkVM serialization and add missing doc comments * add JS/TS fixture tests for dynamic dispatch variant types (record_dynamic, record_with_dynamic_id, external_record_with_dynamic_id)
…ture to match snarkVM (#1210)
* Handle Consensus V14 in deployments * Remove redundant .map_err in latest_stateroot call * Update getOrInitConsensusVersionTestHeights tests and doc comments to handle all currently active test version heights * Deallocate deployment cost tuple
* Add stringToField utility function for converting program and function names to fields for dynamic dispatch * Add exports of the stringToField utility in the js SDK
* Add the dynamic record type * Add JS side tests for the DynamicRecord type * Check visibility on record conversion roundtrip --------- Signed-off-by: Mike Turner <mike@provable.com>
e56f119 to
a2d2ad7
Compare
* Handle Consensus V14 in deployments (#1230) * Handle Consensus V14 in deployments * Remove redundant .map_err in latest_stateroot call * Update getOrInitConsensusVersionTestHeights tests and doc comments to handle all currently active test version heights * Deallocate deployment cost tuple * Updates to make AMM tests works * Also handle upgrade * Address feedback * Cleanup * Cargo fmt lints * Revert .gitignore changes unrelated to this PR * Change core rev to the testnet 4.6.0 candidate * Bump version update to v0.9.18 * Remove unecessary dependencies in the workspace root --------- Co-authored-by: Mike Turner <mike@provable.com>
…ints with comprehensive test coverage
…patch imports in a single pass
… to imported programs
…ix truthy check and malformed JSDoc
…eys across all execution paths
…stubs and review nits
…m to avoid key serialization, fix test stubs
dcc4c69 to
3774d9d
Compare
c0ac20e to
a095833
Compare
6c5a46a to
486b002
Compare
b1382e8 to
3c76a12
Compare
Motivation
Programs using
call.dynamicreference their targets via field-encoded identifiers at runtime, not through static import declarations. The existingresolve_importsonly loads statically declared imports, so dynamically dispatched targets would never be added to the process which would cause authorization and execution to fail for any program usingcall.dynamic.This PR:
resolve_imports_or_builderdispatcher across all WASM entry points (execute,authorize, proving request, and deploy paths)ProgramImportsBuilder, a zero-cost WASM builder that carriesproving/verifyingkeys alongside program source, withprogramNames()/getProgram()for lightweight enumeration andtoObject()/fromObject()for full round-trip serialization including keys as Uint8ArrayKeyStoreinterface for persistent key storage, with automatic load-before-execution (loadKeysFromStore) and save-after-execution (persistExtractedKeys) so the second execution of a program skips key synthesis entirelyLiteral::Identifiervariant from the latestsnarkVMrevTest Plan
WASM tests (Rust):
test_resolve_dynamic_imports- verifies resolve_imports does NOT load dynamic targets, andresolve_dynamic_importsdoestest_resolve_multiple_dynamic_imports- multiple dynamic targets loaded from a single imports objecttest_authorize_dynamic_dispatch-process.authorize()succeeds for acall.dynamicprogram after import resolutiontest_program_imports_program_names- lightweight enumeration without key serializationtest_program_imports_get_program- source retrieval for known/unknown programstest_program_imports_add_key_bytes_*- byte-level key round-trip, validation of missing program, and invalid bytestest_program_imports_to_object_from_object_roundtrip_keys- full structured format round-trip with real VK bytesSDK tests (TypeScript):
buildProgramImportswith static/dynamic imports and network merge,loadKeysFromStorepositive/negative/error paths,persistExtractedKeyswith top-level keys,resolveTopLevelKeysfallback chain,synthesizeKeysauto-persist,run()end-to-end with auto-built builder and post-execution persistenceManual testing:
template-node-ts) updated to useLocalFileKeyStorewithsetKeyStore()for persistent key caching on disktemplate-react-ts) withIndexedDBKeyStore- verified keys persist toIndexedDBafter first execution, second execution logsInserting externally provided proving and verifying keysand skips synthesisNote
Medium Risk
Touches core WASM
ProgramManagerexecution/authorization/deploy/proving-request paths; incorrect import loading or clone semantics could break transaction building for existing consumers.Overview
Adds
ProgramManager::resolve_dynamic_importsto load non-statically-imported programs from the user-providedimportsmap, enabling programs that usecall.dynamicto authorize/execute successfully.Wires this dynamic import resolution (and associated
imports.clone()adjustments) into the WASMProgramManagerentry points for authorization, execution (offline/on-chain/devnode/fee estimation), deployment/upgrade, and proving requests.Expands e2e and SDK test coverage with simple
call.dynamicprograms (single + multiple targets) and new tests forProgramManager.run(),buildAuthorization/buildAuthorizationUnchecked,provingRequest, plus WASM unit tests validating dynamic import loading behavior.Written by Cursor Bugbot for commit af672cd. This will update automatically on new commits. Configure here.