llamaxyz / llama

Llama is an onchain governance and access control framework for smart contracts.
https://llama.xyz
MIT License
47 stars 5 forks source link

feat: change `setRolePermission` parameter from hash to permission data tuple #450

Closed AustinGreen closed 1 year ago

AustinGreen commented 1 year ago

Motivation:

The function signature for assigning a permission to a role was function setRolePermission(uint8 role, bytes32 permissionId, bool hasPermission). This worked nicely because it means the policy does not need to know about the PermissionData struct. The issue was that this made it complicated for offchain infrastructure to derive the (target, selector, strategy) tuple that's the pre-image of the permissionId.

We decided to change it to function setRolePermission(uint8 role, PermissionData memory permissionData, bool hasPermission) to address this concern.

Modifications:

Result:

Offchain infrastructure will be able to easily maintain mappings between (target, selector, strategy) tuples and permissionIds

github-actions[bot] commented 1 year ago

Coverage after merging austin/change-set-role-permission into main will be

83.93%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   LlamaCore.sol99.26%97.30%100%100%292, 393
   LlamaExecutor.sol80%50%100%100%33
   LlamaFactory.sol100%100%100%100%
   LlamaLens.sol72.09%20%100%82.61%175–176, 179, 179, 179–180, 180, 180–181, 181, 181, 189
   LlamaPolicy.sol91.49%88.89%94.59%91.30%324, 361, 361, 361–363, 363, 363, 365–368, 370, 383
   LlamaPolicyMetadata.sol100%100%100%100%
src/accounts
   LlamaAccount.sol100%100%100%100%
src/lib
   ERC721NonTransferableMinimalProxy.sol70.42%72.73%72.73%68.42%102, 104, 106, 118–120, 190, 88, 88, 88, 90, 90, 90, 92, 92, 92, 97, 99
   LlamaUtils.sol100%100%100%100%
   PolicyholderCheckpoints.sol55.88%50%81.82%53.62%125, 179–181, 181, 181–182, 184, 187, 217, 224, 224–226, 228, 228–230, 232, 232–234, 236, 236–238, 257, 260–266, 273, 46, 46–48, 48, 48–49, 51
   SupplyCheckpoints.sol57.14%50%83.33%54.93%131, 183–185, 185, 185–186, 188, 191, 235, 242, 242–244, 246, 246–248, 250, 250–252, 254, 254–256, 275, 278–284, 291, 50, 50–52, 52, 52–53, 55
src/llama-scripts
   LlamaGovernanceScript.sol50%33.33%43.75%53.45%106–108, 115–117, 125–128, 135–137, 146–150, 157–158, 166–168, 67, 74, 74, 76, 89–90, 97–98
src/strategies/absolute
   LlamaAbsolutePeerReview.sol100%100%100%100%
   LlamaAbsoluteQuorum.sol100%100%100%100%
   LlamaAbsoluteStrategyBase.sol94.74%87.50%90.91%97.96%238, 241, 289
src/strategies/relative
   LlamaRelativeHolderQuorum.sol91.67%75%100%100%46, 53
   LlamaRelativeQuantityQuorum.sol0%0%0%0%26, 26, 26–28, 38, 38, 38–40, 45–46, 46, 46–47, 52–53, 53, 53–54
   LlamaRelativeStrategyBase.sol97.59%90.91%100%100%203, 303
   LlamaRelativeUniqueHolderQuorum.sol93.33%83.33%100%100%48, 55