From 3e45ab2d516062a13711bae546ba9f2ee60754c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Wed, 4 Oct 2023 16:15:41 -0600 Subject: [PATCH] Improve `AccessManaged` and `AuthorityUtils` tests (#4632) Co-authored-by: Hadrien Croubois Co-authored-by: Francisco Giordano (cherry picked from commit 0560576c7affdcba3c0158598dcbaa4b7db4df68) --- contracts/mocks/AuthorityMock.sol | 69 ++++++++++ test/access/manager/AccessManaged.test.js | 142 +++++++++++++++++++++ test/access/manager/AuthorityUtils.test.js | 91 +++++++++++++ 3 files changed, 302 insertions(+) create mode 100644 contracts/mocks/AuthorityMock.sol create mode 100644 test/access/manager/AccessManaged.test.js create mode 100644 test/access/manager/AuthorityUtils.test.js diff --git a/contracts/mocks/AuthorityMock.sol b/contracts/mocks/AuthorityMock.sol new file mode 100644 index 000000000..bf2434b0a --- /dev/null +++ b/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()); + } +} diff --git a/test/access/manager/AccessManaged.test.js b/test/access/manager/AccessManaged.test.js new file mode 100644 index 000000000..9e94af615 --- /dev/null +++ b/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()'), + }); + }); + }); +}); diff --git a/test/access/manager/AuthorityUtils.test.js b/test/access/manager/AuthorityUtils.test.js new file mode 100644 index 000000000..346be030b --- /dev/null +++ b/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'); + }); + }); + }); +});