-
Notifications
You must be signed in to change notification settings - Fork 373
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
fix: Longer package Path Spend More Gas [investigate] #2736
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2736 +/- ##
==========================================
- Coverage 60.23% 60.22% -0.01%
==========================================
Files 562 562
Lines 75091 75043 -48
==========================================
- Hits 45228 45196 -32
+ Misses 26482 26473 -9
+ Partials 3381 3374 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
No decision was reached yesterday. I still think making people pay more simply because of the longer username is unfair, but I do agree that shorter paths are more readable. Maybe a good compromise would be to only count gas for the part after |
Description
This PR address the process of identifying and resolviong the cause of gas usage variations during deployment based on the length of package path.
Testing Process
To identify the cause, I conducted tests using txtar testscript. I used the following script, chainging only the length of the package path for check.
Even when an out of gas error occured, the only function visible in the stack trace was the
ExecSignAndBroadcast
function inmaketx.go
file. Therefore, I added additional debugging lines to trace which function were being called.Cause Identification
While running the test script, I discovered that the
NewAnteHandler
function was being called repeatedly. This function contains the following line, whereTxSizeCostPerByte
is set a default value of 10:I added a debugging line like below:
func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer SignatureVerificationGasConsumer, opts AnteOptions) sdk.AnteHandler { // ... newCtx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*store.Gas(len(newCtx.TxBytes())), "txSize") + println("xxxxxxxx ante gas consumed", newCtx.GasMeter().GasConsumed()) // ... }
Output
I executed the script while chainging the pacakge path to make have different lengths and confirmed that the gas amount changed according to the length of
TxBytes()
as follows:Case 1: Package path length 22 (gno.land/p/demo/foo/aa)
TxSizeCosePerByte
NewAnteHandler
callCase 2: Package path length 23 (gno.land/p/demo/foo/aaa)
TxSizeCosePerByte
NewAnteHandler
callWe can observe that as the length of the package path increases, so does the gas usage. However, since gas usage is calculated based on the length of the transaction data obtained through the
TxBytes
method, we can hypothesize that it is also affected by the length of the contract content, not only the package path.[TODO]
Related Issue
#2083
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description