Remove activated feature that checks tx signature len (#21747)

This commit is contained in:
Justin Starry 2021-12-14 09:23:05 -05:00 committed by GitHub
parent 746869fdac
commit e5476913fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 13 additions and 32 deletions

View File

@ -2037,10 +2037,6 @@ fn verify_transaction(
return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into()); return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into());
} }
if !transaction.verify_signatures_len() {
return Err(RpcCustomError::TransactionSignatureVerificationFailure.into());
}
Ok(()) Ok(())
} }

View File

@ -5264,10 +5264,6 @@ impl Bank {
}) })
}?; }?;
if self.verify_tx_signatures_len_enabled() && !sanitized_tx.verify_signatures_len() {
return Err(TransactionError::SanitizeFailure);
}
if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles
|| verification_mode == TransactionVerificationMode::FullVerification || verification_mode == TransactionVerificationMode::FullVerification
{ {
@ -5752,11 +5748,6 @@ impl Bank {
.is_active(&feature_set::no_overflow_rent_distribution::id()) .is_active(&feature_set::no_overflow_rent_distribution::id())
} }
pub fn verify_tx_signatures_len_enabled(&self) -> bool {
self.feature_set
.is_active(&feature_set::verify_tx_signatures_len::id())
}
pub fn versioned_tx_message_enabled(&self) -> bool { pub fn versioned_tx_message_enabled(&self) -> bool {
self.feature_set self.feature_set
.is_active(&feature_set::versioned_tx_message_enabled::id()) .is_active(&feature_set::versioned_tx_message_enabled::id())
@ -14994,12 +14985,14 @@ pub(crate) mod tests {
Some(TransactionError::SanitizeFailure), Some(TransactionError::SanitizeFailure),
); );
} }
// Too many signatures: Success without feature switch // Too many signatures: Sanitization failure
{ {
let tx = make_transaction(TestCase::AddSignature); let tx = make_transaction(TestCase::AddSignature);
assert!(bank assert_eq!(
.verify_transaction(tx.into(), TransactionVerificationMode::FullVerification) bank.verify_transaction(tx.into(), TransactionVerificationMode::FullVerification)
.is_ok()); .err(),
Some(TransactionError::SanitizeFailure),
);
} }
} }

View File

@ -303,11 +303,6 @@ impl Transaction {
Signature::default() Signature::default()
} }
/// Verify the length of signatures matches the value in the message header
pub fn verify_signatures_len(&self) -> bool {
self.signatures.len() == self.message.header.num_required_signatures as usize
}
/// Verify the transaction and hash its message /// Verify the transaction and hash its message
pub fn verify_and_hash_message(&self) -> Result<Hash> { pub fn verify_and_hash_message(&self) -> Result<Hash> {
let message_bytes = self.message_data(); let message_bytes = self.message_data();

View File

@ -200,11 +200,6 @@ impl SanitizedTransaction {
} }
} }
/// Verify the length of signatures matches the value in the message header
pub fn verify_signatures_len(&self) -> bool {
self.signatures.len() == self.message.header().num_required_signatures as usize
}
/// Verify the transaction signatures /// Verify the transaction signatures
pub fn verify(&self) -> Result<()> { pub fn verify(&self) -> Result<()> {
let message_bytes = self.message_data(); let message_bytes = self.message_data();

View File

@ -12,6 +12,7 @@ use {
transaction::{Result, Transaction, TransactionError}, transaction::{Result, Transaction, TransactionError},
}, },
serde::Serialize, serde::Serialize,
std::cmp::Ordering,
}; };
// NOTE: Serialization-related changes must be paired with the direct read at sigverify. // NOTE: Serialization-related changes must be paired with the direct read at sigverify.
@ -29,11 +30,12 @@ impl Sanitize for VersionedTransaction {
fn sanitize(&self) -> std::result::Result<(), SanitizeError> { fn sanitize(&self) -> std::result::Result<(), SanitizeError> {
self.message.sanitize()?; self.message.sanitize()?;
// Once the "verify_tx_signatures_len" feature is enabled, this may be let num_required_signatures = usize::from(self.message.header().num_required_signatures);
// updated to an equality check. match num_required_signatures.cmp(&self.signatures.len()) {
if usize::from(self.message.header().num_required_signatures) > self.signatures.len() { Ordering::Greater => Err(SanitizeError::IndexOutOfBounds),
return Err(SanitizeError::IndexOutOfBounds); Ordering::Less => Err(SanitizeError::InvalidValue),
} Ordering::Equal => Ok(()),
}?;
// Signatures are verified before message keys are mapped so all signers // Signatures are verified before message keys are mapped so all signers
// must correspond to unmapped keys. // must correspond to unmapped keys.