Clanker A-1

In the LpLockerv2.sol contract, the collectRewards() function retrieves liquidity provider rewards, calculates the proportional share of rewards for the creator, interface, and team, and distributes this to each actor. However, if any of these actors is blacklisted on reward tokens that support blacklisting (e.g., USDC), the corresponding token transfer would fail. As a result, none of the actors can collect rewards.

If actors are non-malicious, the team, creator, or interface may update their recipient address and unblock the reward collection. Otherwise, reward collection will be permanently blocked if the actor cannot access the admin address to invoke the recipient update operation or is unwilling to do it for other reasons.

Remediations to Consider

• Update reward collection implementation to use assets pull instead of assets push pattern.

In the LpLockerv2.sol contract, the collectRewards() function calculates the team’s reward based on TEAM_REWARD constant defined in LpLockerv2.sol. On the other hand, MAX_CREATOR_REWARD constant is set in the constructor based on the value of Clanker.MAX_CREATOR_REWARD constant. In case when MAX_CREATOR_REWARD is 80, and the corresponding creator reward is equal to MAX_CREATOR_REWARD, the current implementation in collectRewards() would distribute shares as expected.

However, if MAX_CREATOR_REWARD in Clanker is set to 70 and the corresponding creator reward remains equal to MAX_CREATOR_REWARD (also 70 in this case), the current implementation in collectRewards() would distribute rewards in unexpected amounts. For example, in this specific case, the share of the reward for the team would remain at 20%, but the creator share would be 80% and not 70%, as defined by the MAX_CREATOR_REWARD constant value.

Remediations to Consider

• Add validation to LpLockerv2.sol constructor to verify that the sum of MAX_CREATOR_REWARD and the TEAM_REWARD is 100%.

In the ClankerToken.sol contract, supportsInterface() is implemented in the following way and indicates support for IERC7802, IERC20, and IERC165.

function supportsInterface(bytes4 _interfaceId) public pure returns (bool) {
    return _interfaceId == type(IERC7802).interfaceId
        || _interfaceId == type(IERC20).interfaceId || _interfaceId == type(IERC165).interfaceId;
}

However, as ClankerToken inherits ERC20Votes, it has additional capabilities represented by the IERC5805 (IVotes and IERC6372) interface, which are not properly indicated as supported.

Remediations to Consider

• Consider updating supportsInterface() implementation to indicate IERC5805 support.

Missing events for important state changes

• In Clanker.sol, consider adding events for setDeprecated() and setAdmin() functions.
• In ClankerToken.sol, consider adding events for updateImage() and updateMetadata()

In LpLockerv2.sol, the teamRecipient storage variable contains an address where rewards for the team would be sent. In addition, this address is the default recipient of creator and interface reward shares if they are not set.

Reward assets can be permanently lost if the teamRecipient value is incorrectly set or updated. Therefore, consider adding teamRecipient value validation.

The codebase contains inconsistencies in defining events that treat storage variable updates. Sometimes, only a new value is emitted, e.g., LpLockerv2.TeamRecipientUpdated. On the other hand, in the events such as ClankerVault.MinimumVaultTimeUpdated or ClankerVault.AllocationAdminUpdated, both old and new, values are emitted.

Consider updating event definitions to have more consistent implementation and to facilitate easier off-chain tracking and monitoring integration.

In the Clanker contract, the initialize() function is accessible only by the owner. This function allows the owner to configure references to important underlying components, such as ClankerVault, LpLockerv2, Uniswap V3 Factory, Position Manager, and Swap Router.

The owner may invoke initialize() multiple times, reconfiguring the Clanker system with different underlying system contracts. As a result, the system is not immutable, and due to potential changes, it requires additional user trust.

To minimize required user trust, consider not allowing multiple initialize() invocations or adding a feature to disable multiple invocations when the system becomes stable.