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

Introduce intrinsic fees for transaction validation beyond script & byte costs #529

Merged
merged 46 commits into from
Dec 12, 2023

Conversation

Voxelot
Copy link
Member

@Voxelot Voxelot commented Oct 26, 2023

closes: #461

Increases the granularity of how the base fee for a transaction is computed. These base fee components are referred to as intrinsic costs, which are intended to cover the overhead of operations such as:

  • read, insert, and removal of transaction outputs
  • witness recovery with secp256k1
  • vm initialization for predicates & scripts
  • contract bytecode bmt (binary merkle tree) root calculation
  • storage slot smt (sparse merkle tree) insertions

@Voxelot Voxelot requested a review from xgreenx October 26, 2023 01:13
@Voxelot Voxelot self-assigned this Oct 26, 2023
Base automatically changed from Voxelot/tx-policies to master October 26, 2023 17:16
@xgreenx xgreenx requested review from xgreenx, MitchTurner and a team December 4, 2023 17:45
Co-authored-by: Hannes Karppila <[email protected]>
Dentosal
Dentosal previously approved these changes Dec 7, 2023
src/protocol/tx-validity.md Outdated Show resolved Hide resolved
src/protocol/tx-validity.md Outdated Show resolved Hide resolved
src/protocol/tx-validity.md Outdated Show resolved Hide resolved

```py
min_gas = min_gas(tx)
max_gas = min_gas + tx.gasLimit
Copy link
Contributor

Choose a reason for hiding this comment

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

The maxWitness - witnessSize(tx) also affects the maximum gas=)

Copy link
Contributor

@bvrooman bvrooman Dec 10, 2023

Choose a reason for hiding this comment

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

I'm a bit confused by how max gas is now calculated.

I see in the trait definition of max_gas() is:

fn max_gas(&self, gas_costs: &GasCosts, fee: &FeeParameters) -> Word {
    let remaining_allowed_witness_gas = self
        .witness_limit()
        .saturating_sub(self.witnesses().size_dynamic() as u64)
        .saturating_mul(fee.gas_per_byte);

    self.min_gas(gas_costs, fee)
        .saturating_add(remaining_allowed_witness_gas)
}

This means:

$MaxGas = MinGas + (WitnessBytesLimit - WitnessBytes) * GasPerByte$

In this case, there's no mention of gas limit.

In some checked_transaction tests (e.g. fee_multiple_signed_inputs):

...
let max_fee = fee.max_fee();
let expected_max_fee = expected_min_fee + gas_limit * gas_price;
assert_eq!(max_fee, expected_max_fee);

This means:

$MaxGas = MinGas + GasLimit$

In this case, there's no mention of witnesses.

Is there a connection between gas limit and witnesses now? I.e., $GasLimit = (WitnessBytesLimit - WitnessBytes) * GasPerByte$
?

Copy link
Contributor

@xgreenx xgreenx Dec 11, 2023

Choose a reason for hiding this comment

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

You are looking into max_gas implementation for Create transaction. The Script transaction implements this method in another place.

image

The final formula looks like:

$MaxGas = MinGas + (WitnessBytesLimit - ActualWitnessBytes) * GasPerByte + GasLimit$

src/protocol/tx-validity.md Outdated Show resolved Hide resolved
src/protocol/tx-validity.md Outdated Show resolved Hide resolved
return fees


def gas_balance(tx) -> int:
Copy link
Contributor

Choose a reason for hiding this comment

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

gas_balance sound strange=) Maybe max_gas would be better?

Copy link
Contributor

@bvrooman bvrooman Dec 10, 2023

Choose a reason for hiding this comment

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

This function (if I understand correctly) is meant to calculate how much the user owes (AKA the balance) after transaction execution. I can update the comment to reflect this (because currently it is misleading).

In contrast, max_gas is the maximum gas that the user is willing to spend.

Edit: It looks like this is meant to be the max_gas, and we are later converting max_gas to a balance (reserved_fee_balance). It sounds like we reserve the max gas. We don't actually specify the formula for the actual cost here (which is fine).

src/protocol/tx-validity.md Outdated Show resolved Hide resolved
src/protocol/tx-validity.md Outdated Show resolved Hide resolved
src/protocol/tx-validity.md Outdated Show resolved Hide resolved
@bvrooman bvrooman merged commit d7e18fc into master Dec 12, 2023
2 checks passed
@bvrooman bvrooman deleted the Voxelot/intrinsic-costs branch December 12, 2023 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "intrinsic" gas cost for coin inputs and other non-execution stuff
5 participants