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

Follow-up clean ups and fixed for the VM initialization #527

Merged
merged 2 commits into from
Jul 26, 2023
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
12 changes: 11 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

- [#514](https://github.com/FuelLabs/fuel-vm/pull/514/): Add `ClientId` and `GasCosts` to `ConsensusParameters`.
### Changed

#### Breaking

- [#514](https://github.com/FuelLabs/fuel-vm/pull/514/): Add `ChainId` and `GasCosts` to `ConsensusParameters`.
Break down `ConsensusParameters` into sub-structs to match usage. Change signatures of functions to ask for
necessary fields only.

### Fixed

#### Breaking

- [#527](https://github.com/FuelLabs/fuel-vm/pull/527): The balances are empty during predicate estimation/verification.

## [Version 0.35.1]

### Added
Expand Down
5 changes: 1 addition & 4 deletions fuel-tx/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,11 @@ impl<Tx> TransactionBuilder<Tx> {
let should_prepare_predicate = false;
let sign_keys = HashMap::new();

// TODO: What is a default chain id?
let chain_id = ChainId::default();

Self {
tx,
should_prepare_script,
should_prepare_predicate,
params: ConsensusParameters::standard(chain_id),
params: ConsensusParameters::standard(),
sign_keys,
}
}
Expand Down
3 changes: 1 addition & 2 deletions fuel-tx/src/tests/offset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,8 +469,7 @@ fn iow_offset() {
let bytes = tx.to_bytes();

let mut tx_p = tx.clone();
let chain_id = ChainId::default();
tx_p.precompute(&chain_id)
tx_p.precompute(&ChainId::default())
.expect("Should be able to calculate cache");

tx.inputs().iter().enumerate().for_each(|(x, i)| {
Expand Down
36 changes: 10 additions & 26 deletions fuel-tx/src/tests/valid_cases/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,7 @@ fn coin_signed() {

let block_height = rng.gen();
let err = tx
.check(
block_height,
&ConsensusParameters::standard(Default::default()),
)
.check(block_height, &ConsensusParameters::standard())
.expect_err("Expected failure");

assert_eq!(CheckError::InputWitnessIndexBounds { index: 0 }, err);
Expand Down Expand Up @@ -212,8 +209,7 @@ fn coin_predicate() {
let txhash: Bytes32 = rng.gen();

let predicate = generate_nonempty_padded_bytes(rng);
let chain_id = ChainId::default();
let owner = Input::predicate_owner(&predicate, &chain_id);
let owner = Input::predicate_owner(&predicate, &ChainId::default());

Input::coin_predicate(
rng.gen(),
Expand All @@ -238,8 +234,7 @@ fn coin_predicate() {
.unwrap();

let predicate = vec![];
let chain_id = ChainId::default();
let owner = Input::predicate_owner(&predicate, &chain_id);
let owner = Input::predicate_owner(&predicate, &ChainId::default());

let err = Input::coin_predicate(
rng.gen(),
Expand Down Expand Up @@ -420,13 +415,12 @@ fn message_metadata() {

let block_height = rng.gen();
let err = tx
.check(block_height, &ConsensusParameters::standard(chain_id))
.check(block_height, &ConsensusParameters::standard())
.expect_err("Expected failure");

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

let mut predicate = generate_nonempty_padded_bytes(rng);
let chain_id = ChainId::default();
let recipient = Input::predicate_owner(&predicate, &chain_id);
predicate[0] = predicate[0].wrapping_add(1);

Expand Down Expand Up @@ -588,14 +582,13 @@ fn message_message_coin() {

let block_height = rng.gen();
let err = tx
.check(block_height, &ConsensusParameters::standard(chain_id))
.check(block_height, &ConsensusParameters::standard())
.expect_err("Expected failure");

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

let mut predicate = generate_nonempty_padded_bytes(rng);
let chain_id = ChainId::default();
let recipient = Input::predicate_owner(&predicate, &chain_id);
let recipient = Input::predicate_owner(&predicate, &ChainId::default());
predicate[0] = predicate[0].wrapping_add(1);

let err = Input::message_coin_predicate(
Expand Down Expand Up @@ -699,10 +692,7 @@ fn transaction_with_duplicate_coin_inputs_is_invalid() {
.add_input(b)
.add_witness(rng.gen())
.finalize()
.check_without_signatures(
Default::default(),
&ConsensusParameters::standard(ChainId::default()),
)
.check_without_signatures(Default::default(), &ConsensusParameters::standard())
.expect_err("Expected checkable failure");

assert_eq!(err, CheckError::DuplicateInputUtxoId { utxo_id });
Expand Down Expand Up @@ -739,7 +729,7 @@ fn transaction_with_duplicate_message_inputs_is_invalid() {
.finalize()
.check_without_signatures(
Default::default(),
&ConsensusParameters::standard(ChainId::default()),
&ConsensusParameters::standard(),
)
.expect_err("Expected checkable failure");

Expand Down Expand Up @@ -773,10 +763,7 @@ fn transaction_with_duplicate_contract_inputs_is_invalid() {
.add_output(o)
.add_output(p)
.finalize()
.check_without_signatures(
Default::default(),
&ConsensusParameters::standard(ChainId::default()),
)
.check_without_signatures(Default::default(), &ConsensusParameters::standard())
.expect_err("Expected checkable failure");

assert_eq!(err, CheckError::DuplicateInputContractId { contract_id });
Expand Down Expand Up @@ -810,9 +797,6 @@ fn transaction_with_duplicate_contract_utxo_id_is_valid() {
.add_output(p)
.add_witness(rng.gen())
.finalize()
.check_without_signatures(
Default::default(),
&ConsensusParameters::standard(ChainId::default()),
)
.check_without_signatures(Default::default(), &ConsensusParameters::standard())
.expect("Duplicated UTXO id is valid for contract input");
}
19 changes: 16 additions & 3 deletions fuel-tx/src/transaction/consensus_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,26 @@ pub struct ConsensusParameters {

impl Default for ConsensusParameters {
fn default() -> Self {
Self::standard(ChainId::default())
Self::standard()
}
}

impl ConsensusParameters {
/// Constructor for the `ConsensusParameters` with Standard values
pub fn standard(chain_id: ChainId) -> Self {
/// Constructor for the `ConsensusParameters` with Standard values.
pub fn standard() -> Self {
Self {
tx_params: TxParameters::DEFAULT,
predicate_params: PredicateParameters::DEFAULT,
script_params: ScriptParameters::DEFAULT,
contract_params: ContractParameters::DEFAULT,
fee_params: FeeParameters::DEFAULT,
chain_id: ChainId::default(),
gas_costs: GasCosts::default(),
}
}

/// Constructor for the `ConsensusParameters` with Standard values around `ChainId`.
pub fn standard_with_id(chain_id: ChainId) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a builder pattern be more idiomatic here? I.e., something like:

let parameters = ConsensusParameters::standard().chain_id(chain_id);

Self {
tx_params: TxParameters::DEFAULT,
predicate_params: PredicateParameters::DEFAULT,
Expand Down
4 changes: 2 additions & 2 deletions fuel-tx/src/transaction/types/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ mod tests {
tx.storage_slots.reverse();

let err = tx
.check(0.into(), &ConsensusParameters::standard(ChainId::default()))
.check(0.into(), &ConsensusParameters::standard())
.expect_err("Expected erroneous transaction");

assert_eq!(CheckError::TransactionCreateStorageSlotOrder, err);
Expand All @@ -952,7 +952,7 @@ mod tests {
)
.add_random_fee_input()
.finalize()
.check(0.into(), &ConsensusParameters::standard(ChainId::default()))
.check(0.into(), &ConsensusParameters::standard())
.expect_err("Expected erroneous transaction");

assert_eq!(CheckError::TransactionCreateStorageSlotOrder, err);
Expand Down
2 changes: 1 addition & 1 deletion fuel-tx/src/transaction/types/script.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{
transaction::{
compute_transaction_id,
consensus_parameters::TxParameters,
field::{
GasLimit,
GasPrice,
Expand Down Expand Up @@ -42,7 +43,6 @@ use fuel_types::{
Word,
};

use crate::transaction::consensus_parameters::TxParameters;
#[cfg(feature = "alloc")]
use alloc::vec::Vec;
#[cfg(feature = "std")]
Expand Down
2 changes: 0 additions & 2 deletions fuel-tx/src/transaction/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,7 @@ impl FormatValidityChecks for Transaction {
pub(crate) fn check_common_part<T>(
tx: &T,
block_height: BlockHeight,
//
tx_params: &TxParameters,
//
predicate_params: &PredicateParameters,
) -> Result<(), CheckError>
where
Expand Down
Loading