hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

Constant `DEPOSIT_CAP` should not be hardcoded #15

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xee679aac7efed8d163ae50ab1c3c1559a73fe8b045b0e7db76c0b99ffd61a35a Severity: low

Description: Description

DEPOSIT_CAP should not be a constant value, as some token might easily overpass the cap, for example for a low price token.

Instead this DEPOSIT_CAP should be defined by the owner on initialization.

For example, if the token is PEPE, this will easily filled up as PEPE token is large amount (with same 18 decimal)

File: FM_Rebasing_v1.sol
080:     uint internal constant DEPOSIT_CAP = 100_000_000e18;
...
162:     function _deposit(address from, address to, uint amount) internal {
163:         // Depositing from itself with its own balance would mint tokens without increasing underlying balance.
164:         if (from == address(this)) {
165:             revert Module__FundingManager__CannotSelfDeposit();
166:         }
167: 
168:         if ((amount + token().balanceOf(address(this))) > DEPOSIT_CAP) {
169:             revert Module__FundingManager__DepositCapReached();
170:         }
171: 
172:         _mint(to, amount);
173: 
174:         token().safeTransferFrom(from, address(this), amount);
175: 
176:         emit Deposit(from, to, amount);
177:     }

Recommendation

Consider to add a custom DEPOSIT_CAP value when initializing

PlamenTSV commented 2 months ago

Upgradability deals with this issue.

chainNue commented 2 months ago

Thank you for the comment, will wait for sponsor review.

Upgrading the contract might overkill and affecting others unrelated orchestrator module. Instead of upgrading, it's better to just assign in init function for a custom DEPOSIT_CAP, instead of hardcoding 100_000_000e18.

0xmahdirostami commented 2 months ago

@PlamenTSV thank you, but i think the issue is valid.

marvinkruse commented 1 month ago

We do see the argument that low-value tokens (like meme coins) or high decimal tokens will lead to a (speaking from a value perspective) low cap for the funding manager. We do not see this as a problem though, as upgradeability can fix this and it was intentionally designed this way.