Skip to content

Commit

Permalink
Return PredicateVerificationFailed error to the user (#628)
Browse files Browse the repository at this point in the history
* Removed subtraction of predicate used gas from the gas limit.

* Updated values of teh GTF

* Implemented `Policies` structure.
Updated transaction validity rules according to policies.
Updated fee calculation and refund logic.

* Applied change for VM.
Fixed all tests.

* Minor nits

* Fix test in `fuel-core`

* Fixed review ocmments

* Update fuel-tx/src/transaction/types/create.rs

Co-authored-by: Mitchell Turner <james.mitchell.turner@gmail.com>

* Fixed comments from the PR.
Updated CHANGELOG.md
Simplified `refund` calculation logic

* Added a test for refund

* Apply suggestions from code review

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>

* Renamed `GasLimit` to `ScriptGasLimit`

* Update CHANGELOG.md

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>

* Update CHANGELOG.md

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>

* Return `PredicateVerificationFailed` error to the user

* Renamed `gas_limit` to `script_gas_limit`

* Renamed `gas_limit` to `script_gas_limit`

* Use correct values for policies GTF

* Update CHANGELOG.md

* Address comments

* Fix CI

---------

Co-authored-by: Mitchell Turner <james.mitchell.turner@gmail.com>
Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
  • Loading branch information
3 people authored Nov 9, 2023
1 parent 694b636 commit 9c75006
Show file tree
Hide file tree
Showing 24 changed files with 258 additions and 225 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
#### Breaking

- [#629](https://github.com/FuelLabs/fuel-vm/pull/629): Charge the user for VM initialization.
- [#628](https://github.com/FuelLabs/fuel-vm/pull/628): Renamed `transaction::CheckError` to `transaction::ValidityError`.
Created a new `checked_transaction::CheckError` that combines `ValidityError`
and `PredicateVerificationFailed` errors into one. It allows the return of the
`PredicateVerificationFailed` to the end user instead of losing the reason why predicate verification failed.
- [#623](https://github.com/FuelLabs/fuel-vm/pull/623):
Added support for transaction policies. The `Script` and `Create`
transactions received a new field, `policies`. Policies allow the addition
Expand Down
6 changes: 3 additions & 3 deletions fuel-tx/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
CheckError,
StorageSlot,
Transaction,
ValidityError,
};

use derivative::Derivative;
Expand Down Expand Up @@ -172,12 +172,12 @@ impl AsMut<[u8]> for Contract {
}

impl TryFrom<&Transaction> for Contract {
type Error = CheckError;
type Error = ValidityError;

fn try_from(tx: &Transaction) -> Result<Self, Self::Error> {
match tx {
Transaction::Create(create) => TryFrom::try_from(create),
_ => Err(CheckError::TransactionScriptOutputContractCreated { index: 0 }),
_ => Err(ValidityError::TransactionScriptOutputContractCreated { index: 0 }),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion fuel-tx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ pub use transaction::{
policies,
Cacheable,
Chargeable,
CheckError,
ConsensusParameters,
ContractParameters,
Create,
Expand All @@ -98,6 +97,7 @@ pub use transaction::{
TxId,
TxParameters,
UtxoId,
ValidityError,
Witness,
};

Expand Down
42 changes: 21 additions & 21 deletions fuel-tx/src/tests/valid_cases/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn input_coin_message_signature() {
fn test<Tx: Buildable>(txs: &mut impl Iterator<Item = (Tx, Vec<SecretKey>)>) {
let rng = &mut StdRng::seed_from_u64(8586);

fn check_inputs<Tx: Buildable>(tx: Tx) -> Result<(), CheckError> {
fn check_inputs<Tx: Buildable>(tx: Tx) -> Result<(), ValidityError> {
let chain_id = ChainId::default();
let txhash = tx.id(&chain_id);
let outputs = tx.outputs();
Expand Down Expand Up @@ -57,7 +57,7 @@ fn input_coin_message_signature() {
rng: &mut R,
mut iter: I,
f: F,
) -> Result<(), CheckError>
) -> Result<(), ValidityError>
where
R: Rng + CryptoRng,
I: Iterator<Item = (Tx, Vec<SecretKey>)>,
Expand Down Expand Up @@ -160,7 +160,7 @@ fn coin_signed() {
.check(block_height, &ConsensusParameters::standard())
.expect_err("Expected failure");

assert_eq!(CheckError::InputWitnessIndexBounds { index: 0 }, err);
assert_eq!(ValidityError::InputWitnessIndexBounds { index: 0 }, err);
}

#[test]
Expand Down Expand Up @@ -244,7 +244,7 @@ fn coin_predicate() {
.err()
.unwrap();

assert_eq!(CheckError::InputPredicateEmpty { index: 1 }, err);
assert_eq!(ValidityError::InputPredicateEmpty { index: 1 }, err);

let mut predicate = generate_nonempty_padded_bytes(rng);
let owner = (*Contract::root_from_code(&predicate)).into();
Expand All @@ -265,7 +265,7 @@ fn coin_predicate() {
.err()
.unwrap();

assert_eq!(CheckError::InputPredicateOwner { index: 1 }, err);
assert_eq!(ValidityError::InputPredicateOwner { index: 1 }, err);
}

#[test]
Expand All @@ -291,7 +291,7 @@ fn contract() {
.unwrap();

assert_eq!(
CheckError::InputContractAssociatedOutputContract { index: 1 },
ValidityError::InputContractAssociatedOutputContract { index: 1 },
err
);

Expand All @@ -308,7 +308,7 @@ fn contract() {
.unwrap();

assert_eq!(
CheckError::InputContractAssociatedOutputContract { index: 1 },
ValidityError::InputContractAssociatedOutputContract { index: 1 },
err
);

Expand All @@ -325,7 +325,7 @@ fn contract() {
.unwrap();

assert_eq!(
CheckError::InputContractAssociatedOutputContract { index: 1 },
ValidityError::InputContractAssociatedOutputContract { index: 1 },
err
);
}
Expand Down Expand Up @@ -373,7 +373,7 @@ fn message_metadata() {
.check(block_height, &ConsensusParameters::standard())
.expect_err("Expected failure");

assert_eq!(CheckError::InputWitnessIndexBounds { index: 0 }, err,);
assert_eq!(ValidityError::InputWitnessIndexBounds { index: 0 }, err,);

let mut predicate = generate_nonempty_padded_bytes(rng);
let recipient = Input::predicate_owner(&predicate);
Expand All @@ -392,7 +392,7 @@ fn message_metadata() {
.check(1, &txhash, &[], &[], &Default::default(), &mut None)
.expect_err("Expected failure");

assert_eq!(CheckError::InputPredicateOwner { index: 1 }, err);
assert_eq!(ValidityError::InputPredicateOwner { index: 1 }, err);

let data = vec![0xff; PREDICATE_PARAMS.max_message_data_length as usize + 1];

Expand All @@ -414,7 +414,7 @@ fn message_metadata() {
)
.expect_err("expected max data length error");

assert_eq!(CheckError::InputMessageDataLength { index: 1 }, err,);
assert_eq!(ValidityError::InputMessageDataLength { index: 1 }, err,);

let err = Input::message_data_predicate(
rng.gen(),
Expand All @@ -429,7 +429,7 @@ fn message_metadata() {
.check(1, &txhash, &[], &[], &Default::default(), &mut None)
.expect_err("expected max data length error");

assert_eq!(CheckError::InputMessageDataLength { index: 1 }, err,);
assert_eq!(ValidityError::InputMessageDataLength { index: 1 }, err,);

let predicate = vec![0xff; PREDICATE_PARAMS.max_predicate_length as usize + 1];

Expand All @@ -446,7 +446,7 @@ fn message_metadata() {
.check(1, &txhash, &[], &[], &Default::default(), &mut None)
.expect_err("expected max predicate length error");

assert_eq!(CheckError::InputPredicateLength { index: 1 }, err,);
assert_eq!(ValidityError::InputPredicateLength { index: 1 }, err,);

let predicate_data =
vec![0xff; PREDICATE_PARAMS.max_predicate_data_length as usize + 1];
Expand All @@ -464,7 +464,7 @@ fn message_metadata() {
.check(1, &txhash, &[], &[], &Default::default(), &mut None)
.expect_err("expected max predicate data length error");

assert_eq!(CheckError::InputPredicateDataLength { index: 1 }, err,);
assert_eq!(ValidityError::InputPredicateDataLength { index: 1 }, err,);
}

#[test]
Expand Down Expand Up @@ -498,7 +498,7 @@ fn message_message_coin() {
.check(block_height, &ConsensusParameters::standard())
.expect_err("Expected failure");

assert_eq!(CheckError::InputWitnessIndexBounds { index: 0 }, err,);
assert_eq!(ValidityError::InputWitnessIndexBounds { index: 0 }, err,);

let mut predicate = generate_nonempty_padded_bytes(rng);
let recipient = Input::predicate_owner(&predicate);
Expand All @@ -516,7 +516,7 @@ fn message_message_coin() {
.check(1, &txhash, &[], &[], &Default::default(), &mut None)
.expect_err("Expected failure");

assert_eq!(CheckError::InputPredicateOwner { index: 1 }, err);
assert_eq!(ValidityError::InputPredicateOwner { index: 1 }, err);

let predicate = vec![0xff; PREDICATE_PARAMS.max_predicate_length as usize + 1];

Expand All @@ -532,7 +532,7 @@ fn message_message_coin() {
.check(1, &txhash, &[], &[], &Default::default(), &mut None)
.expect_err("expected max predicate length error");

assert_eq!(CheckError::InputPredicateLength { index: 1 }, err,);
assert_eq!(ValidityError::InputPredicateLength { index: 1 }, err,);

let predicate_data =
vec![0xff; PREDICATE_PARAMS.max_predicate_data_length as usize + 1];
Expand All @@ -549,7 +549,7 @@ fn message_message_coin() {
.check(1, &txhash, &[], &[], &Default::default(), &mut None)
.expect_err("expected max predicate data length error");

assert_eq!(CheckError::InputPredicateDataLength { index: 1 }, err,);
assert_eq!(ValidityError::InputPredicateDataLength { index: 1 }, err,);
}

#[test]
Expand Down Expand Up @@ -584,7 +584,7 @@ fn transaction_with_duplicate_coin_inputs_is_invalid() {
.check_without_signatures(Default::default(), &ConsensusParameters::standard())
.expect_err("Expected checkable failure");

assert_eq!(err, CheckError::DuplicateInputUtxoId { utxo_id });
assert_eq!(err, ValidityError::DuplicateInputUtxoId { utxo_id });
}

#[test]
Expand Down Expand Up @@ -622,7 +622,7 @@ fn transaction_with_duplicate_message_inputs_is_invalid() {
)
.expect_err("Expected checkable failure");

assert_eq!(err, CheckError::DuplicateMessageInputId { message_id });
assert_eq!(err, ValidityError::DuplicateMessageInputId { message_id });
}

#[test]
Expand Down Expand Up @@ -655,7 +655,7 @@ fn transaction_with_duplicate_contract_inputs_is_invalid() {
.check_without_signatures(Default::default(), &ConsensusParameters::standard())
.expect_err("Expected checkable failure");

assert_eq!(err, CheckError::DuplicateInputContractId { contract_id });
assert_eq!(err, ValidityError::DuplicateInputContractId { contract_id });
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions fuel-tx/src/tests/valid_cases/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn contract() {
.err()
.unwrap();

assert_eq!(CheckError::OutputContractInputIndex { index: 2 }, err);
assert_eq!(ValidityError::OutputContractInputIndex { index: 2 }, err);

let err = Output::contract(2, rng.gen(), rng.gen())
.check(
Expand All @@ -79,7 +79,7 @@ fn contract() {
.err()
.unwrap();

assert_eq!(CheckError::OutputContractInputIndex { index: 2 }, err);
assert_eq!(ValidityError::OutputContractInputIndex { index: 2 }, err);
}

#[test]
Expand Down
Loading

0 comments on commit 9c75006

Please sign in to comment.