[Deprecated] Improve CaveatBuilders to be less risky#20
Closed
jeffsmale90 wants to merge 8 commits into
Closed
Conversation
470ef93 to
17b50f6
Compare
- rename allowEmptyCaveats to allowInsecureFullAuthorityDelegations - require allowInsecureFullAuthorityDelegations when signing and creating delegations
…'re mostly just creating delegation object literals).
17b50f6 to
456a574
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
This changes how a developer interacts with CaveatBuilders in order to reduce the risk of signing delegations with insufficient attenuation.
🔄 What Changed?
List the specific changes made:
allowEmptyCaveatstoallowInsecureUnrestrictedDelegationallowInsecureUnrestrictedDelegationnot just when creating a CaveatBuilder, but also when creating, and when signing a DelegationcreateCaveatBuildermoved from root export to/utilscallDatatocalldatain the ExactCalldata terms builder🚀 Why?
Even with the
allowEmptyCaveatsflag, there is a high risk of developers creating delegations with less attenuation than they expect. Because of this, we have made two general changes:It's not much harder to accidentally create empty caveat collections, because the configuration flag is more likely to stand out as a problem, because of it's name. It is now required with creating and signing delegations, which will capture cases when the
CaveatBuidlerinterface is not used, and generally introduces more friction to high risk full authority delegations.Instead of a developer starting with Full Authority, and attenuating that through addition of Caveats, a developer now starts with a "Unit of Authority", which constrains the delegation to interact with a native token, erc20, erc721, ownership, etc, with specific constraints out of the box. The developer may then further attenuate the permission.
🧪 How to Test?
There is reasonable amount of coverage of this new behaviour. A test dapp could be created that creates delegations using these new UnitOfAuthority factories, such as:
List any breaking changes:
No breaking changes
Breaking changes (describe below):
CaveatBuilder now accepts config object for all caveat types
CaveatBuilder may no longer be imported from the root export, but now from the
/utilsexport (we should encourage use of the UnitOfAuthority caveat builder factories.@metamask/delegation-core terms builder now accepts
calldataparameter, rather thancallData(but this is unreleased, so maybe doesn't count).📋 Checklist
Check off completed items:
🔗 Related Issues
Link to related issues:
Closes #
Related to #
📚 Additional Notes
Any additional information, concerns, or context: