Skip to content

Conversation

@FrederikBolding
Copy link
Member

@FrederikBolding FrederikBolding commented Oct 1, 2025

This PR makes two changes to improve performance in snap_setState and snap_manageState:

  • Stop enforcing JSON sizing limits for preinstalled Snaps
  • Stop double checking JSON validity in snap_setState

Note

Optimizes snap_setState/snap_manageState by using getSnap and getJsonSizeUnsafe to enforce size limits only for non-preinstalled Snaps, removes redundant JSON validation, updates hooks/simulation, adds TextEncoder allowances, and adjusts tests and coverage.

  • RPC methods:
    • snap_setState (packages/snaps-rpc-methods/src/permitted/setState.ts): Add getSnap hook; replace getJsonSize with getJsonSizeUnsafe(..., true); enforce storage size only for non-preinstalled Snaps.
    • snap_manageState (packages/snaps-rpc-methods/src/restricted/manageState.ts): Add getSnap hook; move size check to method implementation using getJsonSizeUnsafe(..., true) (skip for preinstalled); validation now uses isValidJson and no longer checks size.
  • Simulation:
    • Extend restricted hooks with getSnap and wire implementation (packages/snaps-simulation/src/simulation.ts).
  • Utils:
    • Enhance getJsonSizeUnsafe to optionally measure byte length via TextEncoder and add tests (packages/snaps-utils/src/json.ts, json.test.ts).
  • Security/Policies:
    • Allow TextEncoder in LavaMoat policies for @metamask/snaps-utils (iframe/node-process/node-thread/webview policies).
  • Tests:
    • Update tests to use getSnap and cover large state error paths; remove size check from param validation (manageState.test.ts, setState.test.ts).
  • Config:
    • Slightly raise Jest coverage thresholds (packages/snaps-rpc-methods/jest.config.js).

Written by Cursor Bugbot for commit 168610a. This will update automatically on new commits. Configure here.

@FrederikBolding FrederikBolding marked this pull request as ready for review October 1, 2025 13:36
@FrederikBolding FrederikBolding requested a review from a team as a code owner October 1, 2025 13:36
cursor[bot]

This comment was marked as outdated.

Mrtenz
Mrtenz previously approved these changes Oct 1, 2025
@FrederikBolding
Copy link
Member Author

@metamaskbot update-pr

Mrtenz
Mrtenz previously approved these changes Oct 1, 2025
@codecov
Copy link

codecov bot commented Oct 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.29%. Comparing base (0c693f0) to head (168610a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3660      +/-   ##
==========================================
+ Coverage   98.27%   98.29%   +0.01%     
==========================================
  Files         417      417              
  Lines       11902    11907       +5     
  Branches     1843     1848       +5     
==========================================
+ Hits        11697    11704       +7     
+ Misses        205      203       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@FrederikBolding FrederikBolding force-pushed the fb/json-validation-perf branch from 232c9a5 to c30529b Compare October 1, 2025 14:14
@FrederikBolding FrederikBolding force-pushed the fb/json-validation-perf branch 2 times, most recently from b843d1d to ead59ca Compare October 2, 2025 07:54
@FrederikBolding FrederikBolding force-pushed the fb/json-validation-perf branch from ead59ca to 168610a Compare October 2, 2025 08:56
@FrederikBolding FrederikBolding added this pull request to the merge queue Oct 2, 2025
Merged via the queue into main with commit 8b4369d Oct 2, 2025
121 checks passed
@FrederikBolding FrederikBolding deleted the fb/json-validation-perf branch October 2, 2025 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants