feat: Gas consumption script #1029
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
evan-forbes
added
T:investigate
testing
MSevey
reviewed
Nov 17, 2022
Comment on lines
+178
to
+180
| for i := 0; i < 300; i++ { | ||
| accounts = append(accounts, tmrand.Str(9)) | ||
| } |
There was a problem hiding this comment.
Append re-allocates memory. So it is more memory performant to initialize the slice and then fill it.
size := 300
accounts = make([]string, 0, size)
for i:=0; i<size; i++ {
accounts[i] = tmrand.Str(9)
}There was a problem hiding this comment.
thx for the reminder, we actually have a linter for this but I'm not sure why it didn't get hit
MSevey
reviewed
Nov 17, 2022
testutil/testnode/sign.go
Outdated
| // inside the client.Context, then broadcasts it synchronously. | ||
| func SignAndBroadcastTx(encCfg encoding.Config, c Context, account string, msg ...sdk.Msg) (res *sdk.TxResponse, err error) { | ||
| opts := []types.TxBuilderOption{ | ||
| types.SetGasLimit(1000000000000000000), |
|
this is still a draft, and is not worth reviewing atm |
5 tasks
|
created a few plots using data collected using the test here |
evan-forbes
added a commit
that referenced
this pull request
Nov 18, 2022
## Overview We need to be able to modify the genesis state when using the testnode. This PR refactors the testnode to allow for that. It also includes a helper function that handles creation of this genesis state and simplify the process of starting an in process node. closes #1037 part of/blocking #658 spun out of and blocking #1029 ## Checklist - [x] New and updated code has appropriate documentation - [x] New and updated code has new and/or updated testing - [x] Required CI checks are passing - [x] Visual proof for any user facing features like CLI or documentation updates - [x] Linked issues closed with keywords Co-authored-by: Rootul P <[email protected]>
|
We very likely won't run this test to trace the gas consumption very often, and there is quite a bit of code, so we should just keep them in their own repo. The only thing this PR should do is enable the investigations to be ran by including the gas consumption monitoring logic imo |
|
closing this, as we don't have plans on merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR adds a test suite that we can use to measure the gas consumed by standard txs and modifies the network tests so that we can use custom genesis parameters. We still have to add more tx types and some visualization would be nice.
Checklist