QA Report

[code423n4]

Low & QA report

Relevant parts of the code are marked with @audit tags.

QA

---

QA-01: Add check that newPotentialOwner != currentOwner to ConduitController.sol:transferOwnership()

While there are no negative effects and the currentOwner can complete the entire ownership transfer flow to themselves, the events emitted would be confusing and misleading, impacting metrics / users looking at them.

    function transferOwnership(address conduit, address newPotentialOwner)
        external
        override
    {
        // Ensure the caller is the current owner of the conduit in question.
        _assertCallerIsConduitOwner(conduit);

        // Ensure the new potential owner is not an invalid address.

        // @audit QA - add a check that newPotentialOwner is not the current owner of the conduit
        if (newPotentialOwner == address(0)) {
            revert NewPotentialOwnerIsZeroAddress(conduit);
        }

        // Emit an event indicating that the potential owner has been updated.
        emit PotentialOwnerUpdated(conduit, newPotentialOwner);

        // Set the new potential owner as the potential owner of the conduit.
        _conduits[conduit].potentialOwner = newPotentialOwner;
    }

QA-02: Emit event or return data if accumulator is not "armed" in Executor.sol:_triggerIfArmed()

If the accumulator is not "armed" the internal function simply returns without emitting an event or returning data, both of which can be useful in diagnosing the issue. This does not follow the pattern seen in the rest of the codebase where events are emitted + return data for errors. Would be a good to have.

    function _triggerIfArmed(bytes memory accumulator) internal {
        // Exit if the accumulator is not "armed".
        if (accumulator.length != 64) {
            // @audit Emit event or return data here
            return;
        }

        // Retrieve the current conduit key from the accumulator.
        bytes32 accumulatorConduitKey = _getAccumulatorConduitKey(accumulator);

        // Perform conduit call.
        _trigger(accumulatorConduitKey, accumulator);
    }

[HardlyDifficult]

Grouping with https://github.com/code-423n4/2022-05-opensea-seaport-findings/issues/89

[GalloDaSballo]

QA-02: Emit event or return data if accumulator is not "armed" in Executor.sol:_triggerIfArmed()

Disagree as it's literally the name of the function

89 - timelock

Disagree as you have no way of proving usage / non-usage of timelock, hence the finding is not actionable

[GalloDaSballo]

I think QA-01 is NC because the effect is firing a second event without a change, edited above to reflect that

1NC