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.

Sign up for GitHub

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

Changes for predicate gas metering #448

Merged
36 commits merged into from
Mar 6, 2023
Merged

Changes for predicate gas metering #448

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
192182d
predicate gas metering changes
possible-panther Jan 17, 2023
ba67f10
add details for vm initialization for remaining gas and detail gas ex…
possible-panther Jan 24, 2023
5dc89a8
math addition
possible-panther Jan 24, 2023
a1c74f6
remove errant symbols
possible-panther Jan 24, 2023
a48a01e
fix lint issue
possible-panther Jan 24, 2023
28f65ce
Update src/vm/index.md
Voxelot Jan 26, 2023
f4a249b
Merge branch 'master' into mitch/predicate-gas-metering
possible-panther Feb 16, 2023
2a6660a
remove math in favor of pseudocode for predicate remaining gas cmputa…
possible-panther Feb 16, 2023
2a77c61
change return false to panic with out of gas for predicate pseudocode
possible-panther Feb 16, 2023
69aeabc
predicate spec updates for gasUsed
possible-panther Feb 20, 2023
1dd020c
updates for setting ggas/cgas
possible-panther Feb 20, 2023
3bb55b4
remove question about setting to zero
possible-panther Feb 20, 2023
9b568f8
add predicate estimation
possible-panther Feb 22, 2023
50919fb
Merge branch 'master' into mitch/predicate-gas-metering
Voxelot Feb 24, 2023
f8a432d
Update constants.md
Voxelot Feb 24, 2023
60429a9
Change cgas to ggas for gas used comparison
Feb 24, 2023
150118e
remove gtf for predicate gas used, and increase precision of language…
possible-panther Feb 27, 2023
c69d49a
Merge branch 'mitch/predicate-gas-metering' of github.aaakk.us.kg-fuel:FuelLa…
possible-panther Feb 27, 2023
8fa27e9
Merge branch 'master' into mitch/predicate-gas-metering
Voxelot Feb 27, 2023
0e0c427
merge from master
Voxelot Feb 27, 2023
6457457
update indices for GTF adding gasUsed for predicate coin/message
possible-panther Feb 27, 2023
ef45cbe
predicate gas used spec updates
possible-panther Feb 27, 2023
6f9c9e6
Merge branch 'master' into mitch/predicate-gas-metering
possible-panther Feb 27, 2023
9d4ad6b
add relevant text to allowed predicate operations for estimation
possible-panther Feb 27, 2023
bce51d3
Update src/vm/index.md
Feb 27, 2023
130d345
remove restriction on jumps
possible-panther Feb 27, 2023
17150f2
Merge branch 'mitch/predicate-gas-metering' of github.aaakk.us.kg-fuel:FuelLa…
possible-panther Feb 27, 2023
cb85d59
use predicate gasUsed vs tx.gasLimit
possible-panther Feb 27, 2023
941dd11
require exact gas usage in predicate verification
possible-panther Feb 27, 2023
f7883b7
use predicateGasUsed over predicate.GasUsed, specify contexts where p…
possible-panther Feb 27, 2023
f9f941c
Merge branch 'master' into mitch/predicate-gas-metering
xgreenx Feb 27, 2023
908cce7
replace gasUsed with predicateGasUsed
possible-panther Feb 28, 2023
994e528
Update src/protocol/tx_format/transaction.md
Voxelot Feb 28, 2023
d2cabd4
Merge branch 'master' into mitch/predicate-gas-metering
Voxelot Mar 1, 2023
13acab2
Merge branch 'master' into mitch/predicate-gas-metering
possible-panther Mar 1, 2023
fb4d390
update reference to list containing 1 item to statement describing be…
possible-panther Mar 1, 2023
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
64 changes: 32 additions & 32 deletions src/protocol/tx_format/transaction.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,22 @@ enum ReceiptType : uint8 {
}
```

| name | type | description |
|--------------------|-----------------------------|------------------------------------------|
| `gasPrice` | `uint64` | Gas price for transaction. |
| `gasLimit` | `uint64` | Gas limit for transaction. |
| `maturity` | `uint32` | Block until which tx cannot be included. |
| `scriptLength` | `uint16` | Script length, in instructions. |
| `scriptDataLength` | `uint16` | Length of script input data, in bytes. |
| `inputsCount` | `uint8` | Number of inputs. |
| `outputsCount` | `uint8` | Number of outputs. |
| `witnessesCount` | `uint8` | Number of witnesses. |
| `receiptsRoot` | `byte[32]` | Merkle root of receipts. |
| `script` | `byte[]` | Script to execute. |
| `scriptData` | `byte[]` | Script input data (parameters). |
| `inputs` | [Input](./input.md)`[]` | List of inputs. |
| `outputs` | [Output](./output.md)`[]` | List of outputs. |
| `witnesses` | [Witness](./witness.md)`[]` | List of witnesses. |
| name | type | description |
|--------------------|-----------------------------|------------------------------------------------------|
| `gasPrice` | `uint64` | Gas price for transaction. |
| `gasLimit` | `uint64` | Gas limit for transaction (including predicate gas). |
| `maturity` | `uint32` | Block until which tx cannot be included. |
| `scriptLength` | `uint16` | Script length, in instructions. |
| `scriptDataLength` | `uint16` | Length of script input data, in bytes. |
| `inputsCount` | `uint8` | Number of inputs. |
| `outputsCount` | `uint8` | Number of outputs. |
| `witnessesCount` | `uint8` | Number of witnesses. |
| `receiptsRoot` | `byte[32]` | Merkle root of receipts. |
| `script` | `byte[]` | Script to execute. |
| `scriptData` | `byte[]` | Script input data (parameters). |
| `inputs` | [Input](./input.md)`[]` | List of inputs. |
| `outputs` | [Output](./output.md)`[]` | List of outputs. |
| `witnesses` | [Witness](./witness.md)`[]` | List of witnesses. |

Given helper `len()` that returns the number of bytes of a field.

Expand All @@ -91,22 +91,22 @@ The receipts root `receiptsRoot` is the root of the [binary Merkle tree](../cryp

## TransactionCreate

| name | type | description |
|------------------------|-----------------------------|---------------------------------------------------|
| `gasPrice` | `uint64` | Gas price for transaction. |
| `gasLimit` | `uint64` | Gas limit for transaction. |
| `maturity` | `uint32` | Block until which tx cannot be included. |
| `bytecodeLength` | `uint16` | Contract bytecode length, in instructions. |
| `bytecodeWitnessIndex` | `uint8` | Witness index of contract bytecode to create. |
| `storageSlotsCount` | `uint16` | Number of storage slots to initialize. |
| `inputsCount` | `uint8` | Number of inputs. |
| `outputsCount` | `uint8` | Number of outputs. |
| `witnessesCount` | `uint8` | Number of witnesses. |
| `salt` | `byte[32]` | Salt. |
| `storageSlots` | `(byte[32], byte[32]])[]` | List of storage slots to initialize (key, value). |
| `inputs` | [Input](./input.md)`[]` | List of inputs. |
| `outputs` | [Output](./output.md)`[]` | List of outputs. |
| `witnesses` | [Witness](./witness.md)`[]` | List of witnesses. |
| name | type | description |
|------------------------|-----------------------------|------------------------------------------------------|
| `gasPrice` | `uint64` | Gas price for transaction. |
| `gasLimit` | `uint64` | Gas limit for transaction (including predicate gas). |
| `maturity` | `uint32` | Block until which tx cannot be included. |
| `bytecodeLength` | `uint16` | Contract bytecode length, in instructions. |
| `bytecodeWitnessIndex` | `uint8` | Witness index of contract bytecode to create. |
| `storageSlotsCount` | `uint16` | Number of storage slots to initialize. |
| `inputsCount` | `uint8` | Number of inputs. |
| `outputsCount` | `uint8` | Number of outputs. |
| `witnessesCount` | `uint8` | Number of witnesses. |
| `salt` | `byte[32]` | Salt. |
| `storageSlots` | `(byte[32], byte[32]])[]` | List of storage slots to initialize (key, value). |
| `inputs` | [Input](./input.md)`[]` | List of inputs. |
| `outputs` | [Output](./output.md)`[]` | List of outputs. |
| `witnesses` | [Witness](./witness.md)`[]` | List of witnesses. |

Transaction is invalid if:

Expand Down
4 changes: 0 additions & 4 deletions src/protocol/tx_validity.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ def available_balance(tx, asset_id) -> int:
return availableBalance

def unavailable_balance(tx, asset_id) -> int:
"""
Note: we don't charge for predicate verification because predicates are
monotonic and the cost of bytes should approximately makes up for this.
"""
sentBalance = sum_outputs(tx, col)
gasBalance = gasPrice * gasLimit / GAS_PRICE_FACTOR
# Size excludes witness data as it is malleable (even by third parties!)
Expand Down
11 changes: 9 additions & 2 deletions src/vm/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,14 @@ For any input of type [`InputType.Coin`](../protocol/tx_format/index.md), a non-

For each such input in the transaction, the VM is [initialized](#vm-initialization), then:

1. `$pc` and `$is` are set to the start of the input's `predicate` field.
1. `$pc` and `$is` are set to the start of the input's `predicate` field.
Voxelot marked this conversation as resolved.
Show resolved Hide resolved
1. `$ggas` and `$cgas` are set to the lower of `tx.gasLimit` or the remaining gas following the previous predicate execution.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm. There's a few issues with this:

  1. It creates a sequential dependence on lower-position predicates. Without any prior information on how much gas is used by each predicate, this requires predicates be verified sequentially.
  2. Even with prior information, this change makes predicates no longer position-independent. Which can make partially-signed transactions more complex to implement.

Overall, I'd prefer something like a fixed max gas for each predicate, independently. And this max gas can be a consensus rule.

If the gas schedule is changed, then predicates may become invalid. But that's the case even if we allow users to define the max gas since there's a hard cap in the form of the block gas limit.

Copy link
Member

@Voxelot Voxelot Feb 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Good points. Having a per predicate gas limit would be better for parallelization. Would there be both a user provided per-predicate gasLimit + consensus rule max gas per predicate? Or would each predicate always assume it has the consensus based max gas, potentially locking up more funds than needed for execution?

The main benefits of using the tx-level gas limit as opposed to predicate level is:

  1. UX simplicity - it is easier for the sdk to estimate the tx gas limit regardless of changes to the gas schedule
  2. Smaller tx payload, as each new field increases our calldata expense / bandwidth
  3. Users only need to lock up the minimal amount of necessary funds to pay for execution, potentially allowing for more user-side parallelism by minimizing gas lockup overhead (assuming there's no user configurable per-predicate gas limit)

Some ways to do parallel execution with this approach could be:

  • Simply execute all predicates in parallel using the same global tx.gasLimit for each one. Total up the amount of gas each predicate consumed and fail the verification after execution if they ran over the limit. In the best case this doesn't add any overhead and keeps the predicate UX simpler. In the worst case the VM may spend more time executing instructions than it should.
    • The main risk with this approach is if the txpool would do extra work, it's possible it could be overloaded more easily. However, since it has to evaluate potentially invalid predicates anyways without being able to collect any fees, it's hard to say if this really worsens the situation for the txpool.
    • The other risk would be during BFT consensus. If a block proposed for the next round exceeded the allowed execution budget it could cause a lag in voting (although it would have to be some very large predicates at the very end of the block). However, given BFT only supports a small validator set, proposing a bad block which exceeds the execution budget could be a punishable/slashable offence which would deter this from happening.
  • Use an atomic counter external to the cgas/ggas registers to track the total gas usage across multiple VM instances. However, this would add execution overhead even in the best case scenario as charging gas for predicates would require an additional CAS operation.

Copy link
Member

Choose a reason for hiding this comment

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

We had a call with @FuelLabs/client and @adlerjohn to discuss this in more detail - summarizing the notes here:

  • Sharing a global gasLimit prevents parallelization between predicates and script execution. We need to know how much gas predicates can use in advance, otherwise if there's an overage we would lose determinism as to whether the script status is reverted or if the predicates are invalid, thus invalidating the entire block.
  • We will add a new malleable field to predicates called gasUsed. This avoids the need to have predicate amounts cover a global gas limit per predicate which could create a lot of dust.
  • Each predicate will have cgas and ggas always initialized to the tx.gasLimit
  • If predicate.gasUsed is set, the vm will ensure the current gas consumption doesn't exceed that (resolving potential DOS concerns)
  • predicate.gasUsed will not be signed over, but the mempool will expect it to be set correctly by the user
  • The user should be able to estimate the gas usage of each predicate via a single tx dry-run
  • predicate.gasUsed is zeroed out on the tx during predicate verification

Copy link
Contributor

Choose a reason for hiding this comment

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

The user should be able to estimate the gas usage of each predicate via a single tx dry-run

And we can execute the predicate without a dry-run on the SDK side potentially in the future(because we don't need access to the on-chain state)


Predicate verification will fail if gas is exhausted during execution. The remaining gas field is set as follows:

```math
{remaining\_gas} = \sum_{i=0}^p gas(predicates[i])
```
Voxelot marked this conversation as resolved.
Show resolved Hide resolved

During predicate mode, hitting any of the following instructions causes predicate verification to halt, returning Boolean `false`:

Expand All @@ -123,7 +130,7 @@ If script bytecode is present, transaction validation requires execution.
The VM is [initialized](#vm-initialization), then:

1. `$pc` and `$is` are set to the start of the transaction's script bytecode.
1. `$ggas` and `$cgas` are set to `tx.gasLimit`.
1. `$ggas` and `$cgas` are set to lower of `tx.gasLimit` or `remaining_gas`. `remaining_gas` is determined by deducting any gas consumed during predicate execution from `tx.gasLimit`.

Following initialization, execution begins.

Expand Down