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

Entitlement inference in entitlement migration grants too many entitlements #3253

Closed
j1010001 opened this issue Apr 17, 2024 · 4 comments · Fixed by #3266
Closed

Entitlement inference in entitlement migration grants too many entitlements #3253

j1010001 opened this issue Apr 17, 2024 · 4 comments · Fixed by #3266

Comments

@j1010001
Copy link
Member

j1010001 commented Apr 17, 2024

Discord thread

context:

  • Entitlement migration pulls all entitlements that a migrated type needs
  • Disjunction access (e.g function needs access(Owner | Deposit)) is translated to conjunction authorization (auth(Owner, Deposit))

This is high priority, because we want to migrate capabilities with as few entitlements as possible, to minimize the chances of the migration causing safety issues for the migrated contracts. It is difficult to determine subset of entitlements actually needed.

  • Potential solutions:
    • Improve entitlement inference in entitlement migration in general
    • Don't include Owner entitlement in NFT. But other developers' contracts might have similar problem
    • Special case NFT in entitlements migration. But other developers' contracts might have similar problem

Impact: NFTs, majority of contracts

@turbolent
Copy link
Member

Been working on an idea to improve this on the side, will push up a PoC soon.

@turbolent
Copy link
Member

In any case we should improve the migration guide. Opened #3268

@turbolent
Copy link
Member

Keeping this open to wait for feedback on real-world contracts

@turbolent
Copy link
Member

Haven't gotten any negative feedback, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants