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.

Sign up for GitHub

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

feat: disable account rent epoch updates #4593

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
+200 −95
Draft

feat: disable account rent epoch updates #4593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e0a4819
feat: disable account rent epoch updates
jstarry Jan 22, 2025
7ac533a
fix test
jstarry Jan 23, 2025
2d6b1fc
fix test
jstarry Jan 23, 2025
3581b32
fix more tests
jstarry Jan 23, 2025
b3eb3b0
remove test
jstarry Jan 23, 2025
395515c
fix test
jstarry Jan 23, 2025
cbf8870
fix buggy test
jstarry Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client-test/tests/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ fn test_account_subscription() {
"lamports": 100,
"data": "",
"executable": false,
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": 0,
},
});
Expand Down
16 changes: 5 additions & 11 deletions ledger/src/blockstore_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2419,7 +2419,7 @@ pub mod tests {
vote_transaction,
},
std::{collections::BTreeSet, slice, sync::RwLock},
test_case::{test_case, test_matrix},
test_case::test_matrix,
trees::tr,
};

Expand Down Expand Up @@ -3496,19 +3496,14 @@ pub mod tests {
assert_eq!(bank.get_balance(&keypair1.pubkey()), 3);
}

#[test_case(true; "rent_collected")]
#[test_case(false; "rent_not_collected")]
fn test_transaction_result_does_not_affect_bankhash(fee_payer_in_rent_partition: bool) {
#[test]
fn test_transaction_result_does_not_affect_bankhash() {
solana_logger::setup();
let GenesisConfigInfo {
genesis_config,
mint_keypair,
..
} = if fee_payer_in_rent_partition {
create_genesis_config(1000)
} else {
create_genesis_config_with_mint_keypair(Keypair::from_seed(&[1u8; 32]).unwrap(), 1000)
};
} = create_genesis_config_with_mint_keypair(Keypair::from_seed(&[1u8; 32]).unwrap(), 1000);

fn get_instruction_errors() -> Vec<InstructionError> {
vec![
Expand Down Expand Up @@ -3656,8 +3651,7 @@ pub mod tests {
.unwrap()
.last_blockhash
);
// AND should not affect bankhash IF the rent is collected during freeze.
assert_eq!(ok_bank_details == bank_details, fee_payer_in_rent_partition);
assert_eq!(ok_bank_details, bank_details);
// Different types of transaction failure should not affect bank hash
if let Some(prev_bank_details) = &err_bank_details {
assert_eq!(
Expand Down
6 changes: 3 additions & 3 deletions programs/sbf/c/src/invoked/invoked.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,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 == UINT64_MAX);
sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == 0);
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);
Expand All @@ -58,7 +58,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 == UINT64_MAX);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == 0);
sol_assert(!accounts[INVOKED_ARGUMENT_INDEX].executable);

sol_assert(
Expand All @@ -67,7 +67,7 @@ extern uint64_t entrypoint(const uint8_t *input) {
&sbf_loader_upgradeable_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 == UINT64_MAX);
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,
Expand Down
6 changes: 3 additions & 3 deletions programs/sbf/rust/invoked/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn process_instruction(
assert_eq!(accounts[ARGUMENT_INDEX].data_len(), 100);
assert!(accounts[ARGUMENT_INDEX].is_signer);
assert!(accounts[ARGUMENT_INDEX].is_writable);
assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, u64::MAX);
assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, 0);
assert!(!accounts[ARGUMENT_INDEX].executable);
{
let data = accounts[ARGUMENT_INDEX].try_borrow_data()?;
Expand All @@ -65,7 +65,7 @@ fn process_instruction(
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].data_len(), 10);
assert!(accounts[INVOKED_ARGUMENT_INDEX].is_signer);
assert!(accounts[INVOKED_ARGUMENT_INDEX].is_writable);
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch, u64::MAX);
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch, 0);
assert!(!accounts[INVOKED_ARGUMENT_INDEX].executable);

assert_eq!(accounts[INVOKED_PROGRAM_INDEX].key, program_id);
Expand All @@ -75,7 +75,7 @@ fn process_instruction(
);
assert!(!accounts[INVOKED_PROGRAM_INDEX].is_signer);
assert!(!accounts[INVOKED_PROGRAM_INDEX].is_writable);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].rent_epoch, u64::MAX);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].rent_epoch, 0);
assert!(accounts[INVOKED_PROGRAM_INDEX].executable);

assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion programs/sbf/tests/programs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4003,7 +4003,7 @@ fn test_program_sbf_inner_instruction_alignment_checks() {
..
} = create_genesis_config(50);
let (bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);
let noop = create_program(&bank, &bpf_loader_deprecated::id(), "solana_sbf_rust_noop");
let noop = create_program(&bank, &bpf_loader::id(), "solana_sbf_rust_noop");
Copy link
Author

Choose a reason for hiding this comment

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

Wild.. this change uncovered a really old buggy test where we serializing unaligned account info data and deserializing it as aligned 😮

let inner_instruction_alignment_check = create_program(
&bank,
&bpf_loader_deprecated::id(),
Expand Down
4 changes: 2 additions & 2 deletions rpc/src/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5894,7 +5894,7 @@ pub mod tests {
"executable": false,
"owner": "11111111111111111111111111111111",
"lamports": rent_exempt_amount,
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": 0,
}
],
Expand Down Expand Up @@ -6245,7 +6245,7 @@ pub mod tests {
"executable": false,
"lamports": (token_account_rent_exempt_amount + 1),
"owner": bs58::encode(spl_token::id()).into_string(),
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": spl_token::state::Account::LEN
},
],
Expand Down
6 changes: 3 additions & 3 deletions rpc/src/rpc_pubsub.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ mod tests {
"lamports": balance,
"data": [BASE64_STANDARD.encode(expected_data), encoding],
"executable": false,
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": expected_data.len(),
},
},
Expand Down Expand Up @@ -1067,7 +1067,7 @@ mod tests {
"lamports": 100,
"data": expected_data,
"executable": false,
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": account.data().len(),
},
},
Expand Down Expand Up @@ -1251,7 +1251,7 @@ mod tests {
"lamports": 100,
"data": "",
"executable": false,
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": 0,
},
},
Expand Down
62 changes: 25 additions & 37 deletions rpc/src/rpc_subscriptions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1268,10 +1268,7 @@ pub(crate) mod tests {
space: usize,
}

fn make_account_result(
non_default_account: bool,
account_result: AccountResult,
) -> serde_json::Value {
fn make_account_result(account_result: AccountResult) -> serde_json::Value {
json!({
"jsonrpc": "2.0",
"method": "accountNotification",
Expand All @@ -1283,7 +1280,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": account_result.lamports,
"owner": "11111111111111111111111111111111",
"rentEpoch": if non_default_account {u64::MAX} else {0},
"rentEpoch": 0,
"space": account_result.space,
},
},
Expand Down Expand Up @@ -1330,31 +1327,25 @@ pub(crate) mod tests {
0,
&system_program::id(),
);
let expected0 = make_account_result(
true,
AccountResult {
lamports: 1,
subscription: 0,
space: 0,
data: "",
},
);
let expected0 = make_account_result(AccountResult {
lamports: 1,
subscription: 0,
space: 0,
data: "",
});

let tx1 = {
let instruction =
system_instruction::transfer(&alice.pubkey(), &mint_keypair.pubkey(), 1);
let message = Message::new(&[instruction], Some(&mint_keypair.pubkey()));
Transaction::new(&[&alice, &mint_keypair], message, blockhash)
};
let expected1 = make_account_result(
false,
AccountResult {
lamports: 0,
subscription: 2,
space: 0,
data: "",
},
);
let expected1 = make_account_result(AccountResult {
lamports: 0,
subscription: 2,
space: 0,
data: "",
});

let tx2 = system_transaction::create_account(
&mint_keypair,
Expand All @@ -1364,15 +1355,12 @@ pub(crate) mod tests {
1024,
&system_program::id(),
);
let expected2 = make_account_result(
true,
AccountResult {
lamports: 1,
subscription: 4,
space: 1024,
data: "error: data too large for bs58 encoding",
},
);
let expected2 = make_account_result(AccountResult {
lamports: 1,
subscription: 4,
space: 1024,
data: "error: data too large for bs58 encoding",
});

let subscribe_cases = vec![
(alice.pubkey(), tx0, expected0),
Expand Down Expand Up @@ -1874,7 +1862,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": 1,
"owner": "Stake11111111111111111111111111111111111111",
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": 16,
},
"pubkey": alice.pubkey().to_string(),
Expand Down Expand Up @@ -2046,7 +2034,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": lamports,
"owner": "Stake11111111111111111111111111111111111111",
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": 16,
},
"pubkey": pubkey,
Expand Down Expand Up @@ -2339,7 +2327,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": lamports,
"owner": "Stake11111111111111111111111111111111111111",
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": 16,
},
"pubkey": pubkey,
Expand Down Expand Up @@ -2846,7 +2834,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": 1,
"owner": "Stake11111111111111111111111111111111111111",
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": 16,
},
},
Expand Down Expand Up @@ -2900,7 +2888,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": 1,
"owner": "Stake11111111111111111111111111111111111111",
"rentEpoch": u64::MAX,
"rentEpoch": 0,
"space": 16,
},
},
Expand Down
9 changes: 9 additions & 0 deletions runtime/src/bank.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3992,6 +3992,15 @@ impl Bank {
return;
}

// When account rent epoch updates are disabled, partitioned rent
// collection no longer has any effect and can be skipped
if self
.feature_set
.is_active(&feature_set::disable_account_rent_epoch_updates::id())
{
return;
}

let mut measure = Measure::start("collect_rent_eagerly-ms");
let partitions = self.rent_collection_partitions();
let count = partitions.len();
Expand Down
Loading
Loading