Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider adding a vault limit for accounts #133

Closed
0x-r4bbit opened this issue Feb 20, 2025 · 1 comment
Closed

Consider adding a vault limit for accounts #133

0x-r4bbit opened this issue Feb 20, 2025 · 1 comment

Comments

@0x-r4bbit
Copy link
Collaborator

There's a couple of places where the protocol has to iterate over account vaults:

function mpBalanceOfAccount(address account) external view returns (uint256) {
address[] memory accountVaults = vaults[account];
uint256 accountTotalMP = 0;
for (uint256 i = 0; i < accountVaults.length; i++) {
VaultData storage vault = vaultData[accountVaults[i]];
accountTotalMP += vault.mpAccrued + _getVaultPendingMP(vault);
}
return accountTotalMP;
}

function getAccountTotalMaxMP(address account) external view returns (uint256) {
address[] memory accountVaults = vaults[account];
uint256 accountTotalMaxMP = 0;
for (uint256 i = 0; i < accountVaults.length; i++) {
accountTotalMaxMP += vaultData[accountVaults[i]].maxMP;
}
return accountTotalMaxMP;
}

function getAccountTotalStakedBalance(address account) external view returns (uint256) {
address[] memory accountVaults = vaults[account];
uint256 accountTotalStake = 0;
for (uint256 i = 0; i < accountVaults.length; i++) {
accountTotalStake += vaultData[accountVaults[i]].stakedBalance;
}
return accountTotalStake;
}

function rewardsBalanceOfAccount(address account) external view returns (uint256) {
address[] memory accountVaults = vaults[account];
uint256 accountTotalRewards = 0;
for (uint256 i = 0; i < accountVaults.length; i++) {
accountTotalRewards += rewardsBalanceOf(accountVaults[i]);
}
return accountTotalRewards;
}

In theory a malicious account can create many vaults which could cause these functions to run out of gas (yes, gas limits exist for view functions on third party node providers).

Accounts would likely only harm themselves (in the worst case they wouldn't be able to retrieve their balances), but we should probably still do whatever makes most sense.

Creating vaults has become cheap and easy ever since we moved to proxy clones. While it's unlikely that users will create hundreds of vaults, it's still possible.

We can avoid this by simply setting a limit for how many vaults an account can create.

@0x-r4bbit
Copy link
Collaborator Author

Discussed this further offline with @gravityblast
Adding a limit adds some additional complexity with regards to vault migration because one could reach the vault limit and be unable to create new vaults for migration (which might be necessary after an upgrade).

This is solvable by deregistering an old vault after migrating.

Anyways, we've concluded that it's not worth it given the likelihood of this issue.

If this does become an issue in the future we can easily fix this with an upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

1 participant