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

allow the verify consensus fault extern to return gasused #330

Merged
merged 2 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions actors/runtime/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use fvm_shared::{ActorID, MethodNum};

pub use self::actor_code::*;
use crate::ActorError;

mod actor_code;

#[cfg(feature = "runtime-wasm")]
Expand Down
5 changes: 3 additions & 2 deletions actors/runtime/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ pub struct ExpectCreateActor {
pub code_id: Cid,
pub actor_id: ActorID,
}

#[derive(Clone, Debug)]
pub struct ExpectedMessage {
pub to: Address,
Expand Down Expand Up @@ -594,8 +595,8 @@ impl Runtime<MemoryBlockstore> for MockRuntime {
.unwrap();

assert!(expected_msg.to == to && expected_msg.method == method && expected_msg.params == params && expected_msg.value == value,
"expectedMessage being sent does not match expectation.\nMessage -\t to: {:?} method: {:?} value: {:?} params: {:?}\nExpected -\t {:?}",
to, method, value, params, expected_msg);
"expectedMessage being sent does not match expectation.\nMessage -\t to: {:?} method: {:?} value: {:?} params: {:?}\nExpected -\t {:?}",
to, method, value, params, expected_msg);

{
let mut balance = self.balance.borrow_mut();
Expand Down
4 changes: 2 additions & 2 deletions fvm/src/externs/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! This module contains the logic to invoke the node by traversing Boundary A.
use fvm_shared::clock::ChainEpoch;
use fvm_shared::consensus::ConsensusFault;
use fvm_shared::consensus::ConsensusFaultWithGas;
use fvm_shared::crypto::randomness::DomainSeparationTag;

pub trait Externs: Rand + Consensus {}
Expand All @@ -14,7 +14,7 @@ pub trait Consensus {
h1: &[u8],
h2: &[u8],
extra: &[u8],
) -> anyhow::Result<Option<ConsensusFault>>;
) -> anyhow::Result<ConsensusFaultWithGas>;
}

/// Randomness provider trait
Expand Down
15 changes: 11 additions & 4 deletions fvm/src/kernel/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use fvm_shared::blockstore::{Blockstore, CborStore};
use fvm_shared::commcid::{
cid_to_data_commitment_v1, cid_to_replica_commitment_v1, data_commitment_v1_to_cid,
};
use fvm_shared::consensus::ConsensusFault;
use fvm_shared::econ::TokenAmount;
use fvm_shared::encoding::{blake2b_256, bytes_32, to_vec, RawBytes};
use fvm_shared::error::ErrorNumber;
Expand Down Expand Up @@ -534,11 +535,17 @@ where

// This syscall cannot be resolved inside the FVM, so we need to traverse
// the node boundary through an extern.
self.call_manager
let ret = self
.call_manager
.externs()
.verify_consensus_fault(h1, h2, extra)
.or_illegal_argument()
.context("fault not verified")
.or_illegal_argument()?;
self.call_manager.charge_gas(GasCharge::new(
"verify_consensus_fault_accesses",
ret.gas_used,
0,
))?;
Ok(ret.fault)
}

fn batch_verify_seals(&mut self, vis: &[SealVerifyInfo]) -> Result<Vec<bool>> {
Expand Down Expand Up @@ -571,7 +578,7 @@ where
false
}
}
},
}
Err(e) => {
log::error!("seal verify internal fail (miner: {}) (err: {:?})", seal.sector_id.miner, e);
false
Expand Down
1 change: 1 addition & 0 deletions fvm/src/kernel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod blocks;
pub mod default;

mod error;

pub use error::{ClassifyResult, Context, ExecutionError, Result, SyscallError};

use crate::call_manager::{CallManager, InvocationResult};
Expand Down
4 changes: 3 additions & 1 deletion fvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ mod test {
struct DummyExterns;

impl Externs for DummyExterns {}

impl Rand for DummyExterns {
fn get_chain_randomness(
&self,
Expand All @@ -87,13 +88,14 @@ mod test {
todo!()
}
}

impl Consensus for DummyExterns {
fn verify_consensus_fault(
&self,
_h1: &[u8],
_h2: &[u8],
_extra: &[u8],
) -> anyhow::Result<Option<fvm_shared::consensus::ConsensusFault>> {
) -> anyhow::Result<fvm_shared::consensus::ConsensusFaultWithGas> {
todo!()
}
}
Expand Down
34 changes: 15 additions & 19 deletions fvm/src/syscalls/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use fvm_shared::piece::PieceInfo;
use fvm_shared::sector::{
AggregateSealVerifyProofAndInfos, RegisteredSealProof, SealVerifyInfo, WindowPoStVerifyInfo,
};
use fvm_shared::ActorID;
use fvm_shared::{sys, ActorID};
use wasmtime::{Caller, Trap};

use super::Context;
Expand Down Expand Up @@ -140,42 +140,38 @@ pub fn verify_post(
/// the "parent grinding fault", in which case it must be the sibling of h1 (same parent tipset) and one of the
/// blocks in the parent of h2 (i.e. h2's grandparent).
///
/// Returns:
/// - The fault type (1-3) or 0 for no fault.
/// - The chain epoch at which the fault happened.
/// - The actor at fault.
pub fn verify_consensus_fault(
mut context: Context<'_, impl Kernel>,
context: Context<'_, impl Kernel>,
h1_off: u32,
h1_len: u32,
h2_off: u32,
h2_len: u32,
extra_off: u32,
extra_len: u32,
) -> Result<(u32, ChainEpoch, ActorID)> {
) -> Result<sys::out::crypto::VerifyConsensusFault> {
let h1 = context.memory.try_slice(h1_off, h1_len)?;
let h2 = context.memory.try_slice(h2_off, h2_len)?;
let extra = context.memory.try_slice(extra_off, extra_len)?;

// TODO the extern should only signal an error in case there was an internal
// interrupting error evaluating the consensus fault. If the outcome is
// "no consensus fault was found", the extern should not error, as doing so
// would interrupt execution via the Trap (at least currently).
let ret = context.kernel.verify_consensus_fault(h1, h2, extra)?;

match ret {
// Consensus fault detected; return the actor as a block.
Some(fault) => Ok((
fault.fault_type as u32,
fault.epoch,
fault
// Consensus fault detected
Some(fault) => Ok(sys::out::crypto::VerifyConsensusFault {
fault: fault.fault_type as u32,
epoch: fault.epoch,
target: fault
.target
.id()
.context("expected a resolved actor address")
.context("kernel returned non-id target address")
.or_fatal()?,
)),
}),
// No consensus fault.
None => Ok((0, 0, 0)),
None => Ok(sys::out::crypto::VerifyConsensusFault {
fault: 0,
epoch: 0,
target: 0,
}),
}
}

Expand Down
8 changes: 4 additions & 4 deletions sdk/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,17 @@ pub fn verify_post(info: &WindowPoStVerifyInfo) -> SyscallResult<bool> {
/// The parameters are all serialized block headers. The third "extra" parameter is consulted only for
/// the "parent grinding fault", in which case it must be the sibling of h1 (same parent tipset) and one of the
/// blocks in the parent of h2 (i.e. h2's grandparent).
/// Returns nil and an error if the headers don't prove a fault.
/// Returns None and an error if the headers don't prove a fault.
#[allow(unused)]
pub fn verify_consensus_fault(
h1: &[u8],
h2: &[u8],
extra: &[u8],
) -> SyscallResult<Option<ConsensusFault>> {
let sys::crypto::out::VerifyConsensusFault {
let fvm_shared::sys::out::crypto::VerifyConsensusFault {
fault,
epoch,
actor,
target,
} = unsafe {
sys::crypto::verify_consensus_fault(
h1.as_ptr(),
Expand All @@ -125,8 +125,8 @@ pub fn verify_consensus_fault(
FromPrimitive::from_u32(fault).expect("received an invalid fault type from the runtime");
Ok(Some(ConsensusFault {
epoch,
target: Address::new_id(target),
fault_type,
target: Address::new_id(actor),
}))
}

Expand Down
12 changes: 1 addition & 11 deletions sdk/src/sys/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,11 @@ super::fvm_syscalls! {
h2_len: u32,
extra_off: *const u8,
extra_len: u32,
) -> Result<self::out::VerifyConsensusFault>;
) -> Result<fvm_shared::sys::out::crypto::VerifyConsensusFault>;

/// Verifies an aggregated batch of sector seal proofs.
pub fn verify_aggregate_seals(agg_off: *const u8, agg_len: u32) -> Result<i32>;

/// Verifies an aggregated batch of sector seal proofs.
pub fn batch_verify_seals(batch_off: *const u8, batch_len: u32, result_off: *const u8) -> Result<()>;
}

/// Module containing multi-value out types of these syscalls.
pub mod out {
#[repr(C)]
pub struct VerifyConsensusFault {
pub fault: u32,
pub epoch: fvm_shared::clock::ChainEpoch,
pub actor: fvm_shared::ActorID,
}
}
10 changes: 10 additions & 0 deletions shared/src/consensus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ pub struct ConsensusFault {
pub fault_type: ConsensusFaultType,
}

/// Result of checking two headers for a consensus fault, with the gas used
/// Only for v14 and earlier
#[derive(Clone, Debug)]
pub struct ConsensusFaultWithGas {
/// The fault.
pub fault: Option<ConsensusFault>,
/// Gas used in checking the fault
pub gas_used: i64,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. There's no real need for a named type here. We can just use (Option<ConsensusFault>, i64).
  2. This doesn't really belong in "shared" (shared is for types shared between actors and the FVM, not the FVM and lotus).


/// Consensus fault types in VM.
#[derive(FromPrimitive, Clone, Copy, Debug)]
#[repr(u8)]
Expand Down
12 changes: 12 additions & 0 deletions shared/src/sys/out.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,15 @@ pub mod send {
pub return_id: BlockId,
}
}

pub mod crypto {
use crate::{ActorID, ChainEpoch};

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct VerifyConsensusFault {
pub fault: u32,
pub epoch: ChainEpoch,
pub target: ActorID,
}
}
4 changes: 2 additions & 2 deletions testing/conformance/src/externs.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use fvm::externs::{Consensus, Externs, Rand};
use fvm_shared::clock::ChainEpoch;
use fvm_shared::consensus::ConsensusFault;
use fvm_shared::consensus::ConsensusFaultWithGas;
use fvm_shared::crypto::randomness::DomainSeparationTag;

use crate::rand::ReplayingRand;
Expand Down Expand Up @@ -49,7 +49,7 @@ impl Consensus for TestExterns {
_h1: &[u8],
_h2: &[u8],
_extra: &[u8],
) -> anyhow::Result<Option<ConsensusFault>> {
) -> anyhow::Result<ConsensusFaultWithGas> {
todo!()
}
}
10 changes: 10 additions & 0 deletions testing/conformance/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ where
self.0.create_actor(code_id, actor_id)
}
}

impl<M, C, K> BlockOps for TestKernel<K>
where
M: Machine,
Expand Down Expand Up @@ -367,6 +368,7 @@ where
self.0.block_get(id)
}
}

impl<M, C, K> CircSupplyOps for TestKernel<K>
where
M: Machine,
Expand All @@ -378,6 +380,7 @@ where
Ok(self.1.circ_supply.clone())
}
}

impl<M, C, K> CryptoOps for TestKernel<K>
where
M: Machine,
Expand Down Expand Up @@ -447,6 +450,7 @@ where
Ok(true)
}
}

impl<M, C, K> DebugOps for TestKernel<K>
where
M: Machine,
Expand All @@ -461,6 +465,7 @@ where
self.0.debug_enabled()
}
}

impl<M, C, K> GasOps for TestKernel<K>
where
M: Machine,
Expand All @@ -471,6 +476,7 @@ where
self.0.charge_gas(name, compute)
}
}

impl<M, C, K> MessageOps for TestKernel<K>
where
M: Machine,
Expand All @@ -493,6 +499,7 @@ where
self.0.msg_value_received()
}
}

impl<M, C, K> NetworkOps for TestKernel<K>
where
M: Machine,
Expand All @@ -511,6 +518,7 @@ where
self.0.network_base_fee()
}
}

impl<M, C, K> RandomnessOps for TestKernel<K>
where
M: Machine,
Expand All @@ -537,6 +545,7 @@ where
.get_randomness_from_beacon(personalization, rand_epoch, entropy)
}
}

impl<M, C, K> SelfOps for TestKernel<K>
where
M: Machine,
Expand All @@ -559,6 +568,7 @@ where
self.0.self_destruct(beneficiary)
}
}

impl<M, C, K> SendOps for TestKernel<K>
where
M: Machine,
Expand Down