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

Update fee calculation logic to accept price #672

Merged
merged 35 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
8175147
WIP
MitchTurner Feb 8, 2024
d1975eb
Revert "WIP"
MitchTurner Feb 8, 2024
bf31410
Replace gas price policy with tip, add gas price to transact interface
MitchTurner Feb 8, 2024
90f4729
Update CHANGELOG
MitchTurner Feb 8, 2024
f13e6bb
fmt
MitchTurner Feb 8, 2024
ba0b1eb
WIP update tests
MitchTurner Feb 8, 2024
9cbcadf
WIP
MitchTurner Feb 9, 2024
d1d11c8
Fix run_script to have free gas
MitchTurner Feb 10, 2024
d431e61
Deal with test gas prices that must be zero
MitchTurner Feb 10, 2024
ed7827b
Fix one test
MitchTurner Feb 12, 2024
c49d037
Fix predicate test
MitchTurner Feb 12, 2024
2d307d2
Fix output update test
MitchTurner Feb 12, 2024
d9091fe
Fix snapshot tests and some other gas price issues in tests
MitchTurner Feb 12, 2024
4ecb8fa
Maybe fix test?
MitchTurner Feb 13, 2024
b80bd39
Fix create test
MitchTurner Feb 14, 2024
fe7bd46
Removed todo
xgreenx Feb 14, 2024
3f5d950
Removed todo for tip GTF
xgreenx Feb 14, 2024
b963c6b
Fix
xgreenx Feb 14, 2024
0f756c3
Fix warnings
MitchTurner Feb 14, 2024
0c3c8ff
Make ci check changes
MitchTurner Feb 14, 2024
39a3036
Revert default impl changes
MitchTurner Feb 16, 2024
c548402
Replace gas_price in prop tests
MitchTurner Feb 19, 2024
32e187e
Replace some tip stuff for serialization and snapshots
MitchTurner Feb 19, 2024
f13085a
Merge branch 'master' into use-gas-price-in-fee-calc
MitchTurner Feb 19, 2024
394d89a
Fix function call
MitchTurner Feb 19, 2024
935ba82
Fix tests
MitchTurner Feb 19, 2024
cb6fb93
Add `gas_price` to `InterpreterParams`
MitchTurner Feb 20, 2024
295152c
Remove commented code
MitchTurner Feb 20, 2024
e2a26e8
Fix prop test to generate `gas_price`
MitchTurner Feb 21, 2024
9c1c3d2
Merge branch 'master' into use-gas-price-in-fee-calc
MitchTurner Feb 21, 2024
9dddcfe
Merge branch 'master' into use-gas-price-in-fee-calc
xgreenx Feb 21, 2024
d7546e2
Remove `gas_price` from predicate execution functions
MitchTurner Feb 21, 2024
fd747ba
Fix tests
MitchTurner Feb 21, 2024
81e91b8
Replace encoding stuff for tip
MitchTurner Feb 22, 2024
17cd96e
Replace the other encoding stuff
MitchTurner Feb 22, 2024
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
9 changes: 7 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Changed
#### Breaking
### Breaking

- [#672](https://github.com/FuelLabs/fuel-vm/pull/672): Remove `GasPrice` policy
- [#672](https://github.com/FuelLabs/fuel-vm/pull/672): Add `gas_price` field to transaction execution
- [#1632](https://github.com/FuelLabs/fuel-core/pull/1632): Removed `ContractsInfo` table. Contract salts and roots are no longer stored in on-chain data.
- [#1632](https://github.com/FuelLabs/fuel-core/pull/1632): Opcode `CROO` now calculates the given contract's root on demand. `CROO` has therefore been changed to a `DependentCost` gas cost.

### Changed

- [#672](https://github.com/FuelLabs/fuel-vm/pull/672): Add `Tip` policy

## [Version 0.45.0]

### Changed
Expand Down
4 changes: 2 additions & 2 deletions fuel-asm/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ crate::enum_try_from! {
PolicyTypes = 0x500,

/// Set `$rA` to `tx.policies[0x00].gasPrice`
PolicyGasPrice = 0x501,
PolicyTip = 0x501,
MitchTurner marked this conversation as resolved.
Show resolved Hide resolved

/// Set `$rA` to `tx.policies[count_ones(0b11 & tx.policyTypes) - 1].witnessLimit`
PolicyWitnessLimit = 0x502,
Expand Down Expand Up @@ -362,7 +362,7 @@ fn encode_gtf_args() {
GTFArgs::WitnessDataLength,
GTFArgs::WitnessData,
GTFArgs::PolicyTypes,
GTFArgs::PolicyGasPrice,
GTFArgs::PolicyTip,
GTFArgs::PolicyWitnessLimit,
GTFArgs::PolicyMaturity,
GTFArgs::PolicyMaxFee,
Expand Down
11 changes: 5 additions & 6 deletions fuel-tx/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::{
field::{
BytecodeLength,
BytecodeWitnessIndex,
GasPrice,
Maturity,
Tip,
Witnesses,
},
Chargeable,
Expand Down Expand Up @@ -119,7 +119,7 @@ impl TransactionBuilder<Script> {
script_gas_limit: Default::default(),
script,
script_data,
policies: Policies::new().with_gas_price(0),
policies: Policies::new(),
inputs: Default::default(),
outputs: Default::default(),
witnesses: Default::default(),
Expand Down Expand Up @@ -148,7 +148,7 @@ impl TransactionBuilder<Create> {
bytecode_witness_index: Default::default(),
salt,
storage_slots,
policies: Policies::new().with_gas_price(0),
policies: Policies::new(),
inputs: Default::default(),
outputs: Default::default(),
witnesses: Default::default(),
Expand Down Expand Up @@ -291,9 +291,8 @@ impl<Tx: Buildable> TransactionBuilder<Tx> {
self.sign_keys.keys()
}

pub fn gas_price(&mut self, gas_price: Word) -> &mut Self {
self.tx.set_gas_price(gas_price);

pub fn tip(&mut self, tip: Word) -> &mut Self {
self.tx.set_tip(tip);
self
}

Expand Down
10 changes: 2 additions & 8 deletions fuel-tx/src/tests/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,6 @@ fn transaction() {
fn create_input_data_offset() {
let rng = &mut StdRng::seed_from_u64(8586);

let gas_price = 100;
let maturity = 10.into();
let bytecode_witness_index = 0x00;
let salt = rng.gen();
Expand Down Expand Up @@ -475,9 +474,7 @@ fn create_input_data_offset() {

let tx = Transaction::create(
bytecode_witness_index,
Policies::new()
.with_maturity(maturity)
.with_gas_price(gas_price),
Policies::new().with_maturity(maturity),
salt,
storage_slot.clone(),
inputs,
Expand Down Expand Up @@ -528,7 +525,6 @@ fn create_input_data_offset() {
fn script_input_coin_data_offset() {
let rng = &mut StdRng::seed_from_u64(8586);

let gas_price = 100;
let gas_limit = 1000;
let maturity = 10.into();

Expand Down Expand Up @@ -597,9 +593,7 @@ fn script_input_coin_data_offset() {
gas_limit,
script.clone(),
script_data.clone(),
Policies::new()
.with_maturity(maturity)
.with_gas_price(gas_price),
Policies::new().with_maturity(maturity),
inputs,
outputs.clone(),
witnesses.clone(),
Expand Down
45 changes: 39 additions & 6 deletions fuel-tx/src/tests/valid_cases/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ fn input_coin_message_signature() {
#[test]
fn coin_signed() {
let rng = &mut StdRng::seed_from_u64(8586);
let arb_gas_price = 1;

let mut tx = Script::default();

Expand All @@ -157,7 +158,11 @@ fn coin_signed() {

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

assert_eq!(ValidityError::InputWitnessIndexBounds { index: 0 }, err);
Expand Down Expand Up @@ -333,6 +338,7 @@ fn contract() {
#[test]
fn message_metadata() {
let rng = &mut StdRng::seed_from_u64(8586);
let arb_gas_price = 1;

let txhash: Bytes32 = rng.gen();

Expand Down Expand Up @@ -370,7 +376,11 @@ fn message_metadata() {

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

assert_eq!(ValidityError::InputWitnessIndexBounds { index: 0 }, err,);
Expand Down Expand Up @@ -471,6 +481,8 @@ fn message_metadata() {
fn message_message_coin() {
let rng = &mut StdRng::seed_from_u64(8586);

let arb_gas_price = 1;

let txhash: Bytes32 = rng.gen();

let predicate = generate_nonempty_padded_bytes(rng);
Expand All @@ -495,7 +507,11 @@ fn message_message_coin() {

let block_height = rng.gen();
let err = tx
.check(block_height, &ConsensusParameters::standard())
.check(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a thought.

Hmm, it is the third time when we have updated a lot of places because of the modified number of arguments for the check function=D

Maybe it makes sense to accumulate all arguments inside of one type like CheckArgs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thinking about this more. There are no impls of check and only 4 impls of check_without_signature. I'm not too worried about it. It would be a lot more work rn to create a new type--all the places we call those functions. And those places would still need to be changed in the future.

This also goes back to the work I did a while ago breaking down the params so functions only use what they need. As soon as we start grouping things together again it gets harder to uphold the Interface Segregation Principle.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pretty ambivalent about this one. I agree that it's a smell if we keep changing the interface, but I don't know if it's worth the work changing now.

block_height,
&ConsensusParameters::standard(),
arb_gas_price,
)
.expect_err("Expected failure");

assert_eq!(ValidityError::InputWitnessIndexBounds { index: 0 }, err,);
Expand Down Expand Up @@ -556,6 +572,7 @@ fn message_message_coin() {
fn transaction_with_duplicate_coin_inputs_is_invalid() {
let rng = &mut StdRng::seed_from_u64(8586);
let utxo_id = rng.gen();
let arb_gas_price = 1;

let a = Input::coin_signed(
utxo_id,
Expand All @@ -581,7 +598,11 @@ fn transaction_with_duplicate_coin_inputs_is_invalid() {
.add_input(b)
.add_witness(rng.gen())
.finalize()
.check_without_signatures(Default::default(), &ConsensusParameters::standard())
.check_without_signatures(
Default::default(),
&ConsensusParameters::standard(),
arb_gas_price,
)
.expect_err("Expected checkable failure");

assert_eq!(err, ValidityError::DuplicateInputUtxoId { utxo_id });
Expand All @@ -590,6 +611,7 @@ fn transaction_with_duplicate_coin_inputs_is_invalid() {
#[test]
fn transaction_with_duplicate_message_inputs_is_invalid() {
let rng = &mut StdRng::seed_from_u64(8586);
let arb_gas_price = 1;
let message_input = Input::message_data_signed(
rng.gen(),
rng.gen(),
Expand Down Expand Up @@ -619,6 +641,7 @@ fn transaction_with_duplicate_message_inputs_is_invalid() {
.check_without_signatures(
Default::default(),
&ConsensusParameters::standard(),
arb_gas_price,
)
.expect_err("Expected checkable failure");

Expand All @@ -628,6 +651,7 @@ fn transaction_with_duplicate_message_inputs_is_invalid() {
#[test]
fn transaction_with_duplicate_contract_inputs_is_invalid() {
let rng = &mut StdRng::seed_from_u64(8586);
let arb_gas_price = 1;
let contract_id = rng.gen();
let fee = Input::coin_signed(
rng.gen(),
Expand All @@ -652,7 +676,11 @@ fn transaction_with_duplicate_contract_inputs_is_invalid() {
.add_output(o)
.add_output(p)
.finalize()
.check_without_signatures(Default::default(), &ConsensusParameters::standard())
.check_without_signatures(
Default::default(),
&ConsensusParameters::standard(),
arb_gas_price,
)
.expect_err("Expected checkable failure");

assert_eq!(err, ValidityError::DuplicateInputContractId { contract_id });
Expand All @@ -662,6 +690,7 @@ fn transaction_with_duplicate_contract_inputs_is_invalid() {
fn transaction_with_duplicate_contract_utxo_id_is_valid() {
let rng = &mut StdRng::seed_from_u64(8586);
let input_utxo_id: UtxoId = rng.gen();
let arb_gas_price = 1;

let a = Input::contract(input_utxo_id, rng.gen(), rng.gen(), rng.gen(), rng.gen());
let b = Input::contract(input_utxo_id, rng.gen(), rng.gen(), rng.gen(), rng.gen());
Expand All @@ -686,6 +715,10 @@ fn transaction_with_duplicate_contract_utxo_id_is_valid() {
.add_output(p)
.add_witness(rng.gen())
.finalize()
.check_without_signatures(Default::default(), &ConsensusParameters::standard())
.check_without_signatures(
Default::default(),
&ConsensusParameters::standard(),
arb_gas_price,
)
.expect("Duplicated UTXO id is valid for contract input");
}
Loading
Loading