Implement suggestions from audit of 4.9 (#4176)

Co-authored-by: Ernesto García <ernestognw@gmail.com>

contracts/governance/Governor.sol

@@ -17,7 +17,7 @@ import "./IGovernor.sol";
 /**
  * @dev Core of the governance system, designed to be extended though various modules.
  *
- * This contract is abstract and requires several function to be implemented in various modules:
+ * This contract is abstract and requires several functions to be implemented in various modules:
  *
  * - A counting module must implement {quorum}, {_quorumReached}, {_voteSucceeded} and {_countVote}
  * - A voting module must implement {_getVotes}
@@ -27,7 +27,6 @@ import "./IGovernor.sol";
  */
 abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receiver, IERC1155Receiver {
     using DoubleEndedQueue for DoubleEndedQueue.Bytes32Deque;
-    using SafeCast for uint256;
 
     bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)");
     bytes32 public constant EXTENDED_BALLOT_TYPEHASH =
@@ -90,25 +89,34 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
      * @dev Function to receive ETH that will be handled by the governor (disabled if executor is a third party contract)
      */
     receive() external payable virtual {
-        require(_executor() == address(this));
+        require(_executor() == address(this), "Governor: must send to executor");
     }
 
     /**
      * @dev See {IERC165-supportsInterface}.
      */
     function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) {
-        // In addition to the current interfaceId, also support previous version of the interfaceId that did not
-        // include the castVoteWithReasonAndParams() function as standard
+        bytes4 governorCancelId = this.cancel.selector ^ this.proposalProposer.selector;
+
+        bytes4 governorParamsId = this.castVoteWithReasonAndParams.selector ^
+            this.castVoteWithReasonAndParamsBySig.selector ^
+            this.getVotesWithParams.selector;
+
+        // The original interface id in v4.3.
+        bytes4 governor43Id = type(IGovernor).interfaceId ^
+            type(IERC6372).interfaceId ^
+            governorCancelId ^
+            governorParamsId;
+
+        // An updated interface id in v4.6, with params added.
+        bytes4 governor46Id = type(IGovernor).interfaceId ^ type(IERC6372).interfaceId ^ governorCancelId;
+
+        // For the updated interface id in v4.9, we use governorCancelId directly.
+
         return
-            interfaceId ==
-            (type(IGovernor).interfaceId ^
-                type(IERC6372).interfaceId ^
-                this.cancel.selector ^
-                this.castVoteWithReasonAndParams.selector ^
-                this.castVoteWithReasonAndParamsBySig.selector ^
-                this.getVotesWithParams.selector) ||
-            // Previous interface for backwards compatibility
-            interfaceId == (type(IGovernor).interfaceId ^ type(IERC6372).interfaceId ^ this.cancel.selector) ||
+            interfaceId == governor43Id ||
+            interfaceId == governor46Id ||
+            interfaceId == governorCancelId ||
             interfaceId == type(IERC1155Receiver).interfaceId ||
             super.supportsInterface(interfaceId);
     }
@@ -210,9 +218,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
     }
 
     /**
-     * @dev Address of the proposer
+     * @dev Returns the account that created a given proposal.
      */
-    function _proposalProposer(uint256 proposalId) internal view virtual returns (address) {
+    function proposalProposer(uint256 proposalId) public view virtual override returns (address) {
         return _proposals[proposalId].proposer;
     }
 
@@ -283,8 +291,8 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
 
         _proposals[proposalId] = ProposalCore({
             proposer: proposer,
-            voteStart: snapshot.toUint64(),
-            voteEnd: deadline.toUint64(),
+            voteStart: SafeCast.toUint64(snapshot),
+            voteEnd: SafeCast.toUint64(deadline),
             executed: false,
             canceled: false,
             __gap_unused0: 0,
@@ -317,9 +325,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
     ) public payable virtual override returns (uint256) {
         uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
-        ProposalState status = state(proposalId);
+        ProposalState currentState = state(proposalId);
         require(
-            status == ProposalState.Succeeded || status == ProposalState.Queued,
+            currentState == ProposalState.Succeeded || currentState == ProposalState.Queued,
             "Governor: proposal not successful"
         );
         _proposals[proposalId].executed = true;
@@ -415,10 +423,12 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
     ) internal virtual returns (uint256) {
         uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
-        ProposalState status = state(proposalId);
+        ProposalState currentState = state(proposalId);
 
         require(
-            status != ProposalState.Canceled && status != ProposalState.Expired && status != ProposalState.Executed,
+            currentState != ProposalState.Canceled &&
+                currentState != ProposalState.Expired &&
+                currentState != ProposalState.Executed,
             "Governor: proposal not active"
         );
         _proposals[proposalId].canceled = true;

contracts/governance/IGovernor.sol

@@ -152,6 +152,12 @@ abstract contract IGovernor is IERC165, IERC6372 {
      */
     function proposalDeadline(uint256 proposalId) public view virtual returns (uint256);
 
+    /**
+     * @notice module:core
+     * @dev The account that created a proposal.
+     */
+    function proposalProposer(uint256 proposalId) public view virtual returns (address);
+
     /**
      * @notice module:user-config
      * @dev Delay, between the proposal is created and the vote starts. The unit this duration is expressed in depends
@@ -164,7 +170,7 @@ abstract contract IGovernor is IERC165, IERC6372 {
 
     /**
      * @notice module:user-config
-     * @dev Delay, between the vote start and vote ends. The unit this duration is expressed in depends on the clock
+     * @dev Delay between the vote start and vote end. The unit this duration is expressed in depends on the clock
      * (see EIP-6372) this contract uses.
      *
      * NOTE: The {votingDelay} can delay the start of the vote. This must be considered when setting the voting

contracts/governance/TimelockController.sol

@@ -6,7 +6,6 @@ pragma solidity ^0.8.0;
 import "../access/AccessControl.sol";
 import "../token/ERC721/IERC721Receiver.sol";
 import "../token/ERC1155/IERC1155Receiver.sol";
-import "../utils/Address.sol";
 
 /**
  * @dev Contract module which acts as a timelocked controller. When set as the
@@ -137,21 +136,21 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
      * @dev Returns whether an id correspond to a registered operation. This
      * includes both Pending, Ready and Done operations.
      */
-    function isOperation(bytes32 id) public view virtual returns (bool registered) {
+    function isOperation(bytes32 id) public view virtual returns (bool) {
         return getTimestamp(id) > 0;
     }
 
     /**
-     * @dev Returns whether an operation is pending or not.
+     * @dev Returns whether an operation is pending or not. Note that a "pending" operation may also be "ready".
      */
-    function isOperationPending(bytes32 id) public view virtual returns (bool pending) {
+    function isOperationPending(bytes32 id) public view virtual returns (bool) {
         return getTimestamp(id) > _DONE_TIMESTAMP;
     }
 
     /**
-     * @dev Returns whether an operation is ready or not.
+     * @dev Returns whether an operation is ready for execution. Note that a "ready" operation is also "pending".
      */
-    function isOperationReady(bytes32 id) public view virtual returns (bool ready) {
+    function isOperationReady(bytes32 id) public view virtual returns (bool) {
         uint256 timestamp = getTimestamp(id);
         return timestamp > _DONE_TIMESTAMP && timestamp <= block.timestamp;
     }
@@ -159,7 +158,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
     /**
      * @dev Returns whether an operation is done or not.
      */
-    function isOperationDone(bytes32 id) public view virtual returns (bool done) {
+    function isOperationDone(bytes32 id) public view virtual returns (bool) {
         return getTimestamp(id) == _DONE_TIMESTAMP;
     }
 
@@ -167,7 +166,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
      * @dev Returns the timestamp at which an operation becomes ready (0 for
      * unset operations, 1 for done operations).
      */
-    function getTimestamp(bytes32 id) public view virtual returns (uint256 timestamp) {
+    function getTimestamp(bytes32 id) public view virtual returns (uint256) {
         return _timestamps[id];
     }
 
@@ -176,7 +175,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
      *
      * This value can be changed by executing an operation that calls `updateDelay`.
      */
-    function getMinDelay() public view virtual returns (uint256 duration) {
+    function getMinDelay() public view virtual returns (uint256) {
         return _minDelay;
     }
 
@@ -190,7 +189,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
         bytes calldata data,
         bytes32 predecessor,
         bytes32 salt
-    ) public pure virtual returns (bytes32 hash) {
+    ) public pure virtual returns (bytes32) {
         return keccak256(abi.encode(target, value, data, predecessor, salt));
     }
 
@@ -204,14 +203,14 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
         bytes[] calldata payloads,
         bytes32 predecessor,
         bytes32 salt
-    ) public pure virtual returns (bytes32 hash) {
+    ) public pure virtual returns (bytes32) {
         return keccak256(abi.encode(targets, values, payloads, predecessor, salt));
     }
 
     /**
      * @dev Schedule an operation containing a single transaction.
      *
-     * Emits events {CallScheduled} and {CallSalt}.
+     * Emits {CallSalt} if salt is nonzero, and {CallScheduled}.
      *
      * Requirements:
      *
@@ -236,7 +235,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
     /**
      * @dev Schedule an operation containing a batch of transactions.
      *
-     * Emits a {CallSalt} event and one {CallScheduled} event per transaction in the batch.
+     * Emits {CallSalt} if salt is nonzero, and one {CallScheduled} event per transaction in the batch.
      *
      * Requirements:
      *

contracts/governance/compatibility/GovernorCompatibilityBravo.sol

@@ -74,7 +74,7 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
         // Stores the full proposal and fallback to the public (possibly overridden) propose. The fallback is done
         // after the full proposal is stored, so the store operation included in the fallback will be skipped. Here we
         // call `propose` and not `super.propose` to make sure if a child contract override `propose`, whatever code
-        // is added their is also executed when calling this alternative interface.
+        // is added there is also executed when calling this alternative interface.
         _storeProposal(_msgSender(), targets, values, signatures, calldatas, description);
         return propose(targets, values, _encodeCalldata(signatures, calldatas), description);
     }
@@ -110,7 +110,7 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
     /**
      * @dev Cancel a proposal with GovernorBravo logic.
      */
-    function cancel(uint256 proposalId) public virtual {
+    function cancel(uint256 proposalId) public virtual override {
         (
             address[] memory targets,
             uint256[] memory values,
@@ -238,9 +238,9 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
         againstVotes = details.againstVotes;
         abstainVotes = details.abstainVotes;
 
-        ProposalState status = state(proposalId);
-        canceled = status == ProposalState.Canceled;
-        executed = status == ProposalState.Executed;
+        ProposalState currentState = state(proposalId);
+        canceled = currentState == ProposalState.Canceled;
+        executed = currentState == ProposalState.Executed;
     }
 
     /**

contracts/governance/compatibility/IGovernorCompatibilityBravo.sol

@@ -90,6 +90,11 @@ abstract contract IGovernorCompatibilityBravo is IGovernor {
      */
     function execute(uint256 proposalId) public payable virtual;
 
+    /**
+     * @dev Cancels a proposal only if the sender is the proposer or the proposer delegates' voting power dropped below the proposal threshold.
+     */
+    function cancel(uint256 proposalId) public virtual;
+
     /**
      * @dev Part of the Governor Bravo's interface: _"Gets actions of a proposal"_.
      */

contracts/governance/extensions/GovernorPreventLateQuorum.sol

@@ -12,14 +12,12 @@ import "../../utils/math/Math.sol";
  * and try to oppose the decision.
  *
  * If a vote causes quorum to be reached, the proposal's voting period may be extended so that it does not end before at
- * least a given number of blocks have passed (the "vote extension" parameter). This parameter can be set by the
- * governance executor (e.g. through a governance proposal).
+ * least a specified time has passed (the "vote extension" parameter). This parameter can be set through a governance
+ * proposal.
  *
  * _Available since v4.5._
  */
 abstract contract GovernorPreventLateQuorum is Governor {
-    using SafeCast for uint256;
-
     uint64 private _voteExtension;
 
     /// @custom:oz-retyped-from mapping(uint256 => Timers.BlockNumber)
@@ -32,9 +30,9 @@ abstract contract GovernorPreventLateQuorum is Governor {
     event LateQuorumVoteExtensionSet(uint64 oldVoteExtension, uint64 newVoteExtension);
 
     /**
-     * @dev Initializes the vote extension parameter: the number of blocks that are required to pass since a proposal
-     * reaches quorum until its voting period ends. If necessary the voting period will be extended beyond the one set
-     * at proposal creation.
+     * @dev Initializes the vote extension parameter: the time in either number of blocks or seconds (depending on the governor
+     * clock mode) that is required to pass since the moment a proposal reaches quorum until its voting period ends. If
+     * necessary the voting period will be extended beyond the one set during proposal creation.
      */
     constructor(uint64 initialVoteExtension) {
         _setLateQuorumVoteExtension(initialVoteExtension);

contracts/governance/extensions/GovernorTimelockCompound.sol

@@ -21,8 +21,6 @@ import "../../vendor/compound/ICompoundTimelock.sol";
  * _Available since v4.3._
  */
 abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor {
-    using SafeCast for uint256;
-
     ICompoundTimelock private _timelock;
 
     /// @custom:oz-retyped-from mapping(uint256 => GovernorTimelockCompound.ProposalTimelock)
@@ -48,18 +46,18 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor {
     }
 
     /**
-     * @dev Overridden version of the {Governor-state} function with added support for the `Queued` and `Expired` status.
+     * @dev Overridden version of the {Governor-state} function with added support for the `Queued` and `Expired` state.
      */
     function state(uint256 proposalId) public view virtual override(IGovernor, Governor) returns (ProposalState) {
-        ProposalState status = super.state(proposalId);
+        ProposalState currentState = super.state(proposalId);
 
-        if (status != ProposalState.Succeeded) {
-            return status;
+        if (currentState != ProposalState.Succeeded) {
+            return currentState;
         }
 
         uint256 eta = proposalEta(proposalId);
         if (eta == 0) {
-            return status;
+            return currentState;
         } else if (block.timestamp >= eta + _timelock.GRACE_PERIOD()) {
             return ProposalState.Expired;
         } else {
@@ -95,7 +93,7 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor {
         require(state(proposalId) == ProposalState.Succeeded, "Governor: proposal not successful");
 
         uint256 eta = block.timestamp + _timelock.delay();
-        _proposalTimelocks[proposalId] = eta.toUint64();
+        _proposalTimelocks[proposalId] = SafeCast.toUint64(eta);
 
         for (uint256 i = 0; i < targets.length; ++i) {
             require(

contracts/governance/extensions/GovernorTimelockControl.sol

@@ -47,19 +47,19 @@ abstract contract GovernorTimelockControl is IGovernorTimelock, Governor {
     }
 
     /**
-     * @dev Overridden version of the {Governor-state} function with added support for the `Queued` status.
+     * @dev Overridden version of the {Governor-state} function with added support for the `Queued` state.
      */
     function state(uint256 proposalId) public view virtual override(IGovernor, Governor) returns (ProposalState) {
-        ProposalState status = super.state(proposalId);
+        ProposalState currentState = super.state(proposalId);
 
-        if (status != ProposalState.Succeeded) {
-            return status;
+        if (currentState != ProposalState.Succeeded) {
+            return currentState;
         }
 
         // core tracks execution, so we just have to check if successful proposal have been queued.
         bytes32 queueid = _timelockIds[proposalId];
         if (queueid == bytes32(0)) {
-            return status;
+            return currentState;
         } else if (_timelock.isOperationDone(queueid)) {
             return ProposalState.Executed;
         } else if (_timelock.isOperationPending(queueid)) {

contracts/governance/extensions/GovernorVotesQuorumFraction.sol

@@ -14,7 +14,6 @@ import "../../utils/math/SafeCast.sol";
  * _Available since v4.3._
  */
 abstract contract GovernorVotesQuorumFraction is GovernorVotes {
-    using SafeCast for *;
     using Checkpoints for Checkpoints.Trace224;
 
     uint256 private _quorumNumerator; // DEPRECATED in favor of _quorumNumeratorHistory
@@ -59,7 +58,7 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {
         }
 
         // Otherwise, do the binary search
-        return _quorumNumeratorHistory.upperLookupRecent(timepoint.toUint32());
+        return _quorumNumeratorHistory.upperLookupRecent(SafeCast.toUint32(timepoint));
     }
 
     /**
@@ -110,12 +109,12 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {
         // Make sure we keep track of the original numerator in contracts upgraded from a version without checkpoints.
         if (oldQuorumNumerator != 0 && _quorumNumeratorHistory._checkpoints.length == 0) {
             _quorumNumeratorHistory._checkpoints.push(
-                Checkpoints.Checkpoint224({_key: 0, _value: oldQuorumNumerator.toUint224()})
+                Checkpoints.Checkpoint224({_key: 0, _value: SafeCast.toUint224(oldQuorumNumerator)})
             );
         }
 
         // Set new quorum for future proposals
-        _quorumNumeratorHistory.push(clock().toUint32(), newQuorumNumerator.toUint224());
+        _quorumNumeratorHistory.push(SafeCast.toUint32(clock()), SafeCast.toUint224(newQuorumNumerator));
 
         emit QuorumNumeratorUpdated(oldQuorumNumerator, newQuorumNumerator);
     }

contracts/governance/utils/IVotes.sol

@@ -25,13 +25,13 @@ interface IVotes {
 
     /**
      * @dev Returns the amount of votes that `account` had at a specific moment in the past. If the `clock()` is
-     * configured to use block numbers, this will return the value the end of the corresponding block.
+     * configured to use block numbers, this will return the value at the end of the corresponding block.
      */
     function getPastVotes(address account, uint256 timepoint) external view returns (uint256);
 
     /**
      * @dev Returns the total supply of votes available at a specific moment in the past. If the `clock()` is
-     * configured to use block numbers, this will return the value the end of the corresponding block.
+     * configured to use block numbers, this will return the value at the end of the corresponding block.
      *
      * NOTE: This value is the sum of all available votes, which is not necessarily the sum of all delegated votes.
      * Votes that have not been delegated are still part of total supply, even though they would not participate in a

contracts/governance/utils/Votes.sol

@@ -59,7 +59,7 @@ abstract contract Votes is Context, EIP712, IERC5805 {
     // solhint-disable-next-line func-name-mixedcase
     function CLOCK_MODE() public view virtual override returns (string memory) {
         // Check that the clock was not modified
-        require(clock() == block.number);
+        require(clock() == block.number, "Votes: broken clock mode");
         return "mode=blocknumber&from=default";
     }
 
@@ -72,7 +72,7 @@ abstract contract Votes is Context, EIP712, IERC5805 {
 
     /**
      * @dev Returns the amount of votes that `account` had at a specific moment in the past. If the `clock()` is
-     * configured to use block numbers, this will return the value the end of the corresponding block.
+     * configured to use block numbers, this will return the value at the end of the corresponding block.
      *
      * Requirements:
      *
@@ -85,7 +85,7 @@ abstract contract Votes is Context, EIP712, IERC5805 {
 
     /**
      * @dev Returns the total supply of votes available at a specific moment in the past. If the `clock()` is
-     * configured to use block numbers, this will return the value the end of the corresponding block.
+     * configured to use block numbers, this will return the value at the end of the corresponding block.
      *
      * NOTE: This value is the sum of all available votes, which is not necessarily the sum of all delegated votes.
      * Votes that have not been delegated are still part of total supply, even though they would not participate in a

contracts/token/ERC20/extensions/ERC20Votes.sol

@@ -50,7 +50,7 @@ abstract contract ERC20Votes is ERC20Permit, IERC5805 {
     // solhint-disable-next-line func-name-mixedcase
     function CLOCK_MODE() public view virtual override returns (string memory) {
         // Check that the clock was not modified
-        require(clock() == block.number);
+        require(clock() == block.number, "ERC20Votes: broken clock mode");
         return "mode=blocknumber&from=default";
     }
 

contracts/token/ERC721/extensions/ERC721Votes.sol

@@ -34,6 +34,8 @@ abstract contract ERC721Votes is ERC721, Votes {
 
     /**
      * @dev Returns the balance of `account`.
+     *
+     * WARNING: Overriding this function will likely result in incorrect vote tracking.
      */
     function _getVotingUnits(address account) internal view virtual override returns (uint256) {
         return balanceOf(account);

test/governance/Governor.test.js

@@ -70,7 +70,7 @@ contract('Governor', function (accounts) {
         );
       });
 
-      shouldSupportInterfaces(['ERC165', 'ERC1155Receiver', 'Governor', 'GovernorWithParams']);
+      shouldSupportInterfaces(['ERC165', 'ERC1155Receiver', 'Governor', 'GovernorWithParams', 'GovernorCancel']);
       shouldBehaveLikeEIP6372(mode);
 
       it('deployment check', async function () {
@@ -84,7 +84,7 @@ contract('Governor', function (accounts) {
 
       it('nominal workflow', async function () {
         // Before
-        expect(await this.mock.$_proposalProposer(this.proposal.id)).to.be.equal(constants.ZERO_ADDRESS);
+        expect(await this.mock.proposalProposer(this.proposal.id)).to.be.equal(constants.ZERO_ADDRESS);
         expect(await this.mock.hasVoted(this.proposal.id, owner)).to.be.equal(false);
         expect(await this.mock.hasVoted(this.proposal.id, voter1)).to.be.equal(false);
         expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(false);
@@ -149,7 +149,7 @@ contract('Governor', function (accounts) {
         await expectEvent.inTransaction(txExecute.tx, this.receiver, 'MockFunctionCalled');
 
         // After
-        expect(await this.mock.$_proposalProposer(this.proposal.id)).to.be.equal(proposer);
+        expect(await this.mock.proposalProposer(this.proposal.id)).to.be.equal(proposer);
         expect(await this.mock.hasVoted(this.proposal.id, owner)).to.be.equal(false);
         expect(await this.mock.hasVoted(this.proposal.id, voter1)).to.be.equal(true);
         expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(true);

test/utils/introspection/SupportsInterface.behavior.js

@@ -90,6 +90,7 @@ const INTERFACES = {
     'castVoteBySig(uint256,uint8,uint8,bytes32,bytes32)',
     'castVoteWithReasonAndParamsBySig(uint256,uint8,string,bytes,uint8,bytes32,bytes32)',
   ],
+  GovernorCancel: ['proposalProposer(uint256)', 'cancel(address[],uint256[],bytes[],bytes32)'],
   GovernorTimelock: ['timelock()', 'proposalEta(uint256)', 'queue(address[],uint256[],bytes[],bytes32)'],
   ERC2981: ['royaltyInfo(uint256,uint256)'],
 };
@@ -120,7 +121,7 @@ function shouldSupportInterfaces(interfaces = []) {
     it('all interfaces are reported as supported', async function () {
       for (const k of interfaces) {
         const interfaceId = INTERFACE_IDS[k] ?? k;
-        expect(await this.contractUnderTest.supportsInterface(interfaceId)).to.equal(true);
+        expect(await this.contractUnderTest.supportsInterface(interfaceId)).to.equal(true, `does not support ${k}`);
       }
     });
 
@@ -130,7 +131,10 @@ function shouldSupportInterfaces(interfaces = []) {
         if (INTERFACES[k] === undefined) continue;
         for (const fnName of INTERFACES[k]) {
           const fnSig = FN_SIGNATURES[fnName];
-          expect(this.contractUnderTest.abi.filter(fn => fn.signature === fnSig).length).to.equal(1);
+          expect(this.contractUnderTest.abi.filter(fn => fn.signature === fnSig).length).to.equal(
+            1,
+            `did not find ${fnName}`,
+          );
         }
       }
     });