Hook

In the registerDerivative function a maxMintingFee parameter exists to limit the maximum fees that a user must pay when invoking the function.

However the derivative registration can pull from multiple licenseTermsIds which may have differing fee currencies, making the shared maximum entirely ineffective for licenseTermsIds which would use several currencies which may have different decimals or wildly different prices.

Consider allowing the user to specify a list of maxMintingFees per licenseTermsId to match whatever unique currency each licenseTermsId could have.

Story protocol supports EIP 7702 which allows the whitelist logic to be opened up to non-whitelisted callers in the LicenseCallerWhitelistHook.

EIP-7702 allows EOAs to delegate their execution capabilities to smart contracts. For example, if Bob is an approved caller, he can delegate his EOA to a simple proxy contract that exposes the registerDerivative and mintLicenseTokens functionality to any caller:

contract BobEOA {
    LicensingModule licensingModule;

    function registerDerivative(...) external {
        licensingModule.registerDerivative(…);
    }

    function mintLicenseTokens(...) external {
        licensingModule.mintLicenseTokens(…);
    }
}

The whitelist validation key is keyed off of the ipOwner, licensorIpId, licenseTemplate, licenseTermsId, and minter (caller), thus a caller who is whitelisted for some combination of those can open up the whitelist to any caller for that same combination.

Monitor whitelisted EOAs and consider removing them if they delegate to proxy contracts. Stay up to date with EIP-7702 developments, particularly proposals for delegate runtime introspection (discussion link)[https://ethereum-magicians.org/t/transient-eip-7702-delegate-runtime-introspection-enabling-safer-smarter-wallet-innovation/24050], which may allow safer detection and handling of such cases.

The documentation for the calculateMintingFee function suggests that The hook should revert if the minting fee calculation is not allowed. However there is no whitelist validation performed, and thus the calculateMintingFee implementation does not abide by this documented behavior.

Consider if _checkWhitelist should be added to the beginning of the calculateMintingFee function, or if the documentation should be updated.

In the LicensingModule there are two functions which allow for the registry of derivative child ip's, the first being registerDerivative and the second being registerDerivativeWithLicenseTokens.

registerDerivative will check the whitelist status by invoking the beforeRegisterDerivative function in it's flow. However the registerDerivativeWithLicenseTokens does not invoke the beforeRegisterDerivative function on the hook.

As a result, the whitelist status is not verified at the time of derivative registration in this second function. An account may have been whitelisted to initially mint these licenseTokenIds, however the whitelist status could have been revoked fro the account with the removeFromWhitelist function before the derivatives are registered with those tokens using the registerDerivativeWithLicenseTokens function.

Furthermore, the whitelist entries are invalidated every time the owner of the licensorIpId changes, the new owner may not expect anyone to be able to mint or register derivatives until they explicitly whitelist them. However as described, existing licenseTokenIds holders can still register derivatives.

Consider introducing a new hook to be called at the time of registerDerivativeWithLicenseTokens invocation, or invoke the same beforeRegisterDerivative hook in this secondary derivative registration function.

In case the LicenseToken is transferable the whitelist can be bypassed in the following way:

• Whitelisted user mints tokens
• Whitelisted user sells / transfers them to not whitelisted users

Consider to prevent transfers of LicenseTokens if they use the whitelist hook or to add the whitelist hook to the transfer flow too.

Otherwise consider to document this behavior so that IP owners are aware.

In the _payMintingFee flow if the mintingFeeByHook value is less than the royaltyInfo.mintingFeeByLicense * amount result then the transaction is reverted. However in the predictMintingLicenseFee function there is no accounting for such a minimum, and if the hook fee result is present it is always returned with no indication of if it would be less than the mintingFeeByLicense requirement leading to a revert.

Consider if the minimum mintingFeeByLicense * amount should be reflected in the predictMintingLicenseFee in some way. Either by a revert, a sentinel failure value, or reporting back the mintingFeeByLicense * amount value if it is greater than the hook fee.

The same two lines of code to calculate the whitelist key are written out four times in the contract:

address ipOwner = IIPAccount(payable(licensorIpId)).owner(); bytes32 key = keccak256(abi.encodePacked(ipOwner, licensorIpId, licenseTemplate, licenseTermsId, minter));

Consider to move the code into a internal function.

The constructor of the LicenseCallerWhitelistHook does not perform a zero address check for the given licenseRegistry.

There are also no zero address checks for the parameters of the addToWhitelist function.

Consider to add a zero address checks to follow best practices.

In the LicenseCallerWhitelistHook documentation for the whitelist mapping, the comment purports that the keys are based on: keccak256(licensorIpId, licenseTemplate, licenseTermsId, minterAddress).

However this is not the case, instead the key is based on five entries instead of four, like so: keccak256(abi.encodePacked(ipOwner, licensorIpId, licenseTemplate, licenseTermsId, minter)).

Correct the comment to reflect that the licensor ipOwner address is the first item in the key.

If the owner of an IP changes than all whitelist entries are invalided. But if the old owner receives the IP back later on the same whitelist entries are valid again.

This behavior may be unexpected for users.

Be aware and consider to document this behavior.

The _payMintingFee does now return the currencyToken but does not include this return value in it's NatSpec.

Consider to add the currencyToken to the NatSpec.