Cleanup - require_static_program_ids_in_transaction (#31767)

require_static_program_ids_in_transaction
This commit is contained in:
Alexander Meißner 2023-06-07 17:12:41 +02:00 committed by GitHub
parent 989e61318b
commit ee2c2ef6c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 33 additions and 120 deletions

View File

@ -178,7 +178,6 @@ fn simulate_transaction(
MessageHash::Compute, MessageHash::Compute,
Some(false), // is_simple_vote_tx Some(false), // is_simple_vote_tx
bank, bank,
true, // require_static_program_ids
) { ) {
Err(err) => { Err(err) => {
return BanksTransactionResultWithSimulation { return BanksTransactionResultWithSimulation {
@ -330,7 +329,6 @@ impl Banks for BanksServer {
MessageHash::Compute, MessageHash::Compute,
Some(false), // is_simple_vote_tx Some(false), // is_simple_vote_tx
bank.as_ref(), bank.as_ref(),
true, // require_static_program_ids
) { ) {
Ok(tx) => tx, Ok(tx) => tx,
Err(err) => return Some(Err(err)), Err(err) => return Some(Err(err)),

View File

@ -1697,7 +1697,6 @@ mod tests {
MessageHash::Compute, MessageHash::Compute,
Some(false), Some(false),
bank.as_ref(), bank.as_ref(),
true, // require_static_program_ids
) )
.unwrap(); .unwrap();

View File

@ -157,7 +157,6 @@ mod tests {
MessageHash::Compute, MessageHash::Compute,
Some(false), Some(false),
bank, bank,
true, // require_static_program_ids
) )
.unwrap() .unwrap()
} }

View File

@ -40,7 +40,6 @@ fn bench_gpusigverify(bencher: &mut Bencher) {
message_hash, message_hash,
None, None,
SimpleAddressLoader::Disabled, SimpleAddressLoader::Disabled,
true, // require_static_program_ids
) )
}?; }?;
@ -82,7 +81,6 @@ fn bench_cpusigverify(bencher: &mut Bencher) {
message_hash, message_hash,
None, None,
SimpleAddressLoader::Disabled, SimpleAddressLoader::Disabled,
true, // require_static_program_ids
) )
}?; }?;

View File

@ -1041,7 +1041,6 @@ mod tests {
message_hash, message_hash,
None, None,
SimpleAddressLoader::Disabled, SimpleAddressLoader::Disabled,
true, // require_static_program_ids
) )
}?; }?;

View File

@ -922,7 +922,6 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String>
MessageHash::Compute, MessageHash::Compute,
None, None,
SimpleAddressLoader::Disabled, SimpleAddressLoader::Disabled,
true, // require_static_program_ids
) )
.map_err(|err| { .map_err(|err| {
warn!("Failed to compute cost of transaction: {:?}", err); warn!("Failed to compute cost of transaction: {:?}", err);

View File

@ -1984,12 +1984,7 @@ impl Blockstore {
.into_iter() .into_iter()
.flat_map(|entry| entry.transactions) .flat_map(|entry| entry.transactions)
.map(|transaction| { .map(|transaction| {
if let Err(err) = transaction.sanitize( if let Err(err) = transaction.sanitize() {
// Don't enable additional sanitization checks until
// all clusters have activated the static program id
// feature gate so that bigtable upload isn't affected
false, // require_static_program_ids
) {
warn!( warn!(
"Blockstore::get_block sanitize failed: {:?}, \ "Blockstore::get_block sanitize failed: {:?}, \
slot: {:?}, \ slot: {:?}, \
@ -2376,9 +2371,7 @@ impl Blockstore {
.cloned() .cloned()
.flat_map(|entry| entry.transactions) .flat_map(|entry| entry.transactions)
.map(|transaction| { .map(|transaction| {
if let Err(err) = transaction.sanitize( if let Err(err) = transaction.sanitize() {
true, // require_static_program_ids
) {
warn!( warn!(
"Blockstore::find_transaction_in_slot sanitize failed: {:?}, \ "Blockstore::find_transaction_in_slot sanitize failed: {:?}, \
slot: {:?}, \ slot: {:?}, \

View File

@ -4516,14 +4516,8 @@ fn sanitize_transaction(
transaction: VersionedTransaction, transaction: VersionedTransaction,
address_loader: impl AddressLoader, address_loader: impl AddressLoader,
) -> Result<SanitizedTransaction> { ) -> Result<SanitizedTransaction> {
SanitizedTransaction::try_create( SanitizedTransaction::try_create(transaction, MessageHash::Compute, None, address_loader)
transaction, .map_err(|err| Error::invalid_params(format!("invalid transaction: {err}")))
MessageHash::Compute,
None,
address_loader,
true, // require_static_program_ids
)
.map_err(|err| Error::invalid_params(format!("invalid transaction: {err}")))
} }
pub fn create_validator_exit(exit: Arc<AtomicBool>) -> Arc<RwLock<Exit>> { pub fn create_validator_exit(exit: Arc<AtomicBool>) -> Arc<RwLock<Exit>> {

View File

@ -349,7 +349,6 @@ pub(crate) mod tests {
MessageHash::Compute, MessageHash::Compute,
None, None,
SimpleAddressLoader::Disabled, SimpleAddressLoader::Disabled,
true, // require_static_program_ids
) )
.unwrap(); .unwrap();

View File

@ -3763,16 +3763,7 @@ impl Bank {
pub fn prepare_entry_batch(&self, txs: Vec<VersionedTransaction>) -> Result<TransactionBatch> { pub fn prepare_entry_batch(&self, txs: Vec<VersionedTransaction>) -> Result<TransactionBatch> {
let sanitized_txs = txs let sanitized_txs = txs
.into_iter() .into_iter()
.map(|tx| { .map(|tx| SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self))
SanitizedTransaction::try_create(
tx,
MessageHash::Compute,
None,
self,
self.feature_set
.is_active(&feature_set::require_static_program_ids_in_transaction::ID),
)
})
.collect::<Result<Vec<_>>>()?; .collect::<Result<Vec<_>>>()?;
let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let tx_account_lock_limit = self.get_transaction_account_lock_limit();
let lock_results = self let lock_results = self
@ -6852,14 +6843,7 @@ impl Bank {
tx.message.hash() tx.message.hash()
}; };
SanitizedTransaction::try_create( SanitizedTransaction::try_create(tx, message_hash, None, self)
tx,
message_hash,
None,
self,
self.feature_set
.is_active(&feature_set::require_static_program_ids_in_transaction::ID),
)
}?; }?;
if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles

View File

@ -367,7 +367,6 @@ mod tests {
MessageHash::Compute, MessageHash::Compute,
Some(true), Some(true),
SimpleAddressLoader::Disabled, SimpleAddressLoader::Disabled,
true, // require_static_program_ids
) )
.unwrap(); .unwrap();
let mut tx_cost = TransactionCost::new_with_capacity(1); let mut tx_cost = TransactionCost::new_with_capacity(1);

View File

@ -39,10 +39,10 @@ pub enum VersionedMessage {
} }
impl VersionedMessage { impl VersionedMessage {
pub fn sanitize(&self, require_static_program_ids: bool) -> Result<(), SanitizeError> { pub fn sanitize(&self) -> Result<(), SanitizeError> {
match self { match self {
Self::Legacy(message) => message.sanitize(), Self::Legacy(message) => message.sanitize(),
Self::V0(message) => message.sanitize(require_static_program_ids), Self::V0(message) => message.sanitize(),
} }
} }

View File

@ -18,7 +18,7 @@ impl TryFrom<VersionedMessage> for SanitizedVersionedMessage {
impl SanitizedVersionedMessage { impl SanitizedVersionedMessage {
pub fn try_new(message: VersionedMessage) -> Result<Self, SanitizeError> { pub fn try_new(message: VersionedMessage) -> Result<Self, SanitizeError> {
message.sanitize(true /* require_static_program_ids */)?; message.sanitize()?;
Ok(Self { message }) Ok(Self { message })
} }

View File

@ -89,7 +89,7 @@ pub struct Message {
impl Message { impl Message {
/// Sanitize message fields and compiled instruction indexes /// Sanitize message fields and compiled instruction indexes
pub fn sanitize(&self, reject_dynamic_program_ids: bool) -> Result<(), SanitizeError> { pub fn sanitize(&self) -> Result<(), SanitizeError> {
let num_static_account_keys = self.account_keys.len(); let num_static_account_keys = self.account_keys.len();
if usize::from(self.header.num_required_signatures) if usize::from(self.header.num_required_signatures)
.saturating_add(usize::from(self.header.num_readonly_unsigned_accounts)) .saturating_add(usize::from(self.header.num_readonly_unsigned_accounts))
@ -143,18 +143,15 @@ impl Message {
.checked_sub(1) .checked_sub(1)
.expect("message doesn't contain any account keys"); .expect("message doesn't contain any account keys");
// switch to rejecting program ids loaded from lookup tables so that // reject program ids loaded from lookup tables so that
// static analysis on program instructions can be performed without // static analysis on program instructions can be performed
// loading on-chain data from a bank // without loading on-chain data from a bank
let max_program_id_ix = if reject_dynamic_program_ids { let max_program_id_ix =
// `expect` is safe because of earlier check that // `expect` is safe because of earlier check that
// `num_static_account_keys` is non-zero // `num_static_account_keys` is non-zero
num_static_account_keys num_static_account_keys
.checked_sub(1) .checked_sub(1)
.expect("message doesn't contain any static account keys") .expect("message doesn't contain any static account keys");
} else {
max_account_ix
};
for ci in &self.instructions { for ci in &self.instructions {
if usize::from(ci.program_id_index) > max_program_id_ix { if usize::from(ci.program_id_index) > max_program_id_ix {
@ -375,9 +372,7 @@ mod tests {
account_keys: vec![Pubkey::new_unique()], account_keys: vec![Pubkey::new_unique()],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_ok()); .is_ok());
} }
@ -396,9 +391,7 @@ mod tests {
}], }],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_ok()); .is_ok());
} }
@ -417,9 +410,7 @@ mod tests {
}], }],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_ok()); .is_ok());
} }
@ -444,13 +435,7 @@ mod tests {
..Message::default() ..Message::default()
}; };
assert!(message.sanitize( assert!(message.sanitize().is_err());
false, // require_static_program_ids
).is_ok());
assert!(message.sanitize(
true, // require_static_program_ids
).is_err());
} }
#[test] #[test]
@ -473,9 +458,7 @@ mod tests {
}], }],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_ok()); .is_ok());
} }
@ -486,9 +469,7 @@ mod tests {
account_keys: vec![Pubkey::new_unique()], account_keys: vec![Pubkey::new_unique()],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_err()); .is_err());
} }
@ -503,9 +484,7 @@ mod tests {
account_keys: vec![Pubkey::new_unique()], account_keys: vec![Pubkey::new_unique()],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_err()); .is_err());
} }
@ -524,9 +503,7 @@ mod tests {
}], }],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_err()); .is_err());
} }
@ -540,9 +517,7 @@ mod tests {
account_keys: (0..=u8::MAX).map(|_| Pubkey::new_unique()).collect(), account_keys: (0..=u8::MAX).map(|_| Pubkey::new_unique()).collect(),
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_ok()); .is_ok());
} }
@ -556,9 +531,7 @@ mod tests {
account_keys: (0..=256).map(|_| Pubkey::new_unique()).collect(), account_keys: (0..=256).map(|_| Pubkey::new_unique()).collect(),
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_err()); .is_err());
} }
@ -577,9 +550,7 @@ mod tests {
}], }],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_ok()); .is_ok());
} }
@ -598,9 +569,7 @@ mod tests {
}], }],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_err()); .is_err());
} }
@ -625,12 +594,7 @@ mod tests {
..Message::default() ..Message::default()
}; };
assert!(message assert!(message.sanitize().is_err());
.sanitize(true /* require_static_program_ids */)
.is_err());
assert!(message
.sanitize(false /* require_static_program_ids */)
.is_err());
} }
#[test] #[test]
@ -653,9 +617,7 @@ mod tests {
}], }],
..Message::default() ..Message::default()
} }
.sanitize( .sanitize()
true, // require_static_program_ids
)
.is_err()); .is_err());
} }

View File

@ -95,9 +95,8 @@ impl SanitizedTransaction {
message_hash: impl Into<MessageHash>, message_hash: impl Into<MessageHash>,
is_simple_vote_tx: Option<bool>, is_simple_vote_tx: Option<bool>,
address_loader: impl AddressLoader, address_loader: impl AddressLoader,
require_static_program_ids: bool,
) -> Result<Self> { ) -> Result<Self> {
tx.sanitize(require_static_program_ids)?; tx.sanitize()?;
let message_hash = match message_hash.into() { let message_hash = match message_hash.into() {
MessageHash::Compute => tx.message.hash(), MessageHash::Compute => tx.message.hash(),

View File

@ -115,11 +115,8 @@ impl VersionedTransaction {
}) })
} }
pub fn sanitize( pub fn sanitize(&self) -> std::result::Result<(), SanitizeError> {
&self, self.message.sanitize()?;
require_static_program_ids: bool,
) -> std::result::Result<(), SanitizeError> {
self.message.sanitize(require_static_program_ids)?;
self.sanitize_signatures()?; self.sanitize_signatures()?;
Ok(()) Ok(())
} }

View File

@ -1131,13 +1131,7 @@ impl EncodedTransaction {
.and_then(|bytes| bincode::deserialize(&bytes).ok()), .and_then(|bytes| bincode::deserialize(&bytes).ok()),
}; };
transaction.filter(|transaction| { transaction.filter(|transaction| transaction.sanitize().is_ok())
transaction
.sanitize(
true, // require_static_program_ids
)
.is_ok()
})
} }
} }