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

Return TxId when error in checking of transactions #897

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Breaking
- [897](https://github.com/FuelLabs/fuel-vm/pull/897): Return `TxId` when error in checking of transactions. From: `CheckError` to `(TxId, CheckError)`.

### Fixed
- [889](https://github.com/FuelLabs/fuel-vm/pull/889): Debugger breakpoint caused receipts to be produced incorrectly.

Expand Down
14 changes: 8 additions & 6 deletions fuel-tx/src/transaction/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use crate::{
ValidityError,
};

use super::TxId;

/// Entity support metadata computation to cache results.
pub trait Cacheable {
/// The cache is already computed.
Expand All @@ -19,7 +21,7 @@ pub trait Cacheable {
fn is_computed(&self) -> bool;

/// Computes the cache for the entity.
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), ValidityError>;
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), (TxId, ValidityError)>;
}

impl Cacheable for super::Transaction {
Expand All @@ -34,7 +36,7 @@ impl Cacheable for super::Transaction {
}
}

fn precompute(&mut self, chain_id: &ChainId) -> Result<(), ValidityError> {
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), (TxId, ValidityError)> {
match self {
Self::Script(tx) => tx.precompute(chain_id),
Self::Create(tx) => tx.precompute(chain_id),
Expand Down Expand Up @@ -62,7 +64,7 @@ pub struct CommonMetadata {
impl CommonMetadata {
/// Computes the `Metadata` for the `tx` transaction.
/// Returns `None` if the transaction is invalid.
pub fn compute<Tx>(tx: &Tx, chain_id: &ChainId) -> Result<Self, ValidityError>
pub fn compute<Tx>(tx: &Tx, chain_id: &ChainId) -> Result<Self, (TxId, ValidityError)>
where
Tx: UniqueIdentifier,
Tx: field::Inputs,
Expand All @@ -86,7 +88,7 @@ impl CommonMetadata {
let i = offset;
offset = offset
.checked_add(input.size())
.ok_or(ValidityError::SerializedInputTooLarge { index })?;
.ok_or((id, ValidityError::SerializedInputTooLarge { index }))?;
inputs_offset_at.push(i);
}

Expand All @@ -96,7 +98,7 @@ impl CommonMetadata {
let i = offset;
offset = offset
.checked_add(output.size())
.ok_or(ValidityError::SerializedOutputTooLarge { index })?;
.ok_or((id, ValidityError::SerializedOutputTooLarge { index }))?;
outputs_offset_at.push(i);
}

Expand All @@ -106,7 +108,7 @@ impl CommonMetadata {
let i = offset;
offset = offset
.checked_add(witnesses.size())
.ok_or(ValidityError::SerializedWitnessTooLarge { index })?;
.ok_or((id, ValidityError::SerializedWitnessTooLarge { index }))?;
witnesses_offset_at.push(i);
}

Expand Down
3 changes: 2 additions & 1 deletion fuel-tx/src/transaction/types/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::{
Input,
Output,
TransactionRepr,
TxId,
ValidityError,
};
use educe::Educe;
Expand Down Expand Up @@ -171,7 +172,7 @@ impl crate::Cacheable for Blob {
self.metadata.is_some()
}

fn precompute(&mut self, chain_id: &ChainId) -> Result<(), ValidityError> {
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), (TxId, ValidityError)> {
self.metadata = None;
self.metadata = Some(ChargeableMetadata {
common: CommonMetadata::compute(self, chain_id)?,
Expand Down
10 changes: 7 additions & 3 deletions fuel-tx/src/transaction/types/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::{
PrepareSign,
StorageSlot,
TransactionRepr,
TxId,
ValidityError,
};
use educe::Educe;
Expand Down Expand Up @@ -262,11 +263,14 @@ impl crate::Cacheable for Create {
self.metadata.is_some()
}

fn precompute(&mut self, chain_id: &ChainId) -> Result<(), ValidityError> {
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), (TxId, ValidityError)> {
self.metadata = None;
let common_metadata = CommonMetadata::compute(self, chain_id)?;
let body_metadata =
CreateMetadata::compute(self).map_err(|e| (common_metadata.id, e))?;
self.metadata = Some(ChargeableMetadata {
common: CommonMetadata::compute(self, chain_id)?,
body: CreateMetadata::compute(self)?,
common: common_metadata,
body: body_metadata,
});
Ok(())
}
Expand Down
3 changes: 2 additions & 1 deletion fuel-tx/src/transaction/types/mint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::{
},
ConsensusParameters,
TransactionRepr,
TxId,
TxPointer,
ValidityError,
};
Expand Down Expand Up @@ -126,7 +127,7 @@ impl crate::Cacheable for Mint {
self.metadata.is_some()
}

fn precompute(&mut self, chain_id: &ChainId) -> Result<(), ValidityError> {
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), (TxId, ValidityError)> {
self.metadata = None;
self.metadata = Some(MintMetadata::compute(self, chain_id));
Ok(())
Expand Down
3 changes: 2 additions & 1 deletion fuel-tx/src/transaction/types/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use crate::{
GasCosts,
Output,
TransactionRepr,
TxId,
ValidityError,
};
use educe::Educe;
Expand Down Expand Up @@ -210,7 +211,7 @@ impl crate::Cacheable for Script {
self.metadata.is_some()
}

fn precompute(&mut self, chain_id: &ChainId) -> Result<(), ValidityError> {
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), (TxId, ValidityError)> {
self.metadata = None;
self.metadata = Some(ChargeableMetadata {
common: CommonMetadata::compute(self, chain_id)?,
Expand Down
10 changes: 7 additions & 3 deletions fuel-tx/src/transaction/types/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::{
Input,
Output,
TransactionRepr,
TxId,
ValidityError,
};
use educe::Educe;
Expand Down Expand Up @@ -271,11 +272,14 @@ impl crate::Cacheable for Upgrade {
self.metadata.is_some()
}

fn precompute(&mut self, chain_id: &ChainId) -> Result<(), ValidityError> {
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), (TxId, ValidityError)> {
self.metadata = None;
let common_metadata = CommonMetadata::compute(self, chain_id)?;
let body_metadata =
UpgradeMetadata::compute(self).map_err(|e| (common_metadata.id, e))?;
self.metadata = Some(ChargeableMetadata {
common: CommonMetadata::compute(self, chain_id)?,
body: UpgradeMetadata::compute(self)?,
common: common_metadata,
body: body_metadata,
});
Ok(())
}
Expand Down
3 changes: 2 additions & 1 deletion fuel-tx/src/transaction/types/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::{
Input,
Output,
TransactionRepr,
TxId,
ValidityError,
};
use core::ops::Deref;
Expand Down Expand Up @@ -278,7 +279,7 @@ impl crate::Cacheable for Upload {
self.metadata.is_some()
}

fn precompute(&mut self, chain_id: &ChainId) -> Result<(), ValidityError> {
fn precompute(&mut self, chain_id: &ChainId) -> Result<(), (TxId, ValidityError)> {
self.metadata = None;
self.metadata = Some(ChargeableMetadata {
common: CommonMetadata::compute(self, chain_id)?,
Expand Down
32 changes: 20 additions & 12 deletions fuel-vm/src/checked_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ pub enum CheckError {
}

/// Performs checks for a transaction
pub trait IntoChecked: FormatValidityChecks + Sized {
pub trait IntoChecked: FormatValidityChecks + UniqueIdentifier + Sized {
/// Metadata produced during the check.
type Metadata: Sized;

Expand All @@ -309,7 +309,7 @@ pub trait IntoChecked: FormatValidityChecks + Sized {
self,
block_height: BlockHeight,
consensus_params: &ConsensusParameters,
) -> Result<Checked<Self>, CheckError>
) -> Result<Checked<Self>, (TxId, CheckError)>
where
Checked<Self>: CheckPredicates,
{
Expand All @@ -329,22 +329,25 @@ pub trait IntoChecked: FormatValidityChecks + Sized {
consensus_params: &ConsensusParameters,
memory: impl Memory,
storage: &impl PredicateStorageRequirements,
) -> Result<Checked<Self>, CheckError>
) -> Result<Checked<Self>, (TxId, CheckError)>
where
Checked<Self>: CheckPredicates,
{
let check_predicate_params = consensus_params.into();
self.into_checked_basic(block_height, consensus_params)?
.check_signatures(&consensus_params.chain_id())?
let tx = self.into_checked_basic(block_height, consensus_params)?;
let id = tx.id();
tx.check_signatures(&consensus_params.chain_id())
.map_err(|e| (id, e))?
.check_predicates(&check_predicate_params, memory, storage)
.map_err(|e| (id, e))
}

/// Returns transaction that passed only `Checks::Basic`.
fn into_checked_basic(
self,
block_height: BlockHeight,
consensus_params: &ConsensusParameters,
) -> Result<Checked<Self>, CheckError>;
) -> Result<Checked<Self>, (TxId, CheckError)>;
}

/// The parameters needed for checking a predicate
Expand Down Expand Up @@ -868,7 +871,7 @@ impl IntoChecked for Transaction {
self,
block_height: BlockHeight,
consensus_params: &ConsensusParameters,
) -> Result<Checked<Self>, CheckError> {
) -> Result<Checked<Self>, (TxId, CheckError)> {
match self {
Self::Script(tx) => {
let (transaction, metadata) = tx
Expand Down Expand Up @@ -1053,7 +1056,8 @@ mod tests {

let err = tx
.into_checked(Default::default(), &ConsensusParameters::standard())
.expect_err("Expected valid transaction");
.expect_err("Expected valid transaction")
.1;

// then
assert!(matches!(
Expand Down Expand Up @@ -1095,7 +1099,8 @@ mod tests {

let err = tx
.into_checked(Default::default(), &ConsensusParameters::standard())
.expect_err("Expected valid transaction");
.expect_err("Expected valid transaction")
.1;

// then
assert!(matches!(
Expand Down Expand Up @@ -1648,7 +1653,8 @@ mod tests {

let err = tx
.into_checked(Default::default(), &ConsensusParameters::standard())
.expect_err("Expected invalid transaction");
.expect_err("Expected invalid transaction")
.1;

// assert that tx without base input assets fails
assert!(matches!(
Expand Down Expand Up @@ -1737,7 +1743,8 @@ mod tests {
// when
let err = transaction
.into_checked(Default::default(), &consensus_params)
.expect_err("overflow expected");
.expect_err("overflow expected")
.1;

// then
let provided = match err {
Expand Down Expand Up @@ -1914,7 +1921,8 @@ mod tests {

let checked = tx
.into_checked(Default::default(), &ConsensusParameters::standard())
.expect_err("Expected valid transaction");
.expect_err("Expected valid transaction")
.1;

assert_eq!(
CheckError::Validity(ValidityError::InsufficientInputAmount {
Expand Down
Loading
Loading