Merge branch 'grarco/gas-in-sig-ver' (#1954)

* origin/grarco/gas-in-sig-ver:
  changelog: add #1954
  Moves signatures verification gas in `Signature`

.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))

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()
         })
     });

core/src/proto/types.rs

@@ -554,6 +554,7 @@ impl Signature {
         verified_pks: &mut HashSet<u8>,
         public_keys_index_map: &AccountPublicKeysMap,
         signer: &Option<Address>,
+        gas_meter: &mut Option<&mut VpGasMeter>,
     ) -> std::result::Result<u8, VerifySigError> {
         // Records whether there are any successful verifications
         let mut verifications = 0;
@@ -565,6 +566,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(),
@@ -585,6 +591,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(),
@@ -1377,7 +1388,7 @@ impl Tx {
         signer: &Option<Address>,
         threshold: u8,
         max_signatures: Option<u8>,
-        mut gas_meter: Option<&mut VpGasMeter>,
+        gas_meter: &mut Option<&mut VpGasMeter>,
     ) -> std::result::Result<Vec<&Signature>, Error> {
         let max_signatures = max_signatures.unwrap_or(u8::MAX);
         // Records the public key indices used in successful signatures
@@ -1404,26 +1415,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);
@@ -1454,7 +1461,7 @@ impl Tx {
             &None,
             1,
             None,
-            None,
+            &mut None,
         )
         .map(|x| *x.first().unwrap())
         .map_err(|_| Error::InvalidWrapperSignature)

core/src/types/key/mod.rs

@@ -124,6 +124,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)]

shared/src/vm/host_env.rs

@@ -1875,7 +1875,7 @@ where
             &Some(signer),
             threshold,
             max_signatures,
-            Some(gas_meter),
+            &mut Some(gas_meter),
         )
         .is_ok(),
     )

tests/src/vm_host_env/mod.rs

@@ -482,7 +482,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())
                         ))
                     )
@@ -503,7 +503,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())
                         ))
                     )