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

Prevent rent-paying account creation #22292

Conversation

CriesofCarrots
Copy link
Contributor

@CriesofCarrots CriesofCarrots commented Jan 5, 2022

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.

Closes #21142

@CriesofCarrots CriesofCarrots changed the title Prevent rent paying account creation Prevent rent-paying account creation Jan 5, 2022
@CriesofCarrots CriesofCarrots force-pushed the prevent-rent-paying-account-creation branch 2 times, most recently from 58926e0 to 9f4918f Compare January 5, 2022 06:41
@CriesofCarrots
Copy link
Contributor Author

CriesofCarrots commented Jan 5, 2022

@jstarry , can you please start looking at this when you have a minute? (There are a bunch of tests that assume it's okay to create rent-paying accounts, so still lots of fixup before formally ready)

To answer your question from here (#21142 (comment), and I'm sorry for taking so long to do so!), I prefer this location because this is where the execution result is still being determined, ie. mutable. This actually seems even more stark to me after your refactor; the execution result should be set and unmutable after execute_loaded_transaction()

Also, in order to allow legacy rent-paying accounts, we need to know the state before execution, which will be clunky to pass along to collect_accounts_to_store.

(edit) However, your point about only examining writable accounts is a fair one. Added a commit to do this. Could potentially remove the RentState::NativeOrSysvar variant now.

@CriesofCarrots CriesofCarrots force-pushed the prevent-rent-paying-account-creation branch 2 times, most recently from c02d320 to f0eb930 Compare January 6, 2022 02:47
runtime/src/bank.rs Outdated Show resolved Hide resolved
Copy link
Member

@jstarry jstarry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Direction looks good to me! Nice that tx fees are still charged

runtime/src/bank.rs Outdated Show resolved Hide resolved
@CriesofCarrots CriesofCarrots force-pushed the prevent-rent-paying-account-creation branch 9 times, most recently from 0fd398e to de7e748 Compare January 7, 2022 20:03
@CriesofCarrots CriesofCarrots marked this pull request as ready for review January 7, 2022 20:17
@CriesofCarrots
Copy link
Contributor Author

Less the assert, this is ready for real review! All the test churn should be confined to the one commit "Fix rpc tests...", so feel free to ignore that one.

@CriesofCarrots CriesofCarrots force-pushed the prevent-rent-paying-account-creation branch from de7e748 to 00c5873 Compare January 7, 2022 23:20
jstarry
jstarry previously approved these changes Jan 8, 2022
Copy link
Member

@jstarry jstarry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, main concern now is how this impacts users who rely on or incidentally create rent-paying accounts.

runtime/src/account_rent_state.rs Show resolved Hide resolved
runtime/src/bank/transaction_account_state_info.rs Outdated Show resolved Hide resolved
runtime/src/account_rent_state.rs Outdated Show resolved Hide resolved
@ryoqun
Copy link
Member

ryoqun commented Jan 10, 2022

i'll keep my eyes on this. maybe we need to advice all exchanges/wallets beforehand about this upcoming change as this will forbid these kinds of affected small SOL transfers. (anyway, these transfers should be erroneous to begin with).

how about adding a small section here: docs/src/integrations/exchange.md (or, i can do as a follow-up).

behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 7, 2022
solana-labs#22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 8, 2022
solana-labs#22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 8, 2022
solana-labs#22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 8, 2022
solana-labs#22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.
behzadnouri added a commit that referenced this pull request Jul 8, 2022
#22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.
mergify bot pushed a commit that referenced this pull request Jul 8, 2022
#22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.

(cherry picked from commit c99d9f0)

# Conflicts:
#	runtime/src/accounts.rs
#	runtime/src/bank.rs
#	runtime/src/expected_rent_collection.rs
#	runtime/src/rent_collector.rs
behzadnouri added a commit that referenced this pull request Jul 10, 2022
#22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.

(cherry picked from commit c99d9f0)
mergify bot added a commit that referenced this pull request Jul 10, 2022
* names fields in RentResullt::CollectRent enum variant (#26449)

Avoiding ambiguous raw tuple:
    CollectRent((Epoch, u64))

Using named fields instead:
    CollectRent {
        new_rent_epoch: Epoch,
        rent_due: u64,
    },

(cherry picked from commit d7201a8)

# Conflicts:
#	runtime/src/bank.rs
#	runtime/src/expected_rent_collection.rs
#	runtime/src/rent_collector.rs

* removes mergify merge conflicts

* preserves rent_epoch for rent exempt accounts (#26479)

#22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.

(cherry picked from commit c99d9f0)

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 13, 2022
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 13, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

This commit restricts rent-paying accounts TTL extension by preventing
increasing lamports on the account if the account stays below the
rent-exempt minimum.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 13, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts TTL extension by preventing
increasing lamports on the account if the account stays below the
rent-exempt minimum.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 13, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts TTL extension by preventing
increasing lamports on the account if the account stays below the
rent-exempt minimum.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 13, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifeline extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 14, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifeline extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 14, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifeline extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 14, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifeline extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 14, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifeline extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jul 15, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifeline extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
behzadnouri added a commit that referenced this pull request Jul 15, 2022
#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifetime extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
mergify bot pushed a commit that referenced this pull request Jul 15, 2022
#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifetime extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.

(cherry picked from commit bf225ba)

# Conflicts:
#	runtime/src/account_rent_state.rs
#	runtime/src/accounts.rs
#	runtime/src/bank/transaction_account_state_info.rs
#	sdk/src/feature_set.rs
mergify bot added a commit that referenced this pull request Jul 15, 2022
…26635)

* restricts rent-paying accounts lifetime extension (#26606)

#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifetime extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.

(cherry picked from commit bf225ba)

# Conflicts:
#	runtime/src/account_rent_state.rs
#	runtime/src/accounts.rs
#	runtime/src/bank/transaction_account_state_info.rs
#	sdk/src/feature_set.rs

* removes mergify merge conflicts

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
Lcchy pushed a commit to Bonfida/solana that referenced this pull request Jul 22, 2022
solana-labs#22292
prevents rent paying accounts creation going forward. However a rent
paying account can linger on for ever if it is continually topped up but
stays below the rent-exempt minimum.
This can prevent eliminating accounts-rewrites and the problematic
rent_epoch field in accounts.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifetime extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
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

Successfully merging this pull request may close these issues.

None yet

6 participants