Skip to content

Fix missing global in eval-worker#2072

Merged
FrederikBolding merged 1 commit intomainfrom
fb/fix-missing-global-in-eval
Jan 5, 2024
Merged

Fix missing global in eval-worker#2072
FrederikBolding merged 1 commit intomainfrom
fb/fix-missing-global-in-eval

Conversation

@FrederikBolding
Copy link
Copy Markdown
Member

The global global was missing in the eval-worker, causing eval to fail when a snap tried to access global.

This PR changes the eval-worker to mirror the BaseSnapExecutor, setting up the endowments as globals in an identical way: https://github.com/MetaMask/snaps/blob/main/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts#L404-L417

@FrederikBolding FrederikBolding requested a review from a team as a code owner January 5, 2024 15:35
@FrederikBolding FrederikBolding changed the title Fix missing global in eval-worker Fix missing global in eval-worker Jan 5, 2024
@FrederikBolding FrederikBolding force-pushed the fb/fix-missing-global-in-eval branch from d86a267 to ee9d530 Compare January 5, 2024 15:35
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (01f7d46) 96.28% compared to head (ee9d530) 96.28%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2072   +/-   ##
=======================================
  Coverage   96.28%   96.28%           
=======================================
  Files         270      270           
  Lines        6300     6300           
  Branches     1017     1017           
=======================================
  Hits         6066     6066           
  Misses        234      234           

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

@FrederikBolding FrederikBolding merged commit 7a0d60d into main Jan 5, 2024
@FrederikBolding FrederikBolding deleted the fb/fix-missing-global-in-eval branch January 5, 2024 15:52
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.

2 participants