Best practices for testing implementations & proxies with AccessControl

Smart Contracts
,
1

Hey folks! I've been lurking on this forum for the past few days while developing with OpenZeppelin and I really appreciate the quality of conversation. Thanks to everyone working to keep the community running smoothly.

:question: Question
What are best practices for testing Beacon implementation contracts, given access control is set in the initializer?

:arrow_right: Scenario
The system I'm developing creates many copies of contract Event. To maintain upgradability of all these contracts, I am using the Beacon proxy pattern. The Event contract inherits OpenZeppelin's AccessControl (the upgradable version). Here's a stripped-down version of the Event contract:

Event contract code 
contract Event is Initializable, AccessControlUpgradeable {
    // AccessControl
    bytes32 public constant ORGANIZER_ROLE = keccak256('ORGANIZER_ROLE');

    // Event metadata
    enum EventState { Inactive, Active, Completed, Cancelled }
    EventState public state;

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }

    function initialize() external initializer {
        __AccessControl_init();

        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
        _grantRole(ORGANIZER_ROLE, msg.sender);
    }

    function setEventState(EventState newState) public onlyRole(ORGANIZER_ROLE) {
        state = newState;
    }

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override(AccessControlUpgradeable)
        returns (bool supported)
    {
        return super.supportsInterface(interfaceId);
    }

I want to test my code to check that only addresses with the ORGANIZER_ROLE are able to call setEventState. Initially, I began by following the suggestions I found here and here to test the implementation contract directly with unit tests. I am using Hardhat, so I wrote a test like so:

Event contract unit test 
describe('Event (implementation)', () => {
  // Fixture to deploy the implementation contract
  async function deployEvent() {
    const Event = await ethers.getContractFactory('Event');
    const event = await Event.deploy();
    await event.deployed();

    return { event };
  }

  it('allows organizer to update EventState', async () => {
    const { event } = await loadFixture(deployEvent);

    await event.setEventState(1);

    expect(await event.state()).to.equal(1);
  });
});

This test reverts. My understanding is this happens because constructor is called when deploying the implementation contract with Event.deploy(), and initialize is not called, meaning __AccessControl_init and both _grantRole calls are never executed (and initialize can never be executed after deployment, because _disableInitializers locks it). It seems to me the only way to test a function with an access control modifier is to deploy the beacon, deploy a proxy, and test the proxy. Here's an example:

Event proxy test 
describe('Event (proxy)', () => {
  // Fixture to deploy the implementation contract, a beacon contract,
  // and a beacon proxy contract
  async function deployEventBeaconAndProxy() {
    const Event = await ethers.getContractFactory('Event');
    const eventBeacon = await upgrades.deployBeacon(Event);
    await eventBeacon.deployed();

    const eventProxy = await upgrades.deployBeaconProxy(eventBeacon, Event, []);
    await eventProxy.deployed();

    return { eventBeacon, eventProxy };
  }

  it('allows organizer to update EventState', async () => {
    const { eventProxy } = await loadFixture(deployEventBeaconAndProxy);

    await eventProxy.setEventState(1);

    expect(await eventProxy.state()).to.equal(1);
  });
});

This test passes. My understanding is this happens because initialize is called when the proxy is deployed with upgrades.deployBeaconProxy(). Unfortunately, as far as I can tell, this is counter to the advice I find in the OpenZeppelin community to unit test the implementation contract directly and only test the proxy for higher-level functionality, like upgrades.

Now we arrive at the question I posed at the beginning: given access control is set in the initializer, what are best practices for testing Beacon implementation contracts? Should I test functionality that is gated behind access control modifiers by testing the proxy?

Any advice and conversation is much appreciated. Cheers!

1 Like
2

In this case, it seems reasonable to test the access control related functionality through the proxy.