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

NoteKeeper.sol: The matured notes are not getting deleted #329

Open
foudufafa opened this issue Aug 10, 2022 · 0 comments
Open

NoteKeeper.sol: The matured notes are not getting deleted #329

foudufafa opened this issue Aug 10, 2022 · 0 comments

Comments

@foudufafa
Copy link

In NoteKeeper.sol, when addNote() is called (use when for bond deposit()), a new note is created and pushed into an array in storage (notes).

But, that note, once fully vested, never gets deleted. Maybe this is on purpose for historical data reasons that is any user can just walk back all notes it had ever purchased.

However, this is a bit of a problem with indexesFor() which iterates over all notes to find out how many are not vested yet. That function is view so no gas cost except that it is used in redeemAll() that is not a view function.

Which means that the more a user bonds, the more the redeemAll() becomes expensive in gas to the point I believe to be just unusable.

I made a quick experiment and copied the indexesFor() function and struct Note. Made a non-view function that uses indexesFor() and with only 100 notes in there, the gas cost to call such function jumps to 277,688. At 500 notes, the gas cost reaches 1,309,438. It is not unthinkable to see a single user overtime to reach 500 notes or even go above 1000 notes which then becomes very large amount of gas.

I noticed this in the code for the redeemAll() function:

@dev                if possible, query indexesFor() off-chain and input in redeem() to save gas

However, it appears that the dapp (frontend) is using redeemAll() so without their knowledge, users can face large amount of gas fee.

Either the dapp needs to be changed or maybe it would be wise to keep the redeemed notes into a separate storage so to avoid the redeemAll() function to just grow and grow in gas overtime?

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

No branches or pull requests

1 participant