Skip to content

Commit 2f87cf9

Browse files
committed
feat: charge explicit gas for batch verification
This won't currently make a difference on mainnet because we only call this from the cron actor (in an implicit message). However, we _will_ need this when we have the miner actor call it directly.
1 parent dd01512 commit 2f87cf9

File tree

2 files changed

+46
-49
lines changed

2 files changed

+46
-49
lines changed

fvm/src/gas/price_list.rs

+21-4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ use crate::kernel::SupportedHashes;
2727
// https://docs.rs/wasmtime/2.0.2/wasmtime/struct.InstanceLimits.html#structfield.table_elements
2828
const TABLE_ELEMENT_SIZE: u32 = 8;
2929

30+
// Parallelism for batch seal verification.
31+
const BATCH_SEAL_PARALLELISM: usize = 8;
32+
3033
/// Create a mapping from enum items to values in a way that guarantees at compile
3134
/// time that we did not miss any member, in any of the prices, even if the enum
3235
/// gets a new member later.
@@ -129,7 +132,10 @@ lazy_static! {
129132
tipset_cid_historical: Gas::new(215_000),
130133

131134
compute_unsealed_sector_cid_base: Gas::new(98647),
132-
verify_seal_base: Gas::new(2000), // TODO revisit potential removal of this
135+
verify_seal_batch: ScalingCost {
136+
flat: Gas::zero(), // TODO: Determine startup overhead.
137+
scale: Gas::new(34721049), // TODO: Maybe re-benchmark?
138+
},
133139

134140
verify_aggregate_seal_per: [
135141
(
@@ -426,7 +432,7 @@ pub struct PriceList {
426432
pub(crate) tipset_cid_historical: Gas,
427433

428434
pub(crate) compute_unsealed_sector_cid_base: Gas,
429-
pub(crate) verify_seal_base: Gas,
435+
pub(crate) verify_seal_batch: ScalingCost,
430436
pub(crate) verify_aggregate_seal_per: HashMap<RegisteredSealProof, Gas>,
431437
pub(crate) verify_aggregate_seal_steps: HashMap<RegisteredSealProof, StepCost>,
432438

@@ -613,9 +619,20 @@ impl PriceList {
613619

614620
/// Returns gas required for seal verification.
615621
#[inline]
616-
pub fn on_verify_seal(&self, _info: &SealVerifyInfo) -> GasCharge {
617-
GasCharge::new("OnVerifySeal", self.verify_seal_base, Zero::zero())
622+
pub fn on_batch_verify_seal(&self, vis: &[SealVerifyInfo]) -> GasCharge {
623+
// Charge based on the expected parallelism, rounding up.
624+
let mut count = vis.len();
625+
let remainder = count % BATCH_SEAL_PARALLELISM;
626+
if remainder > 0 {
627+
count += BATCH_SEAL_PARALLELISM - remainder;
628+
}
629+
GasCharge::new(
630+
"OnBatchVerifySeal",
631+
self.verify_seal_batch.apply(count),
632+
Zero::zero(),
633+
)
618634
}
635+
619636
#[inline]
620637
pub fn on_verify_aggregate_seals(
621638
&self,

fvm/src/kernel/default.rs

+25-45
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ use fvm_shared::sys::out::vm::ContextFlags;
2727
use fvm_shared::{commcid, ActorID};
2828
use lazy_static::lazy_static;
2929
use multihash::MultihashDigest;
30-
use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator};
31-
use rayon::prelude::ParallelDrainRange;
30+
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
3231
use std::io::Write;
3332

3433
use super::blocks::{Block, BlockRegistry};
@@ -575,53 +574,34 @@ where
575574
}
576575

577576
fn batch_verify_seals(&self, vis: &[SealVerifyInfo]) -> Result<Vec<bool>> {
578-
// NOTE: gas has already been charged by the power actor when the batch verify was enqueued.
579-
// Lotus charges "virtual" gas here for tracing only.
580-
let mut items = Vec::new();
581-
for vi in vis {
582-
let t = self
583-
.call_manager
584-
.charge_gas(self.call_manager.price_list().on_verify_seal(vi))?;
585-
items.push((vi, t));
586-
}
587-
log::debug!("batch verify seals start");
588-
let out = items.par_drain(..)
589-
.with_min_len(vis.len() / *NUM_CPUS)
590-
.map(|(seal, timer)| {
591-
let start = GasTimer::start();
592-
let verify_seal_result = std::panic::catch_unwind(|| verify_seal(seal));
593-
let ok = match verify_seal_result {
594-
Ok(res) => {
595-
match res {
596-
Ok(correct) => {
597-
if !correct {
598-
log::debug!(
599-
"seal verify in batch failed (miner: {}) (err: Invalid Seal proof)",
600-
seal.sector_id.miner
601-
);
602-
}
603-
correct // all ok
604-
}
605-
Err(err) => {
606-
log::debug!(
607-
"seal verify in batch failed (miner: {}) (err: {})",
608-
seal.sector_id.miner,
609-
err
610-
);
611-
false
612-
}
613-
}
577+
let t = self
578+
.call_manager
579+
.charge_gas(self.call_manager.price_list().on_batch_verify_seal(vis))?;
580+
581+
// TODO: consider optimizing for one proof (i.e., don't charge a batch overhead?)
582+
let out = vis
583+
.par_iter()
584+
.map(|seal| {
585+
match catch_and_log_panic("verifying seals", || verify_seal(seal)) {
586+
Ok(true) => return true,
587+
Ok(false) => {
588+
log::debug!(
589+
"seal verify in batch failed (miner: {}) (err: Invalid Seal proof)",
590+
seal.sector_id.miner
591+
);
614592
}
615-
Err(e) => {
616-
log::error!("seal verify internal fail (miner: {}) (err: {:?})", seal.sector_id.miner, e);
617-
false
593+
Err(err) => {
594+
log::debug!(
595+
"seal verify in batch failed (miner: {}) (err: {})",
596+
seal.sector_id.miner,
597+
err
598+
);
618599
}
619-
};
620-
timer.stop_with(start);
621-
ok
600+
}
601+
false
622602
})
623603
.collect();
624-
log::debug!("batch verify seals end");
604+
t.stop();
625605
Ok(out)
626606
}
627607

0 commit comments

Comments
 (0)