Prevent creation of rent-paying accounts with non-empty data

[CriesofCarrots]

Problem

A single epoch's worth of rent for an account with data is too little relative to the costs of storing that data.

Summary of Changes

Compare rent-exemption status pre- and post-processing; fail if any end up RentPaying that didn't start that way.

[CriesofCarrots]

Technically, we could make do without the enum, and boil this logic down to a bool: is RentPaying or not. I rather like how explicit and clear the variants make the logic and exceptions, but open to other opinions.

[brooksprumo]

The enum is helpful to me! I always appreciate the added type safety as well.

[jstarry]

I don't think we want to return a TransactionError after processing a transaction. That means it's free to process potentially expensive transactions since fees aren't deducted for tx errors.

[CriesofCarrots]

Yeah, I was pondering adding this TransactionError as a special case in filter_program_errors_and_collect_fee(). But perhaps a better approach would be to pass the rent information into the message processor and evaluate on a per-instruction basis with the other pre/post processing checks. Thoughts?

[jstarry]

I think it's fine to use non-rent-exempt accounts between instructions for passing data around. Would your change prevent that?

[jstarry]

I think that putting the rent changes inside fn collect_accounts_to_store would be nice since it's already we check for rent for newly created accounts. We could modify that to check all written accounts loaded by a transaction.

[jon-chuang]

As for accounts without data, could the network have a hard cap on the number of such accounts (or the i64 diff from time of feature activation) configured for the cluster at a given time?
This can be kept track of in a sysvar (ignoring previous non-rent-exempt len 0 accounts) through the new account creation, and pre and post account check gateways (decrementing the count if a previous non-rent-exempt account goes to 0 lamports or becomes rent exempt).
Sysvar contention can be reduced by having cheap datastructure in bank capturing the diff in each slot and committing to the sysvar at end of every slot.

I think of it as a safety check that would not affect account creation normally, but would come into play if the cluster gets flooded with creation of tiny data len 0 accounts that are not rent-exempt. Tentatively, a cap of 100M of such non-rent-exempt accounts is probably desirable.

If you'd like me to PR this in, I'd be happy to do so.

[CriesofCarrots]

I think that putting the rent changes inside fn collect_accounts_to_store would be nice since it's already we check for rent for newly created accounts. We could modify that to check all written accounts loaded by a transaction.

I don't really like that method for this. To me, this check should be reflected in TransactionExecutionResults, not some separate new thing.

[jstarry]

I don't really like that method for this. To me, this check should be reflected in TransactionExecutionResults, not some separate new thing.

Could you share your reasoning? I still think that fn collect_accounts_to_store is the natural place for this type of check because we already deduct rent for newly created accounts there. Expanding it to apply to all writable accounts loaded b a transaction feels like a lighter change.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.