Improve AccessManaged and AuthorityUtils tests (#4632)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco Giordano <fg@frang.io>
(cherry picked from commit 0560576)

contracts/mocks/AuthorityMock.sol

@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {IAccessManaged} from "../access/manager/IAccessManaged.sol";
+import {IAuthority} from "../access/manager/IAuthority.sol";
+
+contract NotAuthorityMock is IAuthority {
+ function canCall(address /* caller */, address /* target */, bytes4 /* selector */) external pure returns (bool) {
+ revert("AuthorityNoDelayMock: not implemented");
+ }
+}
+
+contract AuthorityNoDelayMock is IAuthority {
+ bool _immediate;
+
+ function canCall(
+ address /* caller */,
+ address /* target */,
+ bytes4 /* selector */
+ ) external view returns (bool immediate) {
+ return _immediate;
+ }
+
+ function _setImmediate(bool immediate) external {
+ _immediate = immediate;
+ }
+}
+
+contract AuthorityDelayMock {
+ bool _immediate;
+ uint32 _delay;
+
+ function canCall(
+ address /* caller */,
+ address /* target */,
+ bytes4 /* selector */
+ ) external view returns (bool immediate, uint32 delay) {
+ return (_immediate, _delay);
+ }
+
+ function _setImmediate(bool immediate) external {
+ _immediate = immediate;
+ }
+
+ function _setDelay(uint32 delay) external {
+ _delay = delay;
+ }
+}
+
+contract AuthorityNoResponse {
+ function canCall(address /* caller */, address /* target */, bytes4 /* selector */) external view {}
+}
+
+contract AuthoritiyObserveIsConsuming {
+ event ConsumeScheduledOpCalled(address caller, bytes data, bytes4 isConsuming);
+
+ function canCall(
+ address /* caller */,
+ address /* target */,
+ bytes4 /* selector */
+ ) external pure returns (bool immediate, uint32 delay) {
+ return (false, 1);
+ }
+
+ function consumeScheduledOp(address caller, bytes memory data) public {
+ emit ConsumeScheduledOpCalled(caller, data, IAccessManaged(msg.sender).isConsumingScheduledOp());
+ }
+}

test/access/manager/AccessManaged.test.js

@@ -0,0 +1,142 @@
+const { expectEvent, time, expectRevert } = require('@openzeppelin/test-helpers');
+const { selector } = require('../../helpers/methods');
+const { expectRevertCustomError } = require('../../helpers/customError');
+const {
+ time: { setNextBlockTimestamp },
+} = require('@nomicfoundation/hardhat-network-helpers');
+const { impersonate } = require('../../helpers/account');
+
+const AccessManaged = artifacts.require('$AccessManagedTarget');
+const AccessManager = artifacts.require('$AccessManager');
+
+const AuthoritiyObserveIsConsuming = artifacts.require('$AuthoritiyObserveIsConsuming');
+
+contract('AccessManaged', function (accounts) {
+ const [admin, roleMember, other] = accounts;
+
+ beforeEach(async function () {
+ this.authority = await AccessManager.new(admin);
+ this.managed = await AccessManaged.new(this.authority.address);
+ });
+
+ it('sets authority and emits AuthorityUpdated event during construction', async function () {
+ await expectEvent.inConstruction(this.managed, 'AuthorityUpdated', {
+ authority: this.authority.address,
+ });
+ expect(await this.managed.authority()).to.eq(this.authority.address);
+ });
+
+ describe('restricted modifier', function () {
+ const method = 'fnRestricted()';
+
+ beforeEach(async function () {
+ this.selector = selector(method);
+ this.role = web3.utils.toBN(42);
+ await this.authority.$_setTargetFunctionRole(this.managed.address, this.selector, this.role);
+ await this.authority.$_grantRole(this.role, roleMember, 0, 0);
+ });
+
+ it('succeeds when role is granted without execution delay', async function () {
+ await this.managed.methods[method]({ from: roleMember });
+ });
+
+ it('reverts when role is not granted', async function () {
+ await expectRevertCustomError(this.managed.methods[method]({ from: other }), 'AccessManagedUnauthorized', [
+ other,
+ ]);
+ });
+
+ it('panics in short calldata', async function () {
+ // We avoid adding the `restricted` modifier to the fallback function because other tests may depend on it
+ // being accessible without restrictions. We check for the internal `_checkCanCall` instead.
+ await expectRevert.unspecified(this.managed.$_checkCanCall(other, '0x1234'));
+ });
+
+ describe('when role is granted with execution delay', function () {
+ beforeEach(async function () {
+ const executionDelay = web3.utils.toBN(911);
+ await this.authority.$_grantRole(this.role, roleMember, 0, executionDelay);
+ });
+
+ it('reverts if the operation is not scheduled', async function () {
+ const calldata = await this.managed.contract.methods[method]().encodeABI();
+ const opId = await this.authority.hashOperation(roleMember, this.managed.address, calldata);
+
+ await expectRevertCustomError(this.managed.methods[method]({ from: roleMember }), 'AccessManagerNotScheduled', [
+ opId,
+ ]);
+ });
+
+ it('succeeds if the operation is scheduled', async function () {
+ // Arguments
+ const delay = time.duration.hours(12);
+ const calldata = await this.managed.contract.methods[method]().encodeABI();
+
+ // Schedule
+ const timestamp = await time.latest();
+ const scheduledAt = timestamp.addn(1);
+ const when = scheduledAt.add(delay);
+ await setNextBlockTimestamp(scheduledAt);
+ await this.authority.schedule(this.managed.address, calldata, when, {
+ from: roleMember,
+ });
+
+ // Set execution date
+ await setNextBlockTimestamp(when);
+
+ // Shouldn't revert
+ await this.managed.methods[method]({ from: roleMember });
+ });
+ });
+ });
+
+ describe('setAuthority', function () {
+ beforeEach(async function () {
+ this.newAuthority = await AccessManager.new(admin);
+ });
+
+ it('reverts if the caller is not the authority', async function () {
+ await expectRevertCustomError(this.managed.setAuthority(other, { from: other }), 'AccessManagedUnauthorized', [
+ other,
+ ]);
+ });
+
+ it('reverts if the new authority is not a valid authority', async function () {
+ await impersonate(this.authority.address);
+ await expectRevertCustomError(
+ this.managed.setAuthority(other, { from: this.authority.address }),
+ 'AccessManagedInvalidAuthority',
+ [other],
+ );
+ });
+
+ it('sets authority and emits AuthorityUpdated event', async function () {
+ await impersonate(this.authority.address);
+ const { receipt } = await this.managed.setAuthority(this.newAuthority.address, { from: this.authority.address });
+ await expectEvent(receipt, 'AuthorityUpdated', {
+ authority: this.newAuthority.address,
+ });
+ expect(await this.managed.authority()).to.eq(this.newAuthority.address);
+ });
+ });
+
+ describe('isConsumingScheduledOp', function () {
+ beforeEach(async function () {
+ this.authority = await AuthoritiyObserveIsConsuming.new();
+ this.managed = await AccessManaged.new(this.authority.address);
+ });
+
+ it('returns bytes4(0) when not consuming operation', async function () {
+ expect(await this.managed.isConsumingScheduledOp()).to.eq('0x00000000');
+ });
+
+ it('returns isConsumingScheduledOp selector when consuming operation', async function () {
+ const receipt = await this.managed.fnRestricted({ from: other });
+ await expectEvent.inTransaction(receipt.tx, this.authority, 'ConsumeScheduledOpCalled', {
+ caller: other,
+ data: this.managed.contract.methods.fnRestricted().encodeABI(),
+ isConsuming: selector('isConsumingScheduledOp()'),
+ });
+ });
+ });
+});

test/access/manager/AuthorityUtils.test.js

@@ -0,0 +1,91 @@
+require('@openzeppelin/test-helpers');
+
+const AuthorityUtils = artifacts.require('$AuthorityUtils');
+const NotAuthorityMock = artifacts.require('NotAuthorityMock');
+const AuthorityNoDelayMock = artifacts.require('AuthorityNoDelayMock');
+const AuthorityDelayMock = artifacts.require('AuthorityDelayMock');
+const AuthorityNoResponse = artifacts.require('AuthorityNoResponse');
+
+contract('AuthorityUtils', function (accounts) {
+ const [user, other] = accounts;
+
+ beforeEach(async function () {
+ this.mock = await AuthorityUtils.new();
+ });
+
+ describe('canCallWithDelay', function () {
+ describe('when authority does not have a canCall function', function () {
+ beforeEach(async function () {
+ this.authority = await NotAuthorityMock.new();
+ });
+
+ it('returns (immediate = 0, delay = 0)', async function () {
+ const { immediate, delay } = await this.mock.$canCallWithDelay(
+ this.authority.address,
+ user,
+ other,
+ '0x12345678',
+ );
+ expect(immediate).to.equal(false);
+ expect(delay).to.be.bignumber.equal('0');
+ });
+ });
+
+ describe('when authority has no delay', function () {
+ beforeEach(async function () {
+ this.authority = await AuthorityNoDelayMock.new();
+ this.immediate = true;
+ await this.authority._setImmediate(this.immediate);
+ });
+
+ it('returns (immediate, delay = 0)', async function () {
+ const { immediate, delay } = await this.mock.$canCallWithDelay(
+ this.authority.address,
+ user,
+ other,
+ '0x12345678',
+ );
+ expect(immediate).to.equal(this.immediate);
+ expect(delay).to.be.bignumber.equal('0');
+ });
+ });
+
+ describe('when authority replies with a delay', function () {
+ beforeEach(async function () {
+ this.authority = await AuthorityDelayMock.new();
+ this.immediate = true;
+ this.delay = web3.utils.toBN(42);
+ await this.authority._setImmediate(this.immediate);
+ await this.authority._setDelay(this.delay);
+ });
+
+ it('returns (immediate, delay)', async function () {
+ const { immediate, delay } = await this.mock.$canCallWithDelay(
+ this.authority.address,
+ user,
+ other,
+ '0x12345678',
+ );
+ expect(immediate).to.equal(this.immediate);
+ expect(delay).to.be.bignumber.equal(this.delay);
+ });
+ });
+
+ describe('when authority replies with empty data', function () {
+ beforeEach(async function () {
+ this.authority = await AuthorityNoResponse.new();
+ });
+
+ it('returns (immediate = 0, delay = 0)', async function () {
+ const { immediate, delay } = await this.mock.$canCallWithDelay(
+ this.authority.address,
+ user,
+ other,
+ '0x12345678',
+ );
+ expect(immediate).to.equal(false);
+ expect(delay).to.be.bignumber.equal('0');
+ });
+ });
+ });
+});