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

Add gas_charge function to the interpreter #8

Merged
merged 2 commits into from
Jul 27, 2021
Merged

Conversation

vlopes11
Copy link
Contributor

@vlopes11 vlopes11 commented Jul 21, 2021

The gas cost for every opcode is expected to be decomposed into atomic
GasUnits.

These will represent individual operations performed while executing the
opcode. Some examples are:

Arithmetic
Branching
MemoryOwnership
MemoryWrite(bytes)
RegisterRead
RegisterWrite

All these atomic operations are associated with a constant gas cost.
Having a mapping between a given opcode + VM state -> decomposed gas
units, it is trivial to calculate the gas cost for any code execution.

Currently, the opcode decomposition into gas units is performed
manually. It can, however, and trivially, be converted into build-time
opcode execution syntax tree conversion into gas units.

The gas cost for every opcode is expected to be decomposed into atomic
`GasUnits`.

These will represent individual operations performed while executing the
opcode. Some examples are:

Arithmetic
Branching
MemoryAlloc(bytes)
MemoryOwnership
MemoryWrite(bytes)
RegisterRead
RegisterWrite

All these atomic operations are associated with a constant gas cost.
Having a mapping between a given opcode + VM state -> decomposed gas
units, it is trivial to calculate the gas cost for any code execution.

Currently, the opcode decomposition into gas units is performed
manually. It can, however, and trivially, be converted into build-time
opcode execution syntax tree conversion into gas units.
@vlopes11 vlopes11 self-assigned this Jul 21, 2021
// TODO enable const flag
// https:/rust-lang/rust/issues/57349
pub fn gas_charge(&mut self, op: &Opcode) -> bool {
let cost = self.gas_price() * self.gas_cost(op);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so gas price * gas used is the cost in coins that ends up being deducted from the free balance of the zero-color coin. When you do gas accounting during execution (here) you only need to care about gas used, not gas price.

At the very end of transaction execution (either nice termination or panic), unused coins are refunded as change: https:/FuelLabs/fuel-specs/blob/c332683080ea3d65d43abe32f143b39526dcdc96/specs/protocol/tx_validity.md#correct-change

Suggested change
let cost = self.gas_price() * self.gas_cost(op);
let cost = self.gas_cost(op);

src/interpreter/gas.rs Outdated Show resolved Hide resolved
@@ -356,205 +356,238 @@ where
}

match op {
Opcode::ADD(ra, rb, rc) if Self::is_valid_register_triple_alu(ra, rb, rc) => {
Opcode::ADD(ra, rb, rc) if Self::is_valid_register_triple_alu(ra, rb, rc) && self.gas_charge(&op) => {
Copy link
Member

@Voxelot Voxelot Jul 23, 2021

Choose a reason for hiding this comment

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

would it be better to have these functions return results with more specific error types besides ExecuteError::OpcodeFailure(op)?

Maybe something like this for example:

Opcode::ADD(ra, rb, rc) => {
    Self::is_valid_register_triple_alu(ra, rb, rc)?;
    self.gas_charge(&op)?;
    self.alu_overflow(ra, Word::overflowing_add, self.registers[rb], self.registers[rc])
}

where gas_charge(&op) returns something like Result<(), ExecuteError>

That way you can have errors for things like "OutOfGas" get propagated directly from the source and simplify the matching syntax a little.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm usually first returning a single bool for these new functions and then later guaranteeing we have no added instructions for error traceability. What I have in mind is to create a raw super-optimal executor, and enable a feature "gas-profiler" that will allow us to trace the gas costs.

However, I just checked the release profile for an OutOfGas error without additional metadata, and we have no additional overhead in this case! I'll add it, its indeed clearer.

Copy link
Contributor

@adlerjohn adlerjohn Jul 26, 2021

Choose a reason for hiding this comment

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

enable a feature "gas-profiler" that will allow us to trace the gas costs.

Just so we're on the same page, by gas profiler do you mean accounting for gas, period, or profiling things like hot spots of gas usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hot spots. The idea I have in mind is to have a tracer that will decompose an executed transaction into a vector of GasUnit

@vlopes11 vlopes11 merged commit c80b739 into master Jul 27, 2021
@vlopes11 vlopes11 deleted the vlopes11/gas branch July 27, 2021 20:09
ControlCplusControlV pushed a commit that referenced this pull request Nov 24, 2022
ControlCplusControlV pushed a commit that referenced this pull request Nov 24, 2022
Very often a opcode is created out of sized slices that are guaranteed
to contain the needed bytes.

This method will provide a fast way to perform conversion of bytes to
opcodes without additional overhead or code verbosity.
ControlCplusControlV pushed a commit that referenced this pull request Nov 24, 2022
ControlCplusControlV pushed a commit that referenced this pull request Nov 24, 2022
ControlCplusControlV pushed a commit that referenced this pull request Nov 29, 2022
ControlCplusControlV pushed a commit that referenced this pull request Dec 1, 2022
ControlCplusControlV pushed a commit that referenced this pull request Dec 1, 2022
Very often a opcode is created out of sized slices that are guaranteed
to contain the needed bytes.

This method will provide a fast way to perform conversion of bytes to
opcodes without additional overhead or code verbosity.
ControlCplusControlV pushed a commit that referenced this pull request Dec 1, 2022
mitchmindtree pushed a commit that referenced this pull request Dec 5, 2022
@mitchmindtree mitchmindtree added the fuel-vm Related to the `fuel-vm` crate. label Dec 9, 2022
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
Very often a opcode is created out of sized slices that are guaranteed
to contain the needed bytes.

This method will provide a fast way to perform conversion of bytes to
opcodes without additional overhead or code verbosity.
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
Very often a opcode is created out of sized slices that are guaranteed
to contain the needed bytes.

This method will provide a fast way to perform conversion of bytes to
opcodes without additional overhead or code verbosity.
xgreenx pushed a commit that referenced this pull request Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuel-vm Related to the `fuel-vm` crate.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants