Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

immeas - gas costs for minting rollovers is inflated by lost positions #284

Closed
sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 27, 2023

immeas

medium

gas costs for minting rollovers is inflated by lost positions

Summary

Either over time or by malicious actions the rolloverQueue can get filled with items that aren't rolled over. They cost gas but doesn't count to the relayerFee payed to the minter.

Vulnerability Detail

When minting rollovers the lost positions are not rolled over:
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L396-L406

This is because when the vault loses, the funds are moved and there might not be funds to pay relayerFee. Hence only winning epochs are rolled over and counted to the total relayerFee payed to the minter:
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L446-L447

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L455-L456

First, even without malicious intent, the gas cost of this is going to grow because users are simply going to ignore to pay the gas to delist from positions they've lost unless they want to participate again.

This is further exacerbated by that the index isn't updated for "lost" positions:
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L453

So it might not be possible to do it piece wise either.

On top of this imagine a malicious actor who uses multiple accounts and enlists positions for relayerFee + 1 on both vaults. The winning vault will get the funds and they can be delisted and withdrawn.
In the losing vault all the positions can be left and increase the gas cost of minting rollovers

This can ofc be compensated by increasing the relayerFee so it becomes worth again but this is at the expense of regular users.

Impact

The gas cost of minting rollovers can be inflated so that it is either not worth or it is very expensive to do rollovers and queued deposits.

Code Snippet

See above

Tool used

Manual Review

Recommendation

I recommend a redesign of the relayerFee solution:

Pay the relayerFee to the next epoch upfront, so you only queue assets - relayerFee to the next epoch. On delist the fee can be returned.

When minting the rollovers you delist lost positions (since they anyway will have to deposit again and re-enlist to active the rollover again). Since the relayerFee is paid upfront the relayer can get paid for all positions either rolled over or delisted.

The important and complicated part is making sure the relayerFee is kept in the vault on loss and not sent to the counter party vault.

Duplicate of #172

@github-actions github-actions bot closed this as completed Apr 3, 2023
@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 3, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant