search

gnosisguild / zodiac-modifier-roles

Smart account toolkit for role-based access control
https://roles.gnosisguild.org
GNU Lesser General Public License v3.0
80 stars 39 forks source link

Ominiscia Audit Feedback #203

Closed cristovaoth closed 1 year ago

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
zodiac-roles-app ❌ Failed (Inspect) May 8, 2023 9:32am
jfschwarz commented 1 year ago

Adding a proper error for this could be helpful to users: https://github.com/gnosis/zodiac-modifier-roles/blob/ominiscia-audit-feedback/packages/evm/contracts/PermissionChecker.sol#L661

(In case an non-existing/deleted allowance is still referenced from conditions.)

cristovaoth commented 1 year ago

Adding a proper error for this could be helpful to users: https://github.com/gnosis/zodiac-modifier-roles/blob/ominiscia-audit-feedback/packages/evm/contracts/PermissionChecker.sol#L661

(In case an non-existing/deleted allowance is still referenced from conditions.)

Adding a proper error for this could be helpful to users: https://github.com/gnosis/zodiac-modifier-roles/blob/ominiscia-audit-feedback/packages/evm/contracts/PermissionChecker.sol#L661

(In case an non-existing/deleted allowance is still referenced from conditions.)

It's an assert. PermissionLoader ensures a Consumption entry is always there. Deleted/Missing gets entry with Consumption.balance zero

cristovaoth commented 1 year ago

Effectively what happens for Missing/Deleted is an AllowanceExceeded error. I think this is expressed already in a test case