refactor(runtime,storage): deduplicate JS SDK host functions with JsCollection trait

[xilosada]

Summary

• JsCollection trait + macro: Extract identical new/new_with_id/id/save/load + orphan-attach + recreate-on-missing patterns into a shared trait with js_collection_wrapper! macro for all 9 JS wrapper types
• Host function macros: impl_crdt_new! and impl_crdt_new_with_id! replace 16 identical ~20-line blocks with one-liner invocations; all 22 load_js_*/save_js_* free functions eliminated
• Critical fixes: Merge registry guard now checks JS SDK version byte (incoming[0]==1) to prevent non-deterministic Rust CRDT state loss; LWW honors timestamps; init_state validates schema pointer; new_with_id delegates to js_load (load-or-create) instead of unconditional overwrite

Net reduction: -1,126 lines across js.rs and js_collections.rs.

Test plan

• cargo fmt — clean
• cargo clippy -p calimero-storage -p calimero-runtime — no new warnings (274 vs 275 before)
• cargo build -p merod --release — succeeds
• E2E 2-player poker workflow (32 steps, 15 assertions) — all pass
• E2E 3-player poker workflow (40 steps, 13 assertions) — all pass

Made with Cursor

---

Note

High Risk
Touches core runtime/storage behavior: refactors CRDT persistence/load paths, adds new WASM host imports, and changes merge logic for root documents, any of which can affect state correctness and compatibility.

Overview
Introduces a shared JsCollection trait plus js_collection_wrapper! macro in calimero-storage to centralize wrapper lifecycle (new/new_with_id, id, save/load), including orphan attachment and recreate-on-missing behavior.

Refactors js_collections host functions to use macros for *_new/*_new_with_id, switches collection operations to JsCollection::js_load/js_save, and adds new JS-facing host APIs: new_with_id variants, GCounter aliases, plus new CRDT types (PNCounter, RGA) with (de)serialization helpers.

Updates runtime/system imports to include register_js_sdk_root_merge and init_state (schema pointer validation + empty result for now), raises default VM log limit to 500, and adds a guarded JS SDK root LWW merge registration in storage’s merge registry.

^{Written by Cursor Bugbot for commit c3c8595. This will update automatically on new commits. Configure here.}

[meroreviewer]

🤖 AI Code Reviewer

Reviewed by 3 agents | Quality score: 100% | Review time: 293.9s

💡 4 suggestions. See inline comments.

---

_{🤖 Generated by AI Code Reviewer | Review ID: review-c9b07cf4}

[meroreviewer]

💡 Deserialization allows WASM to inject arbitrary CRDT state

The crdt_counter_deserialize function accepts borsh-encoded data from WASM and directly saves it as a counter; a malicious WASM app could craft serialized state to bypass normal CRDT operation constraints.

Suggested fix:

Document that this API is trusted for sync/migration purposes, or add validation that the deserialized state is consistent with CRDT invariants before saving.

[meroreviewer]

💡 Catch-all credential variant may mask configuration errors

The Other(BTreeMap<String, String>) catch-all silently accepts any credential format; misconfigured credentials will be parsed but silently skipped at runtime rather than flagged early.

Suggested fix:

Consider logging a warning when `Other` credentials are encountered, or validate known protocol types explicitly to fail fast on misconfiguration.

[meroreviewer]

💡 Magic constant 500 for DEFAULT_MAX_LOGS could be derived or configurable

The calculation (~26 fields × ~4 log lines = ~104 minimum) is documented but the final 500 value is a rough estimate; consider making this configurable at runtime.

Suggested fix:

If log limits are hit in production, exposing this via VMLimits configuration would allow tuning without code changes.

[meroreviewer]

💡 new_with_id correctly changed from create to load-or-create

The impl_crdt_new_with_id! macro now delegates to js_load which prevents data loss by returning existing instances; this is a good fix but the macro comment should clarify that js_load creates a new instance if not found to prevent future confusion.

Suggested fix:

Expand the doc comment to explicitly state the load-or-create semantics of `js_load`.

[github-actions]

E2E Rust Apps Failed

One or more E2E workflows (e2e-kv-store, xcall-example) failed after retries.

Please check the workflow logs for more details.

[github-actions]

E2E Blockchain Proposals Failed

The following proposal workflow(s) failed:

• near

Please check the workflow logs for more details.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix prepared fixes for both issues found in the latest run.

• ✅ Fixed: Deserialized JS collections may never persist
  • Added element_mut().update() call after deserialization to mark the element as dirty before calling js_save(), ensuring the data is persisted.
• ✅ Fixed: JS root merge guard matches non-JS payloads
  • Strengthened the JS SDK root merge guard to validate both existing and incoming data structures including version byte, state length, and collections length fields.

Or push these changes by commenting:

@cursor push 0ba9b62032

Preview (0ba9b62032)

diff --git a/crates/runtime/src/logic/host_functions/js_collections.rs b/crates/runtime/src/logic/host_functions/js_collections.rs
--- a/crates/runtime/src/logic/host_functions/js_collections.rs
+++ b/crates/runtime/src/logic/host_functions/js_collections.rs
@@ -5,13 +5,12 @@
 };
 use calimero_storage::{
     address::Id,
+    entities::Data,
     env::{with_runtime_env, RuntimeEnv},
-    interface::Interface,
     js::{
         JsCollection, JsCounter, JsFrozenStorage, JsLwwRegister, JsPnCounter, JsRga,
         JsUnorderedMap, JsUnorderedSet, JsUserStorage, JsVector,
     },
-    store::MainStorage,
 };
 use std::{
     convert::TryFrom,
@@ -1270,6 +1269,7 @@
             Ok(c) => c,
             Err(err) => return self.write_error_message(dest_register_id, err),
         };
+        counter.element_mut().update();
         match counter.js_save() {
             Ok(()) => {
                 self.write_register_bytes(dest_register_id, counter.id().as_bytes())?;
@@ -1449,6 +1449,7 @@
             Ok(c) => c,
             Err(err) => return self.write_error_message(dest_register_id, err),
         };
+        counter.element_mut().update();
         match counter.js_save() {
             Ok(()) => {
                 self.write_register_bytes(dest_register_id, counter.id().as_bytes())?;
@@ -1578,6 +1579,7 @@
             Ok(r) => r,
             Err(err) => return self.write_error_message(dest_register_id, err),
         };
+        rga.element_mut().update();
         match rga.js_save() {
             Ok(()) => {
                 self.write_register_bytes(dest_register_id, rga.id().as_bytes())?;

diff --git a/crates/storage/src/merge/registry.rs b/crates/storage/src/merge/registry.rs
--- a/crates/storage/src/merge/registry.rs
+++ b/crates/storage/src/merge/registry.rs
@@ -108,7 +108,11 @@
         // byte must be 1. Rust SDK roots are plain Borsh structs without this
         // prefix, so they will fail this guard and fall through to their own
         // registered merge function.
-        if incoming.len() < 5 || incoming[0] != 1 {
+        //
+        // IMPORTANT: We must check BOTH existing and incoming to avoid false positives.
+        // If only one side matches, this is not a valid JS-to-JS merge and we should
+        // reject to let the correct merge function handle it.
+        if !is_js_sdk_root_document(existing) || !is_js_sdk_root_document(incoming) {
             return Err("not a JS SDK root document".into());
         }
         // LWW: newer timestamp wins; on tie, incoming wins.
@@ -129,6 +133,40 @@
     let _ = registry.insert(sentinel, merge_fn);
 }
 
+/// Checks if the given bytes represent a JS SDK root document.
+///
+/// JS SDK root format: [version: u8=1][state: u32 len + borsh][collections: u32 len + borsh]
+/// Returns true if the data has the correct structure.
+fn is_js_sdk_root_document(data: &[u8]) -> bool {
+    // Minimum size: 1 (version) + 4 (state len) = 5 bytes
+    if data.len() < 5 {
+        return false;
+    }
+    // Version must be 1
+    if data[0] != 1 {
+        return false;
+    }
+    // Try to read the state length (u32 little-endian at offset 1)
+    let state_len = u32::from_le_bytes([data[1], data[2], data[3], data[4]]) as usize;
+    // After version (1) + state_len field (4) + state data, there should be collections length
+    let collections_offset = 1 + 4 + state_len;
+    // Minimum: need at least 4 more bytes for collections length
+    if data.len() < collections_offset + 4 {
+        return false;
+    }
+    // Read collections length
+    let collections_len = u32::from_le_bytes([
+        data[collections_offset],
+        data[collections_offset + 1],
+        data[collections_offset + 2],
+        data[collections_offset + 3],
+    ]) as usize;
+    // Total expected length
+    let expected_len = collections_offset + 4 + collections_len;
+    // The data should match exactly (or be at least that long for valid structure)
+    data.len() == expected_len
+}
+
 struct JsSdkRootMerge;
 
 /// Clear the merge registry (for testing only)

_{You can send follow-ups to this agent here.}

[cursor]

Deserialized JS collections may never persist

High Severity

js_save treats Interface::save returning Ok(false) as success. After Borsh deserialization, Element.is_dirty is false, so _deserialize host functions call js_save but nothing is written. They still return success and an id, causing silent state loss or empty recreation on later loads.

Additional Locations (2)

• crates/runtime/src/logic/host_functions/js_collections.rs#L1268-L1275
• crates/runtime/src/logic/host_functions/js_collections.rs#L1447-L1454

 

[cursor]

JS root merge guard matches non-JS payloads

High Severity

register_js_sdk_root_merge_fn accepts any payload with len >= 5 and first byte 1. try_merge_registered iterates merge functions in hash-map order and returns the first success, so this guard can falsely accept non-JS root bytes and force LWW instead of the correct CRDT merge.

 

[github-actions]

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

[github-actions]

"This pull request has been automatically closed because it has been inactive for more than 7 days. Please reopen and see this PR through its review if it is essential."