Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moves signatures verification gas in Signature #1954

Merged
merged 2 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changelog/unreleased/improvements/1954-gas-in-sig-ver.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Increased resoultion of gas accounting for signature verification.
([\#1954](https://github.com/anoma/namada/pull/1954))
2 changes: 1 addition & 1 deletion benches/host_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
});
Expand Down
37 changes: 22 additions & 15 deletions core/src/proto/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,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;
Expand All @@ -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(),
Expand All @@ -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(),
Expand Down Expand Up @@ -1381,7 +1392,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
Expand All @@ -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);
Expand Down Expand Up @@ -1458,7 +1465,7 @@ impl Tx {
&None,
1,
None,
None,
&mut None,
)
.map(|x| *x.first().unwrap())
.map_err(|_| Error::InvalidWrapperSignature)
Expand Down
2 changes: 2 additions & 0 deletions core/src/types/key/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion shared/src/vm/host_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1848,7 +1848,7 @@ where
&Some(signer),
threshold,
max_signatures,
Some(gas_meter),
&mut Some(gas_meter),
)
.is_ok(),
)
Expand Down
4 changes: 2 additions & 2 deletions tests/src/vm_host_env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
))
)
Expand All @@ -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())
))
)
Expand Down