solana: Rent timing is not consistant for writable/read-only access

Problem

Maybe to avoid write lock to accounts, rent collection is currently only done at post transaction. This causes some confusion and odd behavior, specifically https://github.com/solana-labs/solana/issues/7413#issuecomment-615136356 and https://github.com/solana-labs/solana/issues/7413#issuecomment-594627347.

Proposed Solution

Always, apply rent collection on load from AccountsDB to Bank. There is no exception. This solves varying view of program execution and cli/rpc, as well. So, rent is collected at both at pre-transaction and post-transaction.

Come to think of it, I think there is no problem for mutating read-only accounts by maintaining the list of the actually rent collected read-only accounts and writable accounts touched by failed transaction in a bank and flush those account updates immeditely before at Bank::freeze() like with the eager rent collection account updates and by consistently applying rent collection on every access accounts before passing them into programs because rent collection is deterministic and idempotent.

Also, after post-transaction rent collection, if the balance goes below 0, abort the transaction itself with new TransactionError like InsufficientFundsForRent (renamed from NotWritableByRent) or similar, instead of silently writing Account::default() into the AccountDB.

related #7413, #10088.

found via #10348

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 17 (15 by maintainers)

Most upvoted comments

Yeah, seems complicated and I don’t think this was the right place to bring up the conversation. Sorry for detracting the conversation!

No problem! Thanks for helping discussions to go in deep! 😃

So now the only way that InsufficientFundsForRent can happen during a transaction instruction is when adding funds to a new account. I think that’s reasonable and not much of a footgun, thoughts?

Yeah, I agree. 😃

@jackcmay FYI, I’ve changed some cases. How does it look now for you?

If there is no remaining topics to discuss, I’ll leave this issue open as-is to be implemented in mid-term (I hope…). Thanks for inputs!