Add allowlist module for gating FCM contracts

[peterargue]

Closes: #9

Summary

• New IAllowlist interface (isAllowed(address) → bool + AddressAllowed/AddressDisallowed events) — intentionally narrow so any backing (mapping, merkle, signature, composite) can be swapped in by consumers.
• New Allowlist contract: Ownable, mapping-backed, with allow / disallow / allowBatch / disallowBatch. Idempotent edits (no revert and no emit on no-op, matching OZ _grantRole semantics). ZeroAddress custom error guards the allow path.
• 17 unit tests + 1 fuzz test (256 runs) covering state transitions, events, zero-address rejection, idempotency, batch behavior, owner gating on all four admin functions, and ownership transfer.
• No vault wiring in this PR — FCMVault.sol is untouched. Integration into the vault is a follow-up.

Integration

The intention is for a new Allowlist contract to be deployed for each unique list of address. So the vault could have shared/separate lists for deposits, share holders, etc. The vault can have modifiable IAllowlist implementation addresses, then set the address to address(0) to disable the list.

Test plan

• make ci is green (fmt-check, build, test)
• forge test passes 19/19 (1 existing FCMVaultTest + 18 new AllowlistTest)
• forge fmt --check clean
• No changes to FCMVault.sol

[peterargue]

is there a convention we want to follow for contract/function docs?

[jordanschalm]

Looks like this is the recommended format: https://docs.soliditylang.org/en/latest/natspec-format.html#natspec. Which I think you're more or less following already here.

[jordanschalm]

Do you think this script would hit usage limits if the contract was deployed months in the past?

[peterargue]

maybe. I'm going to switch it over to an enumerable set. it roughly double write time gas without impacting reads, but allows us to drop the complex script for minimal extra solidity

[holyfuchs]

I think a normal mapping would be sufficient here. We should never need a list of all members onchain.
Its unlikely we even need a list offchain, but if we do the typical way of doing it would be to reconstruct it using events.

[jordanschalm]

Looks like this is the recommended format: https://docs.soliditylang.org/en/latest/natspec-format.html#natspec. Which I think you're more or less following already here.

[holyfuchs]

I think it might be simpler to use:
https://docs.openzeppelin.com/contracts/5.x/access-control#role-based-access-control
It should provide the same functionality but its less code we have to audit.

pragma solidity ^0.8.20;
import {AccessControl} from "@openzeppelin/contracts/access/AccessControl.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract AccessControlERC20Mint is ERC20, AccessControl {
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE");
    constructor(address minter, address burner) ERC20("MyToken", "TKN") {
        _grantRole(MINTER_ROLE, minter);
        _grantRole(BURNER_ROLE, burner);
    }
    function mint(address to, uint256 amount) public onlyRole(MINTER_ROLE) {
        _mint(to, amount);
    }
    function burn(address from, uint256 amount) public onlyRole(BURNER_ROLE) {
        _burn(from, amount);
    }
}

[dete]

FWIW- Using EnumerableSet (instead of just a dictionary of bools) adds a tiny amount of storage overhead and code complexity while not being strictly necessary. I'm not opposed (especially since the code is already written!), but it would be less code and fewer tests without enumeration support... 🤔

[peterargue]

I originally implemented it this way. The reason I changed was because I think we will ultimately want a way to view the list of allowed addresses. If we use a dictionary, then we need to consume events to rebuild the list. this is much more complex than using EnumerableSet, so moving a small amount of complexity/storage into the contract temporarily seemed like the pragmatic choice.

[dete]

Strikes me that the most common way for the allow list to be used is via a modifier like onlyAllowed or similar. (Just like onlyOwner from Ownable.) As such, defining that modifier in this contract would be pretty useful!

[peterargue]

good idea

[peterargue]

I think it might be simpler to use: docs.openzeppelin.com/contracts/5.x/access-control#role-based-access-control It should provide the same functionality but its less code we have to audit.

pragma solidity ^0.8.20;
import {AccessControl} from "@openzeppelin/contracts/access/AccessControl.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract AccessControlERC20Mint is ERC20, AccessControl {
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE");
    constructor(address minter, address burner) ERC20("MyToken", "TKN") {
        _grantRole(MINTER_ROLE, minter);
        _grantRole(BURNER_ROLE, burner);
    }
    function mint(address to, uint256 amount) public onlyRole(MINTER_ROLE) {
        _mint(to, amount);
    }
    function burn(address from, uint256 amount) public onlyRole(BURNER_ROLE) {
        _burn(from, amount);
    }
}

That's fair. under the covers, they are doing pretty much the same thing. I think the main benefit of using a separate contract is that we could reuse it for multiple vaults or contracts without duplicating the list in multiple places. if we don't think that's going to be needed, AccessControl is simpler.

2 general question @holyfuchs :

1. do you think we'll want to share the same list across multiple contracts?
2. would we want to be able to disable the allowlist, or are we planning to just deprecate and redeploy the vault with the list removed?

[holyfuchs]

I think the main benefit of using a separate contract is that we could reuse it for multiple vaults or contracts without duplicating the list in multiple places.

https://docs.openzeppelin.com/contracts/5.x/access-control#access-management
They have a contract for that as well :)

do you think we'll want to share the same list across multiple contracts?

Probably not for now. If we ever need it switching to the AccessManager should be easy.

would we want to be able to disable the allowlist, or are we planning to just deprecate and redeploy the vault with the list removed?

I would just deprecate and redeploy. We will most likely redeploy a lot of versions before we ever get to the point of removing the access control.

[holyfuchs]

Suggested change

	bytes32 public constant ALLOWED_ROLE = keccak256("ALLOWED_ROLE");
	bytes32 public constant EARLY_ACCESS_ROLE = keccak256("EARLY_ACCESS_ROLE");