ANIME Claimer

When the AnimeClaimer is initially created, the storage variable paused is set to true within the constructor: _getAnimeClaimerStorage().paused = true;

For the AnimeClaimer to become unpaused, it is expected the owner properly configures the contract's other parameters such as dailyTotalWithdrawnLimit and then calls function setPaused(false). The issue is that when attempting to unpause the contract, the code also checks if (!readyForClaim()).

Function readyForClaim checks whether the contract is paused, and if so, returns false: if ($.paused) return false; Since the initial state is paused, if (!readyForClaim()) will be entered. Therefore when the owner or manager attempts to unpause the contract, it reverts with revert NotReadyForClaim(); Ultimately the entire contract's functionality is DoS'd as it is not possible to unpause the contract.

When unpausing, do not revert if the contract is currently paused. Contract configuration parameters can be verified to be set outside of readyForClaim to avoid if ($.paused) return false;

The _claimBatch function prevents withdrawals once the daily withdrawal limit has been met: if ($.dailyTotalWithdrawnLimit <= $.dailyTotalWithdrawn) revert ExceedDailyTotalWithdrawnLimit();

The issue is that the dailyTotalWithdrawn is only reset within function _incrementDailyTotalWithdrawn, but _incrementDailyTotalWithdrawn is only called when a user withdraws a non-zero amount: if (totalWithdrawn != 0)

This leads to a total DoS once a user withdraws to the daily limit, as no more withdraws are possible and the daily limit cannot be reset, even if it is a new day.

Update the daily limit regardless of whether a withdraw has occurred. Consider checking if the block timestamp is a new day and reset the lastWithdrawnDay or simply call _incrementDailyTotalWithdrawn(0) at the beginning of claimBatch.

In order for messaging to work, configurations should be handled properly. According to LayerZero documentation:

For a configuration to be considered correct, the Send Library configurations on Chain A must match Chain B's Receive Library configurations for filtering messages.

However as we can see from the configuration file, while the Send Library for source chain is configured, Receive Library for destination chain is not configured, hence the default values will be used for it in Arbitrum.

While the confirmations configured for Ethereum is “8”, the default confirmations in Arbitrum is configured as “15”, and this will lead to “Block Confirmation Mismatch” explained as:

Messages will be blocked until either the sending OApp has increased the outbound block confirmations, or the receiving OApp decreases the inbound block confirmation threshold.

in LayerZero documentation.

Also default DVN's for Arbitrum are ‘Layerzero Labs’ and ‘Google Cloud’, hence other DVN's configured for source chain (’Nethermind’, ‘Horizen’) won't be utilized as expected and they will just consume fee in source chain.

Implement a proper configuration for destination chain's Receiver Library to match the Sender’s. While doing so following documentation from LayerZero can help: https://docs.layerzero.network/v2/developers/evm/create-lz-oapp/configuring-pathways Be aware of following mismatch scenarios described and be sure Configurations in both end Matches: https://docs.layerzero.network/v2/developers/evm/protocol-gas-settings/default-config#debugging-configurations

For example, the following receiveConfig blob can be used for the arbitrum → ethereum connection:

{
            from: arbitrumContract,
            to: ethereumContract,
            config: {
	            receiveConfig: {
	                ulnConfig: {
	                    // SendUlnConfig and ReceiveUlnConfig confirmations must match
	                    confirmations: BigInt(8),
	                    requiredDVNs: [
	                        // Layerzero, Arbitrum
	                        '0x2f55c492897526677c5b68fb199ea31e2c126416',
	                        // Google, Arbitrum
	                        '0xd56e4eab23cb81f43168f9f45211eb027b9ac7cc',
	                        // Nethermind, Arbitrum
	                        '0xa7b5189bca84cd304d8553977c7c614329750d99',
	                        // Horizen, Arbitrum
	                        '0x19670df5e16bea2ba9b9e68b48c054c5baea06b8',
	                    ],
	                    optionalDVNs: [],
	                    optionalDVNThreshold: 0,
	                },
	            },
	         },
}

if (_SANCTIONS_LIST.isSanctioned(msg.sender) || _SANCTIONS_LIST.isSanctioned(to)) is intended to prevent sanctioned addresses from using the AnimeClaimer. However, this protection can be bypassed simply with a sanctioned NFT owner delegating to a non-sanctioned address. That non-sanctioned address can then claim without issues.

Consider adding a sanctions list validation if the msg.sender is a delegate of the NFT owner.

Keep in mind that users can always transfer the NFT to another address, enabling them to claim the allocation.

The variable dailyTotalWithdrawnLimit can vary between chains. And indeed according to the runbook, the L1 limit is 1e24, while the L2 limit is currently 1.5e23. However, this discrepancy poses a problem where a message sent from the source chain (L1) may not be received in the destination chain (L2). This issue arises when the allocation sent is larger than L2's dailyTotalWithdrawnLimit but smaller than L1's dailyTotalWithdrawnLimit.

Since it is smaller than the L1 limit, it can be sent from the source chain, but because it exceeds the L2 limit, the message becomes stuck at the endpoint and cannot be received by the AnimeClaimer contract in L2.

Make sure the dailyTotalWithdrawnLimit value for L2 is greater or equal to the value set in L1 contract.

AnimeClaimer.readyForClaim is a helper function that ensures the contract is correctly set up to initiate user claims.

However, the function is missing crucial validations:

Ethereum

• OApp correctly wired (peers, endpoint, delegate)

Arbitrum

• OApp correctly wired (peers, endpoint, delegate)
• Existing ANIME token balance

Consider adding the validation above to ensure both contracts are ready to start the claiming process and avoid failed transactions and messages.

There are several protocols which offer NFT flashloans following EIP 6682, which grant ownership of an NFT for a single transaction from a pool of NFTs.

These protocols will allow arbitrary users to flash loan their NFT and claim the corresponding ANIME tokens from the NFTs vest in the merkle root even though they are not the official owners of the NFT.

Additionally, notice that fixed term NFT loans allow the loaner to claim any outstanding vest for the NFT that has not yet been claimed by the owner.

Be aware of this edge case for flash loan providers of NFTs which will receive an ANIME vest. If this is deemed to be an issue those providers can be warned ahead of time to withdraw their NFT from flash loan service before the claimer becomes active.

if (paused == false) can just be rewritten to !paused within function setPaused to be consistent with other boolean operations.

Replace it with !paused.

If the contract was paused on Arbitrum but not on ETH, a user could still send a claim request which would increment the total withdrawn on ETH, yet the withdraw would revert on the destination chain with revert Paused(); Consequently, the to address will not receive their tokens and the request will have to be retried at the Endpoint after an unpause. This may be unexpected to users who had a successful claimBatch call.

Clearly document this behavior to users.

The manager variable is not set for Arbitrum and uuidSigner is not set for ETH within AnimeClaimerSetup but other key parameters are. However this is not a major issue since setters are available within the AnimeClaimer contract.

Be aware of this and considering setting those variables within the script. Also consider checking if those parameters should be set within readyForClaim.

The setUUIDSigner function does not limit the signer address and allows zero as a valid address. If the owner changes the signer address to zero by mistake, it will result in every signature check passing in _claimBatch, and the consequences can be catastrophic even if the mistake is resolved promptly upon discovery.

If the provided signer address is zero, the setUUIDSigner function should revert.

According to the runbook, if the intention is to claim instantly, one can set the start and end times to the past, resulting in the whole allocation amount being returned optimistically by the vested function. However, if the start time is set to be greater than the end time, an underflow will occur in the duration calculation before comparing the start and end times with block.timestamp to return the total allocation, leading to a direct revert and preventing that claim.

Ensure the end time greater than the start time in the root creation process.

When claiming for a collector level claim the tokenId should not be assigned to a nonzero value.

This will be validated in the merkle proof, however out of an abundance of caution it may be prudent to add explicit validations at the contract level that the tokenId is 0 when isForCollector is true.

Consider adding validation that enforces the tokenId to be 0 when isForCollector is true.

The _requestReleasePayload function assigns the msg.sender as the refund receiver for a native refund.

As a result, Smart Contracts which receive a collector allocation, do not have the functionality to delegate, and do not have a receive function cannot claim their vest.

This scenario is unlikely to occur, especially for a contract which is able to claim from the claimer. This finding simply serves as a warning for this case.

When claiming allocation for Elementals, the uuid is defined and a signature verification is needed using the uuidSigner.

However, the first time the user claims allocation for an Elemental, besides checking the signature, the uuid is stored in a mapping: $.nftToUUID[c.nft][c.tokenId] = c.uuid; Therefore, the consecutive claims do not need additional signature verification, as the mapping will ensure its a valid claim: if (c.uuid != $.nftToUUID[c.nft][c.tokenId]) revert InvalidUuid();

Consider validating the signature only when the mapping is updated with the new uuid.

The owner of AnimeClaimer can set the vesting root for the merkle distribution.

However, the function does not prevent the reinitialization of the vesting root. Due to the fact that the snapshot is only taken once, there is no need to leave the option for modifying the value again, as it will lead to accounting issues.

Only allow the vesting root to be set once, or include it as a param in the constructor.

The AnimeClaimer contract defines a struct for all storage variables. Although there are some getter functions to read this variables, there are some still missing that will provide a better user experience:

• lastWithdrawnDay
• dailyTotalWithdrawn

Adding these getters will allow the frontend to correctly display the current contract stats and limits.

The current DVN configuration uses 4 required DVNs, however if there is an outage in any of the 4 DVNs this can cause a backlog of claim messages to build up and eventually be executed all in a single day, exceeding the daily limit.

This can result in DoS'd claims for other users for a period of time.

Consider moving to a setup of optional DVNs whereby for example 4/5 or 3/4 DVNs are required to verify a message before execution. This way no single DVN can prevent a claim message from being executed.

The AnimeClaimer contract does not implement a getter function for the lastWithdrawnDay value.

Consider whether this variable should be exposed through it's own getter function similarly to other storage variables.

In the comment for the LayerZero receive configurations on Arbitrum it is mentioned that the receive library configured is ReceiveUln302 for Ethereum.

However the address provided is correctly the ReceiveUln302 library address on Arbitrum.

Correct the comment to ReceiveUln302 for Arbitrum.

The AnimeClaimerL1Setup performs the setup required for AnimeClaimer in Ethereum.

There is an inconsistency in the function name for setUUIDSigner as the script uses setUuidSigner, causing a revert when its executed.

Consider changing setUuidSigner for the correct function name in the contract, setUUIDSigner.

The readyForClaim function signals when the contract has been properly set up for claiming. However, the paused state is not included in this function, or in the fullyReadyForClaim function.

Users may become confused when reading the returned values of these functions, as even if both return true, the contract is not actually ready for claiming until the owner unpauses it.

Furthermore, the UI will need to combine both the readyForClaim and isPaused getters to enable claiming.

Consider adding the paused state to the fullyReadyForClaim function, and return false if the contract is paused.

Although M-02 was fixed in deployment, the setters in the contract do not validate that the L2 withdrawal limit is larger than the L1 limit.

Consider adding validation in setDailyTotalWithdrawnLimit on the limit parameter depending on the chain id.