Skip to content

Commit

Permalink
feat: improved error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
heueristik committed Jun 14, 2023
1 parent 5de4f04 commit 6205e3e
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
4 changes: 2 additions & 2 deletions packages/contracts/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Revert with an error if the `grantWithCondition` function in `PermissionManager` is called with a condition address that is not a `IPermissionCondition` implementation.
- Revert with an error if the `applySingleTargetPermissions` function in `PermissionManager` is called with `PermissionLib.Operation.GrantWithCondition`.
- Revert with errors (`ConditionNotAContract`, `ConditionInterfacNotSupported`) if the `grantWithCondition` function in `PermissionManager` is called with a condition address that is not a `IPermissionCondition` implementation.
- Revert with an error (`GrantWithConditionNotSupported`) if the `applySingleTargetPermissions` function in `PermissionManager` is called with `PermissionLib.Operation.GrantWithCondition`.
- Fixed logic bug in the `TokenVoting` and `AddresslistVoting` implementations that caused the `createProposal` function to emit the unvalidated `_startDate` and `_endDate` input arguments (that both can be zero) in the `ProposalCreated` event instead of the validated ones.
- Changed the `createProposal` functions in `Multisig` to allow creating proposals when the `_msgSender()` is listed in the current block.
- Changed the `createProposal` functions in `AddresslistVoting` to allow creating proposals when the `_msgSender()` is listed in the current block.
Expand Down
12 changes: 8 additions & 4 deletions packages/contracts/src/core/permission/PermissionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ abstract contract PermissionManager is Initializable {
address newCondition
);

/// @notice Thrown if an address is not a condition.
/// @notice Thrown if a condition address is not a contract.
/// @param condition The address that is not a contract.
error ConditionInvalid(PermissionConditionBase condition);
error ConditionNotAContract(PermissionConditionBase condition);

/// @notice Thrown if a condition contract does not support the `IPermissionCondition` interface.
/// @param condition The address that is not a contract.
error ConditionInterfacNotSupported(PermissionConditionBase condition);

/// @notice Thrown for `ROOT_PERMISSION_ID` or `EXECUTE_PERMISSION_ID` permission grants where `who` or `where` is `ANY_ADDR`.

Expand Down Expand Up @@ -258,11 +262,11 @@ abstract contract PermissionManager is Initializable {
PermissionConditionBase _condition
) internal virtual {
if (!address(_condition).isContract()) {
revert ConditionInvalid(_condition);
revert ConditionNotAContract(_condition);
}

if (!_condition.supportsInterface(type(IPermissionCondition).interfaceId)) {
revert ConditionInvalid(_condition);
revert ConditionInterfacNotSupported(_condition);
}

if (_where == ANY_ADDR && _who == ANY_ADDR) {
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts/test/core/permission/permission-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('Core: PermissionManager', function () {
ethers.constants.AddressZero
)
)
.to.be.revertedWithCustomError(pm, 'ConditionInvalid')
.to.be.revertedWithCustomError(pm, 'ConditionNotAContract')
.withArgs(ethers.constants.AddressZero);
});

Expand All @@ -200,7 +200,7 @@ describe('Core: PermissionManager', function () {
nonConditionContract.address
)
)
.to.be.revertedWithCustomError(pm, 'ConditionInvalid')
.to.be.revertedWithCustomError(pm, 'ConditionInterfacNotSupported')
.withArgs(nonConditionContract.address);
});

Expand Down

0 comments on commit 6205e3e

Please sign in to comment.