Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BREAKING] Move endowments from Controllers to RPC methods #2155

Merged
merged 6 commits into from
Feb 5, 2024

Conversation

david0xd
Copy link
Contributor

@david0xd david0xd commented Feb 1, 2024

This PR is a BREAKING change because certain elements are moved from one package to another and import across the libraries is affected.

Summary of changes

All endowment related functionalities and specifications are moved from the endowments folder within snaps-controllers to the endowments folder located under snaps-rpc-methods package and exported from there.
Export of certain functions from endowments required adjustments to the index files, so these changes are made as well.
Fixed are related imports of the endowments in another packages affected (e.g. snaps-simulator).
Code coverage thresholds are adjusted to the new calculations since there are no functional or logical changes.

Reasons for the changes

New features being implemented require endowment related specifications and functionalities, in different packages.

The example of this is Dynamic Permissions feature which require processing of the raw permission specification in order to request permission grant to the Permission Controller. This process requires specific utility functions as well as the caveat mappers or related specifications which cannot be imported from snaps-controllers package directly into the snaps-rpc-methods package.

The best solution for now is to move the endowment related code to the snaps-rpc-methods package and export it from there.

@david0xd david0xd self-assigned this Feb 1, 2024
Copy link
Member

@FrederikBolding FrederikBolding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mark this as breaking!

@david0xd david0xd changed the title Move endowments from Controllers to RPC methods [BREAKING] Move endowments from Controllers to RPC methods Feb 1, 2024
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5ea4fb7) 0.00% compared to head (46add89) 96.68%.

❗ Current head 46add89 differs from pull request most recent head c0a4318. Consider uploading reports for the commit c0a4318 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##           main    #2155       +/-   ##
=========================================
+ Coverage      0   96.68%   +96.68%     
=========================================
  Files         0      324      +324     
  Lines         0     7305     +7305     
  Branches      0     1129     +1129     
=========================================
+ Hits          0     7063     +7063     
- Misses        0      242      +242     

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

@david0xd david0xd marked this pull request as ready for review February 1, 2024 15:15
@david0xd david0xd requested a review from a team as a code owner February 1, 2024 15:15
hmalik88
hmalik88 previously approved these changes Feb 1, 2024
Copy link
Contributor

@hmalik88 hmalik88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Adjust coverage thresholds because of the moved files

Adjust coverage thresholds in Controllers
@david0xd david0xd merged commit b68353f into main Feb 5, 2024
142 checks passed
@david0xd david0xd deleted the dd/move-endowments branch February 5, 2024 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants