From f015005aea480d007e32d340d622e91c9e48b2e9 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 28 Sep 2023 12:32:41 +0200 Subject: [PATCH 1/2] Moves signatures verification gas in `Signature` --- benches/host_env.rs | 2 +- core/src/proto/types.rs | 37 +++++++++++++++++++++--------------- core/src/types/key/mod.rs | 2 ++ shared/src/vm/host_env.rs | 2 +- tests/src/vm_host_env/mod.rs | 4 ++-- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/benches/host_env.rs b/benches/host_env.rs index 6f385b93bc..8970c34a93 100644 --- a/benches/host_env.rs +++ b/benches/host_env.rs @@ -35,7 +35,7 @@ fn tx_section_signature_validation(c: &mut Criterion) { c.bench_function("tx_section_signature_validation", |b| { b.iter(|| { multisig - .verify_signature(&mut HashSet::new(), &pkim, &None) + .verify_signature(&mut HashSet::new(), &pkim, &None, &mut None) .unwrap() }) }); diff --git a/core/src/proto/types.rs b/core/src/proto/types.rs index a6082fbbab..e5203293aa 100644 --- a/core/src/proto/types.rs +++ b/core/src/proto/types.rs @@ -553,6 +553,7 @@ impl Signature { verified_pks: &mut HashSet, public_keys_index_map: &AccountPublicKeysMap, signer: &Option
, + gas_meter: &mut Option<&mut VpGasMeter>, ) -> std::result::Result { // Records whether there are any successful verifications let mut verifications = 0; @@ -564,6 +565,11 @@ impl Signature { if let Some(pk) = public_keys_index_map.get_public_key_from_index(*idx) { + if let Some(meter) = gas_meter { + meter + .consume(VERIFY_TX_SIG_GAS_COST) + .map_err(|_| VerifySigError::OutOfGas)?; + } common::SigScheme::verify_signature( &pk, &self.get_raw_hash(), @@ -584,6 +590,11 @@ impl Signature { if let Some(map_idx) = public_keys_index_map.get_index_from_public_key(pk) { + if let Some(meter) = gas_meter { + meter + .consume(VERIFY_TX_SIG_GAS_COST) + .map_err(|_| VerifySigError::OutOfGas)?; + } common::SigScheme::verify_signature( pk, &self.get_raw_hash(), @@ -1381,7 +1392,7 @@ impl Tx { signer: &Option
, threshold: u8, max_signatures: Option, - mut gas_meter: Option<&mut VpGasMeter>, + gas_meter: &mut Option<&mut VpGasMeter>, ) -> std::result::Result, Error> { let max_signatures = max_signatures.unwrap_or(u8::MAX); // Records the public key indices used in successful signatures @@ -1408,26 +1419,22 @@ impl Tx { } // Finally verify that the signature itself is valid - let prev_verifieds = verified_pks.len(); let amt_verifieds = signatures .verify_signature( &mut verified_pks, &public_keys_index_map, signer, + gas_meter, ) - .map_err(|_| { - Error::InvalidSectionSignature( - "found invalid signature.".to_string(), - ) + .map_err(|e| { + if let VerifySigError::OutOfGas = e { + Error::OutOfGas + } else { + Error::InvalidSectionSignature( + "found invalid signature.".to_string(), + ) + } }); - // Compute the cost of the signature verifications - if let Some(x) = gas_meter.as_mut() { - let amt_verified = usize::from(amt_verifieds.is_err()) - + verified_pks.len() - - prev_verifieds; - x.consume(VERIFY_TX_SIG_GAS_COST * amt_verified as u64) - .map_err(|_| Error::OutOfGas)?; - } // Record the section witnessing these signatures if amt_verifieds? > 0 { witnesses.push(signatures); @@ -1458,7 +1465,7 @@ impl Tx { &None, 1, None, - None, + &mut None, ) .map(|x| *x.first().unwrap()) .map_err(|_| Error::InvalidWrapperSignature) diff --git a/core/src/types/key/mod.rs b/core/src/types/key/mod.rs index 1287956b13..d39912299f 100644 --- a/core/src/types/key/mod.rs +++ b/core/src/types/key/mod.rs @@ -123,6 +123,8 @@ pub enum VerifySigError { MissingData, #[error("Signature belongs to a different scheme from the public key.")] MismatchedScheme, + #[error("Signature verification went out of gas")] + OutOfGas, } #[allow(missing_docs)] diff --git a/shared/src/vm/host_env.rs b/shared/src/vm/host_env.rs index 7806d1abef..1479ed7d8f 100644 --- a/shared/src/vm/host_env.rs +++ b/shared/src/vm/host_env.rs @@ -1848,7 +1848,7 @@ where &Some(signer), threshold, max_signatures, - Some(gas_meter), + &mut Some(gas_meter), ) .is_ok(), ) diff --git a/tests/src/vm_host_env/mod.rs b/tests/src/vm_host_env/mod.rs index 68ebd76dff..de24784846 100644 --- a/tests/src/vm_host_env/mod.rs +++ b/tests/src/vm_host_env/mod.rs @@ -483,7 +483,7 @@ mod tests { &None, 1, None, - Some(&mut VpGasMeter::new_from_tx_meter( + &mut Some(&mut VpGasMeter::new_from_tx_meter( &TxGasMeter::new_from_sub_limit(u64::MAX.into()) )) ) @@ -504,7 +504,7 @@ mod tests { &None, 1, None, - Some(&mut VpGasMeter::new_from_tx_meter( + &mut Some(&mut VpGasMeter::new_from_tx_meter( &TxGasMeter::new_from_sub_limit(u64::MAX.into()) )) ) From 6fba21a6f664fd3c4366b13e20fe1716f5a825e1 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 28 Sep 2023 12:36:20 +0200 Subject: [PATCH 2/2] changelog: add #1954 --- .changelog/unreleased/improvements/1954-gas-in-sig-ver.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/1954-gas-in-sig-ver.md diff --git a/.changelog/unreleased/improvements/1954-gas-in-sig-ver.md b/.changelog/unreleased/improvements/1954-gas-in-sig-ver.md new file mode 100644 index 0000000000..27ba173c46 --- /dev/null +++ b/.changelog/unreleased/improvements/1954-gas-in-sig-ver.md @@ -0,0 +1,2 @@ +- Increased resoultion of gas accounting for signature verification. + ([\#1954](https://github.com/anoma/namada/pull/1954)) \ No newline at end of file