refactor sig verification gas handling

benches/host_env.rs

@@ -48,7 +48,12 @@ 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, &mut None)
+                .verify_signature(
+                    &mut HashSet::new(),
+                    &pkim,
+                    &None,
+                    &mut || Ok(()),
+                )
                 .unwrap()
         })
     });

core/src/proto/types.rs

@@ -20,7 +20,7 @@ use sha2::{Digest, Sha256};
 use thiserror::Error;
 
 use super::generated::types;
-use crate::ledger::gas::{self, GasMetering, VpGasMeter, VERIFY_TX_SIG_GAS};
+use crate::ledger::gas;
 use crate::ledger::storage::{KeccakHasher, Sha256Hasher, StorageHasher};
 use crate::types::account::AccountPublicKeysMap;
 use crate::types::address::Address;
@@ -552,13 +552,16 @@ impl Signature {
     }
 
     /// Verify that the signature contained in this section is valid
-    pub fn verify_signature(
+    pub fn verify_signature<F>(
         &self,
         verified_pks: &mut HashSet<u8>,
         public_keys_index_map: &AccountPublicKeysMap,
         signer: &Option<Address>,
-        gas_meter: &mut Option<&mut VpGasMeter>,
-    ) -> std::result::Result<u8, VerifySigError> {
+        consume_verify_sig_gas: &mut F,
+    ) -> std::result::Result<u8, VerifySigError>
+    where
+        F: FnMut() -> std::result::Result<(), crate::ledger::gas::Error>,
+    {
         // Records whether there are any successful verifications
         let mut verifications = 0;
         match &self.signer {
@@ -569,11 +572,7 @@ 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)
-                                .map_err(VerifySigError::OutOfGas)?;
-                        }
+                        consume_verify_sig_gas()?;
                         common::SigScheme::verify_signature(
                             &pk,
                             &self.get_raw_hash(),
@@ -594,11 +593,7 @@ 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)
-                                .map_err(VerifySigError::OutOfGas)?;
-                        }
+                        consume_verify_sig_gas()?;
                         common::SigScheme::verify_signature(
                             pk,
                             &self.get_raw_hash(),
@@ -1245,15 +1240,18 @@ impl Tx {
 
     /// Verify that the section with the given hash has been signed by the given
     /// public key
-    pub fn verify_signatures(
+    pub fn verify_signatures<F>(
         &self,
         hashes: &[crate::types::hash::Hash],
         public_keys_index_map: AccountPublicKeysMap,
         signer: &Option<Address>,
         threshold: u8,
         max_signatures: Option<u8>,
-        gas_meter: &mut Option<&mut VpGasMeter>,
-    ) -> std::result::Result<Vec<&Signature>, Error> {
+        mut consume_verify_sig_gas: F,
+    ) -> std::result::Result<Vec<&Signature>, Error>
+    where
+        F: FnMut() -> std::result::Result<(), crate::ledger::gas::Error>,
+    {
         let max_signatures = max_signatures.unwrap_or(u8::MAX);
         // Records the public key indices used in successful signatures
         let mut verified_pks = HashSet::new();
@@ -1284,7 +1282,7 @@ impl Tx {
                             &mut verified_pks,
                             &public_keys_index_map,
                             signer,
-                            gas_meter,
+                            &mut consume_verify_sig_gas,
                         )
                         .map_err(|e| {
                             if let VerifySigError::OutOfGas(inner) = e {
@@ -1314,6 +1312,8 @@ impl Tx {
     /// Verify that the sections with the given hashes have been signed together
     /// by the given public key. I.e. this function looks for one signature that
     /// covers over the given slice of hashes.
+    /// Note that this method doesn't consider gas cost and hence it shouldn't
+    /// be used from txs or VPs.
     pub fn verify_signature(
         &self,
         public_key: &common::PublicKey,
@@ -1325,7 +1325,7 @@ impl Tx {
             &None,
             1,
             None,
-            &mut None,
+            || Ok(()),
         )
         .map(|x| *x.first().unwrap())
         .map_err(|_| Error::InvalidWrapperSignature)

core/src/types/key/mod.rs

@@ -123,7 +123,7 @@ pub enum VerifySigError {
     #[error("Signature belongs to a different scheme from the public key.")]
     MismatchedScheme,
     #[error("Signature verification went out of gas: {0}")]
-    OutOfGas(crate::ledger::gas::Error),
+    OutOfGas(#[from] crate::ledger::gas::Error),
 }
 
 #[allow(missing_docs)]

shared/src/vm/host_env.rs

@@ -1928,7 +1928,7 @@ where
         &Some(signer),
         threshold,
         max_signatures,
-        &mut Some(gas_meter),
+        || gas_meter.consume(gas::VERIFY_TX_SIG_GAS),
     ) {
         Ok(_) => Ok(HostEnvResult::Success.to_i64()),
         Err(err) => match err {
@@ -2100,6 +2100,7 @@ where
         .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?;
 
     let sentinel = unsafe { env.ctx.sentinel.get() };
+    let gas_meter = unsafe { env.ctx.gas_meter.get() };
     tx_charge_gas(env, gas)?;
     let hashes = <[Hash; 1]>::try_from_slice(&hash_list)
         .map_err(TxRuntimeError::EncodingError)?;
@@ -2127,19 +2128,13 @@ where
 
     let tx = unsafe { env.ctx.tx.get() };
 
-    tx_charge_gas(
-        env,
-        gas::VERIFY_TX_SIG_GAS * public_keys_map.idx_to_pk.len() as u64,
-    )?;
     match tx.verify_signatures(
         &hashes,
         public_keys_map,
         &None,
         threshold,
         max_signatures,
-        // This uses VpGasMeter, so we're charging the gas here manually
-        // instead
-        &mut None,
+        || gas_meter.consume(gas::VERIFY_TX_SIG_GAS),
     ) {
         Ok(_) => Ok(HostEnvResult::Success.to_i64()),
         Err(err) => match err {

tests/src/vm_host_env/mod.rs

@@ -36,7 +36,6 @@ mod tests {
     use namada::types::time::DateTimeUtc;
     use namada::types::token::{self, Amount};
     use namada::types::{address, key};
-    use namada_core::ledger::gas::{TxGasMeter, VpGasMeter};
     use namada_core::ledger::ibc::context::transfer_mod::testing::DummyTransferModule;
     use namada_core::ledger::ibc::Error as IbcActionError;
     use namada_test_utils::TestWasms;
@@ -479,9 +478,7 @@ mod tests {
                         &None,
                         1,
                         None,
-                        &mut Some(&mut VpGasMeter::new_from_tx_meter(
-                            &TxGasMeter::new_from_sub_limit(u64::MAX.into())
-                        ))
+                        || Ok(())
                     )
                     .is_ok()
             );
@@ -497,9 +494,7 @@ mod tests {
                         &None,
                         1,
                         None,
-                        &mut Some(&mut VpGasMeter::new_from_tx_meter(
-                            &TxGasMeter::new_from_sub_limit(u64::MAX.into())
-                        ))
+                        || Ok(())
                     )
                     .is_err()
             );