feat: add ALL_PERMISSIONS flag to encoding #384
Conversation
Develop to main
Develop to main
Release v0.9.0
Release v0.9.1
Release v0.9.2
Release v0.10.0
Release v0.11.0
Release v0.11.1
Release v0.12.0
Release v0.13.0
Release v0.14.0-beta.0
Release v0.14.0
Release v0.14.1
Release v0.14.3
chore: release v0.14.4
chore: release v0.15.0
Release v0.16.0
Release v0.17.0
Release v0.17.1
…18.0 Release v0.18.0
Update main branch to sync latest docs
…ase--branches--main--components--erc725.js chore(main): release 0.19.0
…ase--branches--main--components--erc725.js chore(main): release 0.20.0
…ase--branches--main--components--erc725.js chore(main): release 0.20.1
…ase--branches--main--components--erc725.js chore(main): release 0.21.0
…ase--branches--main--components--erc725.js chore(main): release 0.21.1
…ase--branches--main--components--erc725.js chore(main): release 0.21.2
chore: release 0.21.3 (ERC725Alliance#361)
…ase--branches--main--components--erc725.js chore(main): release 0.21.3
…ase--branches--main--components--erc725.js chore(main): release 0.22.0
chore: sync main
…ase--branches--main--components--erc725.js chore(main): release 0.23.0
| @@ -179,7 +179,8 @@ export const LSP6_DEFAULT_PERMISSIONS = { | |||
| DECRYPT : "0x0000000000000000000000000000000000000000000000000000000000100000", | |||
| SIGN : "0x0000000000000000000000000000000000000000000000000000000000200000", | |||
| EXECUTE_RELAY_CALL : "0x0000000000000000000000000000000000000000000000000000000000400000", | |||
| ERC4337_PERMISSION : "0x0000000000000000000000000000000000000000000000000000000000800000" | |||
| ERC4337_PERMISSION : "0x0000000000000000000000000000000000000000000000000000000000800000", | |||
There was a problem hiding this comment.
Suggest adding a comment that this is not a standard permission defined in LSP6, but used for integration of ERC4337 standard.
Otherwise, people might look into the LSP6 specs to understand what this is used for, but will not find it.
Do we have a link in our docs or a guide that mentions this permission? Maybe worth adding it here.
@Hugoo @skimaharvey wdyt?
There was a problem hiding this comment.
i agree that this is confusing. I am not sure if the confusion starts at the lib level but imo more at the LIPs level and probably we should fix it there first
| @@ -1337,6 +1343,7 @@ describe('Running @erc725/erc725.js tests...', () => { | |||
| SIGN: false, | |||
| EXECUTE_RELAY_CALL: false, | |||
| ERC4337_PERMISSION: false, | |||
| ALL_PERMISSIONS: false, | |||
There was a problem hiding this comment.
It might be worth adding some extra tests to tests that when all permission is set and others are set to false, it removes them, etc... More cases
There was a problem hiding this comment.
yes ideally we can randomize them like i did for some tests i added. I find it not efficient to do one test at a time. Will think about it
What does this PR introduce?
ALL_PERMISSIONSflag to encodingLet's you set all permissions on one flag. Also added the logic where you can do combinations where:
ALL_PERMISSIONSwithCHANGE_OWNERgives you the right hex.ALL_PERMISSIONSwith permissions that are already part of theALL_PERMISSIONSvalue does not affect the hex etc