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

Charge transaction for signatures verification as part of min_fee #599

Closed
xgreenx opened this issue Oct 5, 2023 · 1 comment
Closed
Assignees

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Oct 5, 2023

Problem overview

ECR is the most expensive opcode in the VM (~1700 noops). However, we charge significantly less than that currently for verifying signatures used by coin or message inputs. This is partly because witness data is not chargeable. If we were to charge for signatures in witness data and also increase the GAS_PER_BYTE factor until the signature cost is adequately captured, this would disproportionately increase the cost of other bytes which should otherwise be cheap. We should have an independent charge which is based on the number of signatures a transaction has to verify, instead of warping the cost of bytes to cover it.

Implementation details

The check_signatures method uses is_predicate_owner_valid and recover_witness under the hood(based on the type of inputs). They are the most expensive operations during the verification of the signatures.

For recover_witness we can use the the gas cost of the ecr1 opcode.

For is_predicate_owner_valid, it is more tricky because we use BMT under the hood + hashing function. For BMT we can either come up with a logarithmic formula or use hardcoded gas based on the worst possible scenario(the maximum size of the contract is 17 MB, while the predicate is less).

Both should affect the final min_fee in TransactionFee::checked_from_values. It may require some architectural modification into the TransactionFee structure because it requires inputs to calculate min_fee.

@xgreenx xgreenx self-assigned this Oct 5, 2023
@xgreenx xgreenx removed their assignment Oct 12, 2023
bvrooman pushed a commit to FuelLabs/fuel-core that referenced this issue Nov 1, 2023
Related issues:
- FuelLabs/fuel-vm#599

This PR adds a benchmark to determine the gas price of calculating
contract roots.

---------

Co-authored-by: xgreenx <[email protected]>
bvrooman pushed a commit to FuelLabs/fuel-core that referenced this issue Nov 1, 2023
@bvrooman
Copy link
Contributor

bvrooman commented Nov 1, 2023

Closed by #613 and #618

@bvrooman bvrooman closed this as completed Nov 1, 2023
crypto523 pushed a commit to crypto523/fuel-core that referenced this issue Oct 7, 2024
Related issues:
- FuelLabs/fuel-vm#599

This PR adds a benchmark to determine the gas price of calculating
contract roots.

---------

Co-authored-by: xgreenx <[email protected]>
crypto523 pushed a commit to crypto523/fuel-core that referenced this issue Oct 7, 2024
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

No branches or pull requests

2 participants