-
Notifications
You must be signed in to change notification settings - Fork 719
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
Changes for predicate gas metering #448
Conversation
src/vm/index.md
Outdated
@@ -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. | |||
1. `$ggas` and `$cgas` are set to the lower of `tx.gasLimit` or the remaining gas following the previous predicate execution. |
There was a problem hiding this comment.
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:
- 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.
- 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.
There was a problem hiding this comment.
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:
- UX simplicity - it is easier for the sdk to estimate the tx gas limit regardless of changes to the gas schedule
- Smaller tx payload, as each new field increases our calldata expense / bandwidth
- 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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
fix lint error
Co-authored-by: Brandon Kite <[email protected]>
…bs/fuel-specs into mitch/predicate-gas-metering
# Conflicts: # src/protocol/tx_format/input.md # src/protocol/tx_validity.md # src/vm/instruction_set.md
update pseudocode for minimum available gas allowed Co-authored-by: Brandon Kite <[email protected]>
…bs/fuel-specs into mitch/predicate-gas-metering
…redicateGasUsed is zero
…havior for contract instructions during predicate.
This updates the description of script gas to explicitly mention that it includes predicate gas costs and removes comments mentioning that gas is not charged for predicates.
Closes #440