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

v2.0: Store epoch in MaxAge (backport of #3485) #3501

Merged
merged 2 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -762,7 +762,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 @@ -842,7 +847,7 @@ mod tests {
let bid = TransactionBatchId::new(0);
let id = TransactionId::new(0);
let max_age = MaxAge {
epoch_invalidation_slot: bank.slot(),
sanitized_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
};
let work = ConsumeWork {
Expand Down Expand Up @@ -891,7 +896,7 @@ mod tests {
let bid = TransactionBatchId::new(0);
let id = TransactionId::new(0);
let max_age = MaxAge {
epoch_invalidation_slot: bank.slot(),
sanitized_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
};
let work = ConsumeWork {
Expand Down Expand Up @@ -941,7 +946,7 @@ mod tests {
let id1 = TransactionId::new(1);
let id2 = TransactionId::new(0);
let max_age = MaxAge {
epoch_invalidation_slot: bank.slot(),
sanitized_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
};
consume_sender
Expand Down Expand Up @@ -1002,7 +1007,7 @@ mod tests {
let id1 = TransactionId::new(1);
let id2 = TransactionId::new(0);
let max_age = MaxAge {
epoch_invalidation_slot: bank.slot(),
sanitized_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
};
consume_sender
Expand Down Expand Up @@ -1056,6 +1061,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 @@ -1140,27 +1146,27 @@ mod tests {
transactions: txs,
max_ages: vec![
MaxAge {
epoch_invalidation_slot: bank.slot() - 1,
sanitized_epoch: bank.epoch() - 1,
alt_invalidation_slot: Slot::MAX,
},
MaxAge {
epoch_invalidation_slot: bank.slot(),
sanitized_epoch: bank.epoch(),
alt_invalidation_slot: Slot::MAX,
},
MaxAge {
epoch_invalidation_slot: bank.slot() + 1,
sanitized_epoch: bank.epoch() + 1,
alt_invalidation_slot: Slot::MAX,
},
MaxAge {
epoch_invalidation_slot: u64::MAX,
sanitized_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot() - 1,
},
MaxAge {
epoch_invalidation_slot: u64::MAX,
sanitized_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot(),
},
MaxAge {
epoch_invalidation_slot: u64::MAX,
sanitized_epoch: bank.epoch(),
alt_invalidation_slot: bank.slot() + 1,
},
],
Expand Down
2 changes: 1 addition & 1 deletion core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ impl Consumer {
// This means that the transaction may cross and epoch boundary (not allowed),
// or account lookup tables may have been closed.
let pre_results = txs.iter().zip(max_ages).map(|(tx, max_age)| {
if bank.slot() > max_age.epoch_invalidation_slot {
if bank.epoch() != max_age.sanitized_epoch {
// Epoch has rolled over. Need to fully re-verify the transaction.
// Pre-compiles are verified here.
// Attempt re-sanitization after epoch-cross.
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 {
super::immutable_deserialized_packet::ImmutableDeserializedPacket,
solana_sdk::{clock::Slot, transaction::SanitizedTransaction},
solana_sdk::{
clock::{Epoch, Slot},
transaction::SanitizedTransaction,
},
std::{fmt::Display, sync::Arc},
};

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 sanitized_epoch: Epoch,
pub alt_invalidation_slot: Slot,
}

impl MaxAge {
pub const MAX: Self = Self {
sanitized_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 @@ -588,8 +588,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 @@ -675,10 +675,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 @@ -30,7 +30,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 @@ -506,9 +506,7 @@ impl SchedulerController {
(root_bank, working_bank)
};
let alt_resolved_slot = root_bank.slot();
let last_slot_in_epoch = working_bank
.epoch_schedule()
.get_last_slot_in_epoch(working_bank.epoch());
let sanitized_epoch = root_bank.epoch();
let transaction_account_lock_limit = working_bank.get_transaction_account_lock_limit();
let vote_only = working_bank.vote_only_bank();

Expand All @@ -531,7 +529,7 @@ impl SchedulerController {
.build_sanitized_transaction(
vote_only,
root_bank.as_ref(),
working_bank.get_reserved_account_keys(),
root_bank.get_reserved_account_keys(),
)
.map(|(tx, deactivation_slot)| (packet.clone(), tx, deactivation_slot))
})
Expand All @@ -554,7 +552,7 @@ impl SchedulerController {
arc_packets.push(packet);
transactions.push(tx);
max_ages.push(calculate_max_age(
last_slot_in_epoch,
sanitized_epoch,
deactivation_slot,
alt_resolved_slot,
));
Expand Down Expand Up @@ -680,11 +678,10 @@ impl SchedulerController {
}
}

/// 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 @@ -697,13 +694,13 @@ impl SchedulerController {
/// period, i.e. the transaction's address lookups are valid until
/// AT LEAST this slot.
fn calculate_max_age(
last_slot_in_epoch: Slot,
sanitized_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,
sanitized_epoch,
alt_invalidation_slot: alt_min_expire_slot,
}
}
Expand Down Expand Up @@ -1213,23 +1210,23 @@ mod tests {
#[test]
fn test_calculate_max_age() {
let current_slot = 100;
let last_slot_in_epoch = 1000;
let sanitized_epoch = 10;

// ALT deactivation slot is delayed
assert_eq!(
calculate_max_age(last_slot_in_epoch, current_slot - 1, current_slot),
calculate_max_age(sanitized_epoch, current_slot - 1, current_slot),
MaxAge {
epoch_invalidation_slot: last_slot_in_epoch,
sanitized_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(sanitized_epoch, u64::MAX, current_slot),
MaxAge {
epoch_invalidation_slot: last_slot_in_epoch,
sanitized_epoch,
alt_invalidation_slot: current_slot + solana_sdk::slot_hashes::get_entries() as u64,
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,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 @@ -233,10 +232,7 @@ mod tests {
);
let transaction_ttl = SanitizedTransactionTTL {
transaction: SanitizedTransaction::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 @@ -327,13 +323,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 @@ -351,13 +341,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 @@ -372,12 +356,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 @@ -161,7 +161,6 @@ mod tests {
packet::Packet,
signature::Keypair,
signer::Signer,
slot_history::Slot,
system_instruction,
transaction::{SanitizedTransaction, Transaction},
},
Expand Down Expand Up @@ -199,10 +198,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