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

Full gas price SCRs #6286

Closed
wants to merge 11 commits into from
Closed

Full gas price SCRs #6286

wants to merge 11 commits into from

Conversation

sasurobert
Copy link
Contributor

Reasoning behind the pull request

The smart contract results (SCRs) can carry value and data. The same is valid with the regular transactions which can carry value and data. We can also have 0 value transactions, and those transactions still need to cover the base cost of their processing which is 50k gas limit for processing and 1500*len(transaction.Data) for the data field. This leads to a disproportionate processing gas cost for the smart contract results when compared to regular transactions, even though the time cost for executing the smart contract results is similar.

Proposed changes

As the current code goes, we take from the sender the baseCost*gasPrice + extraGas * gasPrice/100, and we have to start by changing this to take into account allGas * gasPrice and give back to the user after the processing ends the refunded amount. Any way, for 100% of the SC executions we create a refund, there is no EXACT gas computation done for apps, everyone is overestimating a little bit, as that is comfortable. There will be a bigger upfront cost for the transaction, but with more refunds. This is a must change for any txType which is other than SCExecutionOnDestionation - for others, we know the exact gasCost.
The next change which has to be done is when computing the refund. Right now the refund is equal with gasPrice/100 * gasRemaining. Here a more complex operation is going to take place as we will check how much is the sum of all the SCR.Data from the VMOutput and how many SCRs were generated. For every SCR generated we take the basePrice of 50K (same as moveBalance) with full price, and for the sum we take 1500 with full price.
GasUsedForProcessing = VMOutput.GasUsed - 50K * number of SCRs - 1.5K * SUM (len(SCR.Data)_for all SCRs generated)
Refund = VMOutput.GasRemaining * GasPrice + GasUsedForProcessing * (1 - factor)
By doing this change, SC developers will get a higher return as well.

Testing procedure

Run all the tests and see that balances are correct.

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

… full-gasPrice-SCRs

# Conflicts:
#	common/constants.go
#	common/enablers/enableEpochsHandler_test.go
#	config/epochConfig.go
#	config/tomlConfig_test.go
#	integrationTests/testProcessorNode.go
#	node/metrics/metrics.go
#	node/metrics/metrics_test.go
#	process/transaction/baseProcess.go
#	process/transaction/shardProcess.go
# Conflicts:
#	process/transaction/shardProcess.go
@sasurobert sasurobert closed this Jul 1, 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

Successfully merging this pull request may close these issues.

1 participant