ERC-20 Native and Token

The token defaults to address(0) (native). While token remains unset, anyone can call deposit() to fund the contract with ETH and, if whitelisted in the merkle tree and within the window, call claim() to withdraw ETH to themselves. Because setToken() is permanently disabled after totalClaimed > 0, a single successful ETH claim irreversibly locks the distribution to native token, preventing the admin from switching to the intended ERC20. An attacker can front-run by depositing ETH and claiming as soon as the window opens if the admin forgets to set the token first.

Require explicit token configuration before startTime or use a sentinel value instead of address(0).

When token_ != token, the functions releasable and vestedAmount revert with ClaimAdmin__ZeroAddress(), which does not represent the true reason for the error.

Change the revert name to match the cause of the error.

Function releasable risks underflow if an admin were to change the cliff with setCliff such that vestedAmount(token_, uint64(block.timestamp)) becomes zero and totalClaimed is positive. This would block integrators from viewing the true amount of releasable funds.

Do not move the cliff after claims have been made.

Fee-on-transfer and rebasing tokens are not currently supported by the system. For example, Claim._claim delivers the exact _amount. If the configured token is fee-on-transfer / rebasing / otherwise non-standard, the hook still credits claimed and totalClaimed with the full _amount even though the user receives less (or more).

Restrict tokens to plain ERC-20/native asset.

ERC20NativeClaim.claim allows a claim amount of zero, hence the event Claimed could be emitted repeatedly as _claimChecks only validates that if (claimed[msg.sender] != 0).

Either block zero claim amounts or clearly document this behavior.

The deposit functions of the ClaimAdmin contract are callable by everyone but are not pausable.

Consider to add whenNotPaused to all external functions to follow best practices.

If the admin withdraws tokens, the live balance shrinks. The recomputed totalReleased drops as well, and for users who already claimed based on the previous total allocation the new userTotalReleased can fall below their historical claimed amount, which makes _toClaim revert with BaseVesting__AllocationDecreased.

Also note that with direct contract donations or whenever contract balance exceeds the total allocated, the contract immediately treats the higher balance as though it had been part of the original allocation. The linear curve _release therefore jumps up, so every user can instantly claim their proportional share of the donation instead of the donation vesting over the remaining duration. Users will also pull more than their recorded allocation, and total claims will exceed total allocation/total snapshotted.

Use a fixed ledger of “total allocated” tokens that is only ever updated by explicit funding events instead of reliance on .balance or balanceOf. Then _toClaim will not ignore the passed parameters.

setStartTime and setEndTime remain callable by the admin after launch. Because every claim runs _checkStartTime() / _checkEndTime(), pushing startTime forward or pulling endTime back once users have begun claiming immediately bricks the flow for everyone else.

There is no guard comparable to totalClaimed > 0, so a routine timetable adjustment moves the eligible window outside the current block timestamp and all claim calls revert

Consider preventing start and end time updates after claims begun.

The admin may lower cliffReleasePercentage at any time. After claims have started, the new curve in _release pushes the expected total release at the current timestamp below what has already been paid out. When _toClaim recomputes each user’s, userTotalReleased becomes smaller than claimed[msg.sender] so every subsequent claim reverts with BaseVesting__AllocationDecreased. This permanently DoSes all claimants.

Forbid percentage decreases after totalClaimed > 0

Updating allocations after any user has claimed can also DoS existing claimants. Function updateAllocations adjusts totalAllocated, which is later used as the denominator in _toClaim. If an admin onboards a new wallet or increases some other wallet once another user has already claimed, the recalculated share userTotalReleased = userAmount * totalReleased / totalAmount shrinks for the existing claimer, making it less than claimed[msg.sender] and triggering BaseVesting__AllocationDecreased on every future claim. In practice, a single late allocation can freeze all earlier participants.

Furthermore, any allocation decreases after other users have already claimed risk an ERC20InsufficientBalance revert since old totalClaimed history is maintained even if the allocations have changed or the admin withdrew tokens.

Disallow allocation changes once totalClaimed > 0 or vesting begins.

Raising startTime once vesting has begun makes _release recompute with a later start, decreasing totalReleased. The next claim hits the same AllocationDecreased revert, freezing everyone until the original start time is restored.

Prevent startTime from moving forward once vesting begins.

If an admin shortens endTime below the current cliff, the subtraction in linearDuration underflows: endTime - cliff;

Consequently, any _release function call reverts until the admin fixes the timestamps.

Modify setEndTime to ensure the new endTime doesn't fall below the current cliff.

Updating the Merkle root after anyone has claimed reruns _toClaim with the new (snapshot,total) pair. If the refreshed leaf lowers either value, userTotalReleased drops below claimed[msg.sender], so every subsequent claim reverts with BaseVesting__AllocationDecreased. This bricks all prior claimants even when the admin is just trying to admit new wallets or fix a typo in a snapshot amount.

Lock the root once totalClaimed > 0 or clearly document this risk.

Claims always transfer to msg.sender. If the whitelisted address is a smart contract without a payable receive/fallback, native token transfers will revert and the user cannot ever claim. There is no option to specify an alternative recipient address.

Consider adding a claim variant to allow a different recipient. Otherwise, clearly document this behavior.

The depositedRewards which tracks deposits is never used and is not an accurate representation of how much funds are available for vesting. Also note that the passed _token does not have to match the token being vested out.

Clearly document this behavior.

The _toClaim function relies on balance + totalClaimed for calculation rather than totalAllocated. Consequently, users can pull more funds out than their intended allocation.

Clearly document this is intended behavior.

Each Merkle leaf encodes both the user’s snapshotAmount and a caller-supplied totalSnapshoted. During claim, the contract takes that same pair and computes the payout as snapshotAmount / totalSnapshoted. If whoever prepares the snapshot accidentally (or intentionally) gives one address a smaller totalSnapshoted than everyone else, the proof will still verify and that address will receive a disproportionately large share of the vested tokens.

Ensure all leaves have the same totalSnapshotted.

In function _vestingSchedule, when startTime equals the timestamp, _timePoints() returns -1 (requires calculation) but _release() simply calculates: (totalAllocation * 0) / duration() = 0

This case could be shortcutted in _timePoints() .

If timestamp <= startTime return 0 timePoints.

The updateAllocations() function validates that each user's allocation is non-negative immediately after each iteration, rather than after all updates are processed. Consequently, multi-step allocation adjustments where a user's allocation temporarily goes negative before being corrected to be positive will revert.

Clearly document this behavior.

The initializer does not validate that _admin is non-zero. If initialized with address(0), no one will hold DEFAULT_ADMIN_ROLE, which disables all core functionality.

Add a zero-address check for _admin and revert with ClaimAdmin__ZeroAddress() if _admin == address(0).

The IERC20 interface is imported in the SnapshotVesting.sol file but not used.

Consider to remove unused code.

Function updateAllocations accepts wallet = address(0) in the Allocation struct, which will strand a portion of totalAllocated.

Ensure allocations are appropriately passed by the DEFAULT_ADMIN_ROLE or add contract-level validation.

Function _checkCliffReleasePercentage forbids setting the percentage of the cliff allocation unlock to 100%. If the product requirement includes “full unlock at cliff", it will not be supported.

Consider if this is expected behavior and clearly document if so. ERC20NativeClaim can be used for 100% claims but a merkle root needs to be created.

The calculation uint256 userTotalReleased = (userAmount * totalReleased) / totalAmount; can potentially overflow when performing (userAmount * totalReleased) on large values before being able to reach the division.

Consider using mulDiv or bound allocation amounts appropriately.

Function _grantRole ignores the boolean returned by EnumerableSet.add, so re‑granting a role that an account already holds still emits RoleGranted. Similarly, _revokeRole has the same issue on removal — a failed remove (account never had the role) still emits RoleRevoked. This may confuse off-chain indexers relying on events to reflect actual membership changes.

Clearly document this behavior.

Both ERC7201 namespaced storage is used and plain storage is used, so care must be taken with upgrades.

Consider sticking with just ERC7201 namespaced storage.

The function vestedAmount reverts when token_ != $.setup.token with ClaimAdmin__ZeroAddress(), which does not represent the true reason for the error.

Change the revert name to BaseVesting__TokenMismatch.

As noted in , a _token that is not the token being vested can be deposited by users which will credit towards depositedRewards and emit a Deposited event. This may lead to issues where a user deposits the valid vesting token, the admin changes the vesting token, but users continue to deposit the old vesting token.

Consider restricting function deposit(address _token, uint256 _amount) so that the _token must match the $.setup.token.

It is possible to set the startTime after the cliff with the setStartTime function which does not make sense.

Consider to revert in that case.

Currently the ClaimAdmin uses Pausable from solidstate instead of PausableUpgradeable from OZ which is inconsistent with the other inherited contracts: AccessControlEnumerableUpgradeable and UUPSUpgradeable.

Consider using OZ's PausableUpgradeable or clearly document this.

BaseVesting floors both the stream and per-user share. Consequently, tiny allocations therefore vest to zero and never cross the BaseVesting__ZeroReleased guard, leaving some users unable to claim and producing stranded dust.

Clearly document this behavior.

Function setMerkleRoot validates that the _merkleRoot is not empty bytes with if (_merkleRoot == bytes32(0)) revert MerkleWhitelist__ZeroMerkleRoot();. However, _setMerkleRoot performs the exact same check: if (_merkleRoot == bytes32(0)) return;

Consider removing the redundant check.

"The token to be claimed - address(0) for native token, otherwise ERC20." is invalid since the native token is represented by the NATIVE_SENTINEL_ADDR = 0x000000000000000000000000000000000000dEaD

Update the comment to appropriately reflect the native token.