Submitted by PetarTolev, also found by joaovwfreire, bearonbike, Drynooo, and MrPotatoMagic
https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/MigrationManager.sol#L264 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/MigrationManager.sol#L285 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/MigrationManager.sol#L335-L347 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L293 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L345-L379
LockManager.lockOnBehalf is called when the user has MunchableNFT which should be migrated.
The LockManager._lock function called by the lockOnBehalf handles the actual locking process. This function does several important steps:
The remainder is an important part of this process. It represents any leftover amount that wasnt enough to mint an additional NFT. Normally, this remainder is added to the next lock amount, allowing users to efficiently utilize all their tokens over time.
However, when _lock is called from MigrationManager, it improperly resets the remainder to 0 (step 5). This happens because the function doesnt differentiate whether its being called from the migration process or a standard lock request. As a result, users end up receiving fewer MunchableNFTs than they should because the small leftover amounts are discarded instead of being carried over to the next lock.
The users funds wont be used efficiently, resulting in fewer received MunchableNFTs. Also, the user wont be able to unlock these unused funds and relock them to be used. This happens because the MigrationManager resets the unlock time, forcing the user to wait an extra lockDuration.
When MigrationManager.migrateAllNFTs or MigrationManager.migratePurchasedNFTs is called
Do not reset the remainder storage variable when the _lock function is called from the MigrationManager. Instead, ensure the remainder is correctly accumulated and carried over, allowing users to receive the correct amount of MunchableNFTs based on their total locked amount. This can be done by adding a condition to check the caller and handle the remainder accordingly.
Context
0xsomeone (judge) commented:
0xinsanity (Munchables) commented:
0xsomeone (judge) commented:
Note: For full discussion, see here.
