This repository was archived by the owner on Jan 22, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Re-do rent collection check on rent-exempt account #11349
Merged
ryoqun
merged 11 commits into
solana-labs:master
from
ryoqun:account-storage-exhaustion
Aug 17, 2020
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
1d0e8eb
wip: re-do rent collection check on rent-exempt account
ryoqun 8c6c447
Let's see how the ci goes
ryoqun ae90f3f
Restore previous code
ryoqun 2be9db9
Well, almost all new changes are revertable
ryoqun 69f0e9c
Update doc
ryoqun 884b5aa
Add test and gating
ryoqun 3b987d2
Fix tests
ryoqun 8eca8d3
Fix tests, especially avoid to change abi...
ryoqun 12a9bd5
Fix more tests...
ryoqun 7dd0f5f
Fix snapshot restore
ryoqun 047b43a
Align to _new_ with better uninitialized detection
ryoqun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,11 +24,11 @@ Even if those processes are out of scope of rent collection, all of manipulated | |
|
|
||
| ## Actual processing of collecting rent | ||
|
|
||
| Rent is due for one epoch's worth of time, and accounts always have `Account::rent_epoch` of `current_epoch + 1`. | ||
| Rent is due for one epoch's worth of time, and accounts have `Account::rent_epoch` of `current_epoch` or `current_epoch + 1` depending on the rent regime. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rwalker-apple Hi, I'm yet again tweaking the rent protocol a bit, mainly affected by this dos possibility: #11342. Could you review this in your free time? |
||
|
|
||
| If the account is in the exempt regime, `Account::rent_epoch` is simply pushed to `current_epoch + 1`. | ||
| If the account is in the exempt regime, `Account::rent_epoch` is simply updated to `current_epoch`. | ||
|
|
||
| If the account is non-exempt, the difference between the next epoch and `Account::rent_epoch` is used to calculate the amount of rent owed by this account \(via `Rent::due()`\). Any fractional lamports of the calculation are truncated. Rent due is deducted from `Account::lamports` and `Account::rent_epoch` is updated to the next epoch. If the amount of rent due is less than one lamport, no changes are made to the account. | ||
| If the account is non-exempt, the difference between the next epoch and `Account::rent_epoch` is used to calculate the amount of rent owed by this account \(via `Rent::due()`\). Any fractional lamports of the calculation are truncated. Rent due is deducted from `Account::lamports` and `Account::rent_epoch` is updated to `current_epoch + 1` (= next epoch). If the amount of rent due is less than one lamport, no changes are made to the account. | ||
|
|
||
| Accounts whose balance is insufficient to satisfy the rent that would be due simply fail to load. | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,7 +36,7 @@ extern uint64_t entrypoint(const uint8_t *input) { | |
| sol_assert(accounts[ARGUMENT_INDEX].data_len == 100); | ||
| sol_assert(accounts[ARGUMENT_INDEX].is_signer); | ||
| sol_assert(accounts[ARGUMENT_INDEX].is_writable); | ||
| sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == 1); | ||
| sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == 0); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jackcmay Like the RPC land, this pr changes slightly changes the semantics of |
||
| sol_assert(!accounts[ARGUMENT_INDEX].executable); | ||
| for (int i = 0; i < accounts[ARGUMENT_INDEX].data_len; i++) { | ||
| sol_assert(accounts[ARGUMENT_INDEX].data[i] == i); | ||
|
|
@@ -48,7 +48,7 @@ extern uint64_t entrypoint(const uint8_t *input) { | |
| sol_assert(accounts[INVOKED_ARGUMENT_INDEX].data_len == 10); | ||
| sol_assert(accounts[INVOKED_ARGUMENT_INDEX].is_signer); | ||
| sol_assert(accounts[INVOKED_ARGUMENT_INDEX].is_writable); | ||
| sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == 1); | ||
| sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == 0); | ||
| sol_assert(!accounts[INVOKED_ARGUMENT_INDEX].executable); | ||
|
|
||
| sol_assert( | ||
|
|
@@ -57,7 +57,7 @@ extern uint64_t entrypoint(const uint8_t *input) { | |
| &bpf_loader_id)); | ||
| sol_assert(!accounts[INVOKED_PROGRAM_INDEX].is_signer); | ||
| sol_assert(!accounts[INVOKED_PROGRAM_INDEX].is_writable); | ||
| sol_assert(accounts[INVOKED_PROGRAM_INDEX].rent_epoch == 1); | ||
| sol_assert(accounts[INVOKED_PROGRAM_INDEX].rent_epoch == 0); | ||
| sol_assert(accounts[INVOKED_PROGRAM_INDEX].executable); | ||
|
|
||
| sol_assert(SolPubkey_same(accounts[INVOKED_PROGRAM_INDEX].key, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CriesofCarrots @jstarry Strictly speaking, we're breaking some semantics (=~ compatibility) of our rpc api a bit here according to this new rule for
rentEpoch: https://github.com/solana-labs/solana/pull/11349/files#r469691300.It's an incompatible change, but the change is soooo subtle, I think. So, I believe there should be virtually no one relying on
rentEpochbeing the current epoch or the next for rent-exempt accounts in any-conceivable way for the bizarre usecases.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, if you have something to come to mind in terms of various partner integartions, please let me know. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ryoqun agreed, very unlikely this will cause any issues. I highly doubt anyone is using
rentEpochright nowThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know of any integrations using
rentEpoch; I'm comfortable with this breaking change.