Skip to content

Commit

Permalink
MaxAge refactoring to hold resolved epoch
Browse files Browse the repository at this point in the history
  • Loading branch information
apfitzge committed Nov 5, 2024
1 parent 54be4c9 commit d5c5878
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 72 deletions.
28 changes: 17 additions & 11 deletions core/src/banking_stage/consume_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,12 @@ mod tests {
..
} = create_slow_genesis_config(10_000);
let (bank, bank_forks) = Bank::new_no_wallclock_throttle_for_tests(&genesis_config);
let bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::new_unique(), 1));
// Warp to next epoch for MaxAge tests.
let bank = Arc::new(Bank::new_from_parent(
bank.clone(),
&Pubkey::new_unique(),
bank.get_epoch_info().slots_in_epoch,
));

let ledger_path = get_tmp_ledger_path_auto_delete!();
let blockstore = Blockstore::open(ledger_path.path())
Expand Down Expand Up @@ -889,7 +894,7 @@ mod tests {
let bid = TransactionBatchId::new(0);
let id = TransactionId::new(0);
let max_age = MaxAge {
epoch_invalidation_slot: bank.slot(),
resolved_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
};
let work = ConsumeWork {
Expand Down Expand Up @@ -938,7 +943,7 @@ mod tests {
let bid = TransactionBatchId::new(0);
let id = TransactionId::new(0);
let max_age = MaxAge {
epoch_invalidation_slot: bank.slot(),
resolved_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
};
let work = ConsumeWork {
Expand Down Expand Up @@ -988,7 +993,7 @@ mod tests {
let id1 = TransactionId::new(1);
let id2 = TransactionId::new(0);
let max_age = MaxAge {
epoch_invalidation_slot: bank.slot(),
resolved_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
};
consume_sender
Expand Down Expand Up @@ -1049,7 +1054,7 @@ mod tests {
let id1 = TransactionId::new(1);
let id2 = TransactionId::new(0);
let max_age = MaxAge {
epoch_invalidation_slot: bank.slot(),
resolved_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
};
consume_sender
Expand Down Expand Up @@ -1103,6 +1108,7 @@ mod tests {
.unwrap()
.set_bank_for_test(bank.clone());
assert!(bank.slot() > 0);
assert!(bank.epoch() > 0);

// No conflicts between transactions. Test 6 cases.
// 1. Epoch expiration, before slot => still succeeds due to resanitizing
Expand Down Expand Up @@ -1187,27 +1193,27 @@ mod tests {
transactions: txs,
max_ages: vec![
MaxAge {
epoch_invalidation_slot: bank.slot() - 1,
resolved_epoch: bank.epoch() - 1,
alt_invalidation_slot: Slot::MAX,
},
MaxAge {
epoch_invalidation_slot: bank.slot(),
resolved_epoch: bank.epoch(),
alt_invalidation_slot: Slot::MAX,
},
MaxAge {
epoch_invalidation_slot: bank.slot() + 1,
resolved_epoch: bank.epoch() + 1,
alt_invalidation_slot: Slot::MAX,
},
MaxAge {
epoch_invalidation_slot: u64::MAX,
resolved_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot() - 1,
},
MaxAge {
epoch_invalidation_slot: u64::MAX,
resolved_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
},
MaxAge {
epoch_invalidation_slot: u64::MAX,
resolved_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot() + 1,
},
],
Expand Down
4 changes: 2 additions & 2 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,8 @@ impl Consumer {
let pre_results = txs.iter().zip(max_ages).map(|(tx, max_age)| {
// If the transaction was sanitized before this bank's epoch,
// additional checks are necessary.
if bank.slot() > max_age.epoch_invalidation_slot {
// Reserved key set may have cahnged, so we must verify that
if bank.epoch() != max_age.resolved_epoch {
// Reserved key set may have changed, so we must verify that
// no writable keys are reserved.
bank.check_reserved_keys(tx)?;
}
Expand Down
14 changes: 12 additions & 2 deletions core/src/banking_stage/scheduler_messages.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use {
solana_runtime_transaction::runtime_transaction::RuntimeTransaction,
solana_sdk::{clock::Slot, transaction::SanitizedTransaction},
solana_sdk::{
clock::{Epoch, Slot},
transaction::SanitizedTransaction,
},
std::fmt::Display,
};

Expand Down Expand Up @@ -38,10 +41,17 @@ impl Display for TransactionId {

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub struct MaxAge {
pub epoch_invalidation_slot: Slot,
pub resolved_epoch: Epoch,
pub alt_invalidation_slot: Slot,
}

impl MaxAge {
pub const MAX: Self = Self {
resolved_epoch: Epoch::MAX,
alt_invalidation_slot: Slot::MAX,
};
}

/// Message: [Scheduler -> Worker]
/// Transactions to be consumed (i.e. executed, recorded, and committed)
pub struct ConsumeWork {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,8 +622,8 @@ mod tests {
crossbeam_channel::{unbounded, Receiver},
itertools::Itertools,
solana_sdk::{
clock::Slot, compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message,
packet::Packet, pubkey::Pubkey, signature::Keypair, signer::Signer, system_instruction,
compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, packet::Packet,
pubkey::Pubkey, signature::Keypair, signer::Signer, system_instruction,
transaction::Transaction,
},
std::{borrow::Borrow, sync::Arc},
Expand Down Expand Up @@ -709,10 +709,7 @@ mod tests {
);
let transaction_ttl = SanitizedTransactionTTL {
transaction,
max_age: MaxAge {
epoch_invalidation_slot: Slot::MAX,
alt_invalidation_slot: Slot::MAX,
},
max_age: MaxAge::MAX,
};
const TEST_TRANSACTION_COST: u64 = 5000;
container.insert_new_transaction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use {
solana_sdk::{
self,
address_lookup_table::state::estimate_last_valid_slot,
clock::{Slot, FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE},
clock::{Epoch, Slot, FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE},
fee::FeeBudgetLimits,
saturating_add_assign,
transaction::SanitizedTransaction,
Expand Down Expand Up @@ -686,11 +686,10 @@ impl<T: LikeClusterInfo> SchedulerController<T> {
}
}

/// Given the last slot in the epoch, the minimum deactivation slot,
/// and the current slot, return the `MaxAge` that should be used for
/// the transaction. This is used to determine the maximum slot that a
/// transaction will be considered valid for, without re-resolving addresses
/// or resanitizing.
/// Given the epoch, the minimum deactivation slot, and the current slot,
/// return the `MaxAge` that should be used for the transaction. This is used
/// to determine the maximum slot that a transaction will be considered valid
/// for, without re-resolving addresses or resanitizing.
///
/// This function considers the deactivation period of Address Table
/// accounts. If the deactivation period runs past the end of the epoch,
Expand All @@ -702,14 +701,10 @@ impl<T: LikeClusterInfo> SchedulerController<T> {
/// slots, the value used here is the lower-bound on the deactivation
/// period, i.e. the transaction's address lookups are valid until
/// AT LEAST this slot.
fn calculate_max_age(
last_slot_in_epoch: Slot,
deactivation_slot: Slot,
current_slot: Slot,
) -> MaxAge {
fn calculate_max_age(resolved_epoch: Epoch, deactivation_slot: Slot, current_slot: Slot) -> MaxAge {
let alt_min_expire_slot = estimate_last_valid_slot(deactivation_slot.min(current_slot));
MaxAge {
epoch_invalidation_slot: last_slot_in_epoch,
resolved_epoch,
alt_invalidation_slot: alt_min_expire_slot,
}
}
Expand Down Expand Up @@ -1221,23 +1216,23 @@ mod tests {
#[test]
fn test_calculate_max_age() {
let current_slot = 100;
let last_slot_in_epoch = 1000;
let resolved_epoch = 10;

// ALT deactivation slot is delayed
assert_eq!(
calculate_max_age(last_slot_in_epoch, current_slot - 1, current_slot),
calculate_max_age(resolved_epoch, current_slot - 1, current_slot),
MaxAge {
epoch_invalidation_slot: last_slot_in_epoch,
resolved_epoch,
alt_invalidation_slot: current_slot - 1
+ solana_sdk::slot_hashes::get_entries() as u64,
}
);

// no deactivation slot
assert_eq!(
calculate_max_age(last_slot_in_epoch, u64::MAX, current_slot),
calculate_max_age(resolved_epoch, u64::MAX, current_slot),
MaxAge {
epoch_invalidation_slot: last_slot_in_epoch,
resolved_epoch,
alt_invalidation_slot: current_slot + solana_sdk::slot_hashes::get_entries() as u64,
}
);
Expand Down
34 changes: 6 additions & 28 deletions core/src/banking_stage/transaction_scheduler/transaction_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,8 @@ mod tests {
use {
super::*,
solana_sdk::{
clock::Slot, compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message,
packet::Packet, signature::Keypair, signer::Signer, system_instruction,
transaction::Transaction,
compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, packet::Packet,
signature::Keypair, signer::Signer, system_instruction, transaction::Transaction,
},
};

Expand All @@ -234,10 +233,7 @@ mod tests {
);
let transaction_ttl = SanitizedTransactionTTL {
transaction: RuntimeTransaction::from_transaction_for_tests(tx),
max_age: MaxAge {
epoch_invalidation_slot: Slot::MAX,
alt_invalidation_slot: Slot::MAX,
},
max_age: MaxAge::MAX,
};
const TEST_TRANSACTION_COST: u64 = 5000;
TransactionState::new(
Expand Down Expand Up @@ -328,13 +324,7 @@ mod tests {
transaction_state,
TransactionState::Unprocessed { .. }
));
assert_eq!(
transaction_ttl.max_age,
MaxAge {
epoch_invalidation_slot: Slot::MAX,
alt_invalidation_slot: Slot::MAX,
}
);
assert_eq!(transaction_ttl.max_age, MaxAge::MAX);

let _ = transaction_state.transition_to_pending();
assert!(matches!(
Expand All @@ -352,13 +342,7 @@ mod tests {
transaction_state,
TransactionState::Unprocessed { .. }
));
assert_eq!(
transaction_ttl.max_age,
MaxAge {
epoch_invalidation_slot: Slot::MAX,
alt_invalidation_slot: Slot::MAX,
}
);
assert_eq!(transaction_ttl.max_age, MaxAge::MAX);

// ensure transaction_ttl is not lost through state transitions
let transaction_ttl = transaction_state.transition_to_pending();
Expand All @@ -373,12 +357,6 @@ mod tests {
transaction_state,
TransactionState::Unprocessed { .. }
));
assert_eq!(
transaction_ttl.max_age,
MaxAge {
epoch_invalidation_slot: Slot::MAX,
alt_invalidation_slot: Slot::MAX,
}
);
assert_eq!(transaction_ttl.max_age, MaxAge::MAX);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@ mod tests {
solana_runtime_transaction::runtime_transaction::RuntimeTransaction,
solana_sdk::{
compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, packet::Packet,
signature::Keypair, signer::Signer, slot_history::Slot, system_instruction,
transaction::Transaction,
signature::Keypair, signer::Signer, system_instruction, transaction::Transaction,
},
};

Expand Down Expand Up @@ -194,10 +193,7 @@ mod tests {
);
let transaction_ttl = SanitizedTransactionTTL {
transaction: tx,
max_age: MaxAge {
epoch_invalidation_slot: Slot::MAX,
alt_invalidation_slot: Slot::MAX,
},
max_age: MaxAge::MAX,
};
const TEST_TRANSACTION_COST: u64 = 5000;
(transaction_ttl, packet, priority, TEST_TRANSACTION_COST)
Expand Down

0 comments on commit d5c5878

Please sign in to comment.