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

Added a temporary field used by Receipt::Panic to fill contract_id if any #261

Merged
merged 3 commits into from
Nov 3, 2022
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
19 changes: 17 additions & 2 deletions src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use fuel_tx::{
Receipt, Script, ScriptCheckedMetadata, Transaction, TransactionFee, TransactionRepr, UniqueIdentifier,
};
use fuel_types::bytes::{SerializableVec, SizedBytes};
use fuel_types::{Address, AssetId, Word};
use fuel_types::{Address, AssetId, ContractId, Word};

mod alu;
mod balances;
Expand Down Expand Up @@ -46,7 +46,6 @@ use crate::profiler::InstructionLocation;
pub use balances::RuntimeBalances;
pub use memory::MemoryRange;

#[derive(Debug, Clone)]
/// VM interpreter.
///
/// The internal state of the VM isn't expose because the intended usage is to
Expand All @@ -55,6 +54,7 @@ pub use memory::MemoryRange;
///
/// These can be obtained with the help of a [`crate::transactor::Transactor`]
/// or a client implementation.
#[derive(Debug, Clone)]
pub struct Interpreter<S, Tx = ()> {
registers: [Word; VM_REGISTER_COUNT],
memory: Vec<u8>,
Expand All @@ -69,6 +69,21 @@ pub struct Interpreter<S, Tx = ()> {
#[cfg(feature = "profile-any")]
profiler: Profiler,
params: ConsensusParameters,
/// `PanicContext` after the latest execution. It is consumed by `append_panic_receipt`
/// and is `PanicContext::None` after consumption.
panic_context: PanicContext,
}

/// Sometimes it is possible to add some additional context information
/// regarding panic reasons to simplify debugging.
// TODO: Move this enum into `fuel-tx` and use it inside of the `Receipt::Panic` as meta
// information. Maybe better to have `Vec<PanicContext>` to provide more information.
#[derive(Debug, Clone)]
pub(crate) enum PanicContext {
/// No additional information.
None,
/// `ContractId` retrieved during instruction execution.
ContractId(ContractId),
}

impl<S, Tx> Interpreter<S, Tx> {
Expand Down
12 changes: 5 additions & 7 deletions src/interpreter/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ use crate::error::{Bug, BugId, BugVariant, RuntimeError};
use crate::storage::InterpreterStorage;

use fuel_asm::PanicReason;
use fuel_tx::{Input, Output, Receipt};
use fuel_tx::{Output, Receipt};
use fuel_types::bytes::{self, Deserializable};
use fuel_types::{Address, AssetId, Bytes32, Bytes8, ContractId, RegisterId, Word};

use crate::arith::{add_usize, checked_add_usize, checked_add_word, checked_sub_word};
use crate::interpreter::PanicContext;
use core::slice;

impl<S, Tx> Interpreter<S, Tx>
Expand Down Expand Up @@ -67,6 +68,7 @@ where

// the contract must be declared in the transaction inputs
if !self.transaction().input_contracts().any(|id| id == contract_id) {
self.panic_context = PanicContext::ContractId(contract_id.clone());
return Err(PanicReason::ContractNotInInputs.into());
};

Expand Down Expand Up @@ -157,12 +159,8 @@ where
// Safety: Memory bounds are checked by the interpreter
let contract = unsafe { ContractId::as_ref_unchecked(&self.memory[b..bx]) };

if !self
.transaction()
.inputs()
.iter()
.any(|input| matches!(input, Input::Contract { contract_id, .. } if contract_id == contract))
{
if !self.transaction().input_contracts().any(|input| input == contract) {
self.panic_context = PanicContext::ContractId(contract.clone());
return Err(PanicReason::ContractNotInInputs.into());
}

Expand Down
2 changes: 2 additions & 0 deletions src/interpreter/constructors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use super::{ExecutableTransaction, Interpreter, RuntimeBalances};
use crate::consts::*;
use crate::context::Context;
use crate::interpreter::PanicContext;
use crate::state::Debugger;
use crate::storage::MemoryStorage;

Expand Down Expand Up @@ -35,6 +36,7 @@ where
#[cfg(feature = "profile-any")]
profiler: Profiler::default(),
params,
panic_context: PanicContext::None,
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/interpreter/contract.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::{ExecutableTransaction, Interpreter};
use crate::consts::*;
use crate::error::RuntimeError;
use crate::interpreter::PanicContext;
use crate::storage::InterpreterStorage;

use fuel_asm::{PanicReason, RegisterId, Word};
Expand Down Expand Up @@ -45,6 +46,7 @@ where
let contract = unsafe { ContractId::as_ref_unchecked(&self.memory[c..cx]) };

if !self.transaction().input_contracts().any(|input| contract == input) {
self.panic_context = PanicContext::ContractId(contract.clone());
return Err(PanicReason::ContractNotInInputs.into());
}

Expand Down Expand Up @@ -81,6 +83,7 @@ where
.input_contracts()
.any(|contract| &destination == contract)
{
self.panic_context = PanicContext::ContractId(destination);
return Err(PanicReason::ContractNotInInputs.into());
}

Expand Down
15 changes: 8 additions & 7 deletions src/interpreter/flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::arith;
use crate::call::{Call, CallFrame};
use crate::consts::*;
use crate::error::RuntimeError;
use crate::interpreter::PanicContext;
use crate::state::ProgramState;
use crate::storage::InterpreterStorage;

Expand Down Expand Up @@ -128,16 +129,15 @@ where
let pc = self.registers[REG_PC];
let is = self.registers[REG_IS];

let receipt = Receipt::panic(self.internal_contract_or_default(), result, pc, is);
let mut receipt = Receipt::panic(self.internal_contract_or_default(), result, pc, is);

let receipt = match result.reason() {
PanicReason::ContractNotInInputs => {
let call = Call::try_from(&self.memory[self.registers[result.instruction().ra()] as usize..])
.expect("append panic receipt error");
receipt.with_panic_contract_id(Some(*call.to()))
match self.panic_context {
PanicContext::None => {}
PanicContext::ContractId(contract_id) => {
receipt = receipt.with_panic_contract_id(Some(contract_id));
}
_ => receipt,
};
self.panic_context = PanicContext::None;

self.append_receipt(receipt);
}
Expand Down Expand Up @@ -173,6 +173,7 @@ where
.input_contracts()
.any(|contract| call.to() == contract)
{
self.panic_context = PanicContext::ContractId(call.to().clone());
return Err(PanicReason::ContractNotInInputs.into());
}

Expand Down
28 changes: 26 additions & 2 deletions tests/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use fuel_asm::PanicReason::{
ArithmeticOverflow, ContractNotInInputs, ErrorFlag, ExpectedUnallocatedStack, MemoryOverflow,
};
use fuel_tx::field::{Outputs, Script as ScriptField};
use fuel_vm::util::test_helpers::{check_expected_reason_for_opcodes, check_reason_for_transaction};
use fuel_vm::util::test_helpers::check_expected_reason_for_opcodes;

const SET_STATUS_REG: RegisterId = 0x39;
// log2(VM_MAX_MEM) - used to set a pointer to the memory boundary via SHL: 1<<log2(VM_MAX_MEM)
Expand Down Expand Up @@ -534,7 +534,31 @@ fn ldc_reason_helper(cmd: Vec<Opcode>, expected_reason: PanicReason, should_patc
.expect("failed to check tx");
}

check_reason_for_transaction(client, tx_deploy_loader, expected_reason);
let receipts = client.transact(tx_deploy_loader);
if let Receipt::Panic {
id: _,
reason,
contract_id: actual_contract_id,
..
} = receipts.get(0).expect("No receipt")
{
assert_eq!(
&expected_reason,
reason.reason(),
"Expected {}, found {}",
expected_reason,
reason.reason()
);
match expected_reason {
PanicReason::ContractNotInInputs => {
assert!(actual_contract_id.is_some());
assert_ne!(actual_contract_id, &Some(contract_id));
}
_ => {}
};
} else {
panic!("Script should have panicked");
}
}

#[test]
Expand Down