SpiritVestingFactory::createSpiritVestingContract overwrites the spiritVestings mapping for the same recipient
Description:
SpiritVestingFactory::createSpiritVestingContract() is as follows:
function createSpiritVestingContract(
address recipient,
uint256 amount,
uint256 cliffAmount,
uint32 cliffDate,
uint32 endDate
) external onlyTreasury returns (address newSpiritVestingContract) {
if (!(cliffAmount < amount)) revert FORBIDDEN();
uint256 vestingDuration = endDate - cliffDate;
uint256 vestingAmount = amount - cliffAmount;
int96 flowRate = int256(vestingAmount / vestingDuration).toInt96();
// Add the remainder to the cliff amount
cliffAmount += vestingAmount - (uint96(flowRate) * vestingDuration);
// Deploy the new SPIRIT Token Vesting contract
newSpiritVestingContract =
address(new SpiritVesting(VESTING_SCHEDULER, SPIRIT, recipient, cliffDate, flowRate, cliffAmount, endDate));
// Maps the recipient address to the new SPIRIT Token Vesting contract
spiritVestings[recipient] = newSpiritVestingContract; //@audit overwrites previous vesting contract if exists
// Transfer the tokens from the treasury to the new vesting contract
SPIRIT.transferFrom(msg.sender, newSpiritVestingContract, amount);
// Emit the events
emit Transfer(address(0), recipient, amount);
emit SpiritVestingCreated(recipient, newSpiritVestingContract);
}Note that creating more than 1 vesting contract for the same recipient overwrites the old now. The only impact is that the SpiritVestingFactory::balanceOf() function will point to the new vesting contract only.
Impact:
Informational, inconsistent behaviour.
Recommended Mitigation:
Consider replacing the mapping with an array.
0xSimao:
Fixed in PR #2.