Skip to content

Commit

Permalink
Include tip in fee (#682)
Browse files Browse the repository at this point in the history
* WIP

* Revert "WIP"

This reverts commit e8ae067.

* Replace gas price policy with tip, add gas price to transact interface

* Update CHANGELOG

* fmt

* WIP update tests

* WIP

* Fix run_script to have free gas

* Deal with test gas prices that must be zero

* Fix one test

* Fix predicate test

* Fix output update test

* Fix snapshot tests and some other gas price issues in tests

* Maybe fix test?

* Fix create test

* Removed todo

* Removed todo for tip GTF

* Fix

* Fix warnings

* Make ci check changes

* Revert default impl changes

* Replace gas_price in prop tests

* Replace some tip stuff for serialization and snapshots

* Fix function call

* Fix tests

* Add `gas_price` to `InterpreterParams`

* Remove commented code

* Fix prop test to generate `gas_price`

* Remove `gas_price` from predicate execution functions

* Fix tests

* Replace encoding stuff for tip

* Replace the other encoding stuff

* Add failing test :)

* Add code for passing test

* Split test

* Update CHANGELOG

* Appease Clippy-sama

* Use public interfaces for UTs

* Update fuel-tx/src/transaction/fee.rs

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>

* Update fuel-tx/src/transaction/fee.rs

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>

* Update fuel-tx/src/transaction/fee.rs

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>

* Fix compilation, move tip into tx methods, include in more prop tests

* fmt

---------

Co-authored-by: xgreenx <xgreenx9999@gmail.com>
  • Loading branch information
MitchTurner and xgreenx authored Feb 26, 2024
1 parent 54110f1 commit a7dc1c0
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Changed
#### Breaking

- [#682](https://github.com/FuelLabs/fuel-vm/pull/682): Include `Tip` policy in fee calculation
- [#683](https://github.com/FuelLabs/fuel-vm/pull/683): Simplify `InterpreterStorage` by removing dependency on `MerkleRootStorage` and removing `merkle_` prefix from method names.
- [#678](https://github.com/FuelLabs/fuel-vm/pull/678): Zero malleable fields before execution. Remove some now-obsolete GTF getters. Don't update `tx.receiptsRoot` after pushing receipts, and do it after execution instead.
- [#672](https://github.com/FuelLabs/fuel-vm/pull/672): Remove `GasPrice` policy
Expand Down
22 changes: 16 additions & 6 deletions fuel-tx/src/transaction/fee.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::{
field,
field::WitnessLimit,
field::{
Tip,
WitnessLimit,
},
input::{
coin::{
CoinPredicate,
Expand All @@ -13,6 +16,7 @@ use crate::{
MessageDataSigned,
},
},
policies::PolicyType,
FeeParameters,
GasCosts,
Input,
Expand Down Expand Up @@ -151,11 +155,13 @@ pub trait Chargeable: field::Inputs + field::Witnesses + field::Policies {
fee: &FeeParameters,
gas_price: Word,
) -> u128 {
gas_to_fee(
let tip = self.tip();
let gas_fee = gas_to_fee(
self.min_gas(gas_costs, fee),
gas_price,
fee.gas_price_factor,
)
);
gas_fee.saturating_add(tip as u128)
}

/// Returns the maximum possible fee after the end of transaction execution.
Expand All @@ -167,11 +173,13 @@ pub trait Chargeable: field::Inputs + field::Witnesses + field::Policies {
fee: &FeeParameters,
gas_price: Word,
) -> u128 {
gas_to_fee(
let tip = self.tip();
let gas_fee = gas_to_fee(
self.max_gas(gas_costs, fee),
gas_price,
fee.gas_price_factor,
)
);
gas_fee.saturating_add(tip as u128)
}

/// Returns the fee amount that can be refunded back based on the `used_gas` and
Expand All @@ -190,7 +198,9 @@ pub trait Chargeable: field::Inputs + field::Witnesses + field::Policies {
let min_gas = self.min_gas(gas_costs, fee);

let total_used_gas = min_gas.saturating_add(used_gas);
let used_fee = gas_to_fee(total_used_gas, gas_price, fee.gas_price_factor);
let tip = self.policies().get(PolicyType::Tip).unwrap_or(0);
let used_fee = gas_to_fee(total_used_gas, gas_price, fee.gas_price_factor)
.saturating_add(tip as u128);

let refund = self
.max_fee(gas_costs, fee, gas_price)
Expand Down
100 changes: 91 additions & 9 deletions fuel-vm/src/checked_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,16 +647,19 @@ impl From<PredicateVerificationFailed> for CheckError {
}

#[cfg(feature = "random")]
#[allow(non_snake_case)]
#[cfg(test)]
mod tests {
#![allow(clippy::cast_possible_truncation)]

use super::*;
use alloc::vec;
use fuel_asm::op;
use fuel_crypto::SecretKey;
use fuel_tx::{
field::{
ScriptGasLimit,
Tip,
WitnessLimit,
Witnesses,
},
Expand Down Expand Up @@ -920,6 +923,7 @@ mod tests {
gas_limit: u64,
input_amount: u64,
gas_price_factor: u64,
tip: u64,
seed: u64,
) -> TestResult {
// dont divide by zero
Expand All @@ -931,7 +935,7 @@ mod tests {
let gas_costs = GasCosts::default();
let fee_params = FeeParameters::DEFAULT.with_gas_price_factor(gas_price_factor);
let base_asset_id = rng.gen();
let tx = predicate_message_coin_tx(rng, gas_limit, input_amount);
let tx = predicate_message_coin_tx(rng, gas_limit, input_amount, tip);

if let Ok(valid) =
is_valid_max_fee(&tx, gas_price, &gas_costs, &fee_params, &base_asset_id)
Expand All @@ -950,6 +954,7 @@ mod tests {
input_amount: u64,
gas_price_factor: u64,
seed: u64,
tip: u64,
) -> TestResult {
// dont divide by zero
if gas_price_factor == 0 {
Expand All @@ -959,7 +964,7 @@ mod tests {
let rng = &mut StdRng::seed_from_u64(seed);
let gas_costs = GasCosts::default();
let fee_params = FeeParameters::DEFAULT.with_gas_price_factor(gas_price_factor);
let tx = predicate_message_coin_tx(rng, gas_limit, input_amount);
let tx = predicate_message_coin_tx(rng, gas_limit, input_amount, tip);

// Given
let used_gas = 0;
Expand All @@ -986,6 +991,7 @@ mod tests {
input_amount: u64,
gas_price: u64,
gas_price_factor: u64,
tip: u64,
seed: u64,
) -> TestResult {
// verify min fee a transaction can consume based on bytes is correct
Expand All @@ -998,7 +1004,7 @@ mod tests {
let gas_costs = GasCosts::default();
let fee_params = FeeParameters::DEFAULT.with_gas_price_factor(gas_price_factor);
let base_asset_id = rng.gen();
let tx = predicate_message_coin_tx(rng, gas_limit, input_amount);
let tx = predicate_message_coin_tx(rng, gas_limit, input_amount, tip);

if let Ok(valid) =
is_valid_min_fee(&tx, &gas_costs, &fee_params, &base_asset_id, gas_price)
Expand Down Expand Up @@ -1467,6 +1473,76 @@ mod tests {
assert_eq!(err, CheckError::Validity(ValidityError::BalanceOverflow));
}

fn arb_tx(rng: &mut StdRng) -> Script {
let input_amount = 1000;
let gas_limit = 1000;
base_asset_tx(rng, input_amount, gas_limit)
}

#[test]
fn into_checked_basic__min_fee_calc_includes_tip() {
let rng = &mut StdRng::seed_from_u64(2322u64);
let gas_price = 1;
let mut tx = arb_tx(rng);

// given
let tipless_tx = tx.clone();

let min_fee_without_tip = tipless_tx
.into_checked_basic(1.into(), &ConsensusParameters::standard(), gas_price)
.unwrap()
.metadata()
.fee
.min_fee();

let tip = 100;

// when
tx.set_tip(tip);

let min_fee_with_tip = tx
.into_checked_basic(1.into(), &ConsensusParameters::standard(), gas_price)
.unwrap()
.metadata()
.fee
.min_fee();

// then
assert_eq!(min_fee_without_tip + tip, min_fee_with_tip);
}

#[test]
fn into_checked_basic__max_fee_calc_includes_tip() {
let rng = &mut StdRng::seed_from_u64(2322u64);
let gas_price = 1;
let mut tx = arb_tx(rng);

// given
let tipless_tx = tx.clone();

let max_fee_without_tip = tipless_tx
.into_checked_basic(1.into(), &ConsensusParameters::standard(), gas_price)
.unwrap()
.metadata()
.fee
.max_fee();

let tip = 100;

// when
tx.set_tip(tip);

let max_fee_with_tip = tx
.into_checked_basic(1.into(), &ConsensusParameters::standard(), gas_price)
.unwrap()
.metadata()
.fee
.max_fee();

// then
assert_eq!(max_fee_without_tip + tip, max_fee_with_tip);
}

#[test]
fn gas_fee_cant_overflow() {
let rng = &mut StdRng::seed_from_u64(2322u64);
Expand Down Expand Up @@ -1527,7 +1603,7 @@ mod tests {
CheckError::Validity(ValidityError::InsufficientInputAmount {
asset: any_asset,
expected: input_amount + 1,
provided: input_amount
provided: input_amount,
}),
checked
);
Expand Down Expand Up @@ -1564,7 +1640,7 @@ mod tests {
.into_checked(
block_height,
&ConsensusParameters::standard_with_id(chain_id),
arb_gas_price
arb_gas_price,
)
.unwrap()
// Sets Checks::Signatures
Expand Down Expand Up @@ -1597,11 +1673,11 @@ mod tests {
.into_checked(
block_height,
&consensus_params,
arb_gas_price
arb_gas_price,
)
.unwrap()
// Sets Checks::Predicates
.check_predicates( &check_predicate_params)
.check_predicates(&check_predicate_params)
.unwrap();
assert!(checked
.checks()
Expand Down Expand Up @@ -1656,7 +1732,9 @@ mod tests {
.try_into()
.map_err(|_| ValidityError::BalanceOverflow)?;

let result = max_fee == available_balances.fee.max_fee();
let max_fee_with_tip = max_fee + tx.tip();

let result = max_fee_with_tip == available_balances.fee.max_fee();
Ok(result)
}

Expand Down Expand Up @@ -1702,7 +1780,9 @@ mod tests {
.try_into()
.map_err(|_| ValidityError::BalanceOverflow)?;

Ok(min_fee == available_balances.fee.min_fee())
let min_fee_with_tip = min_fee + tx.tip();

Ok(min_fee_with_tip == available_balances.fee.min_fee())
}

fn valid_coin_tx(
Expand Down Expand Up @@ -1784,8 +1864,10 @@ mod tests {
rng: &mut StdRng,
gas_limit: u64,
input_amount: u64,
tip: u64,
) -> Script {
TransactionBuilder::script(vec![], vec![])
.tip(tip)
.script_gas_limit(gas_limit)
.add_input(Input::message_coin_predicate(
rng.gen(),
Expand Down

0 comments on commit a7dc1c0

Please sign in to comment.