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?
In
NoteKeeper.sol, whenaddNote()is called (use when for bonddeposit()), 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 isviewso no gas cost except that it is used inredeemAll()that is not aviewfunction.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 andstruct Note. Made a non-view function that usesindexesFor()and with only 100 notes in there, the gas cost to call such function jumps to277,688. At 500 notes, the gas cost reaches1,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: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?