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

Tool for testing runtime parameter changes against mainnet traffic #7735

Open
jakmeier opened this issue Sep 30, 2022 · 3 comments
Open

Tool for testing runtime parameter changes against mainnet traffic #7735

jakmeier opened this issue Sep 30, 2022 · 3 comments
Assignees
Labels
A-contract-runtime Area: contract compilation and execution, virtual machines, etc T-contract-runtime Team: issues relevant to the contract runtime team

Comments

@jakmeier
Copy link
Contributor

When we change gas parameter values, this generally has the potential to break existing contracts. Checking how many contracts are affected is a bit tricky.

In principle, it could just be done by replaying old traffic on a new binary with the changes implemented. But this is not quite as straight forward, as any change to the parameter is a protocol change and is thus incompatible with blocks of an older protocol version. We would have to disabled some checks to make this work. Then we could maybe reuse some counting logic from with_ext_cost_counter that we already use in the estimator.

Another way of doing it would be to read execution results and gas profiles from the DB. This should also be much faster as transactions are not replayed. Implementing such a tool requires some effort as well, but overall seems like a good idea. So far, we have been implementing similar functionalities in scrips, such as this code to evaluate the effect of increasing TTN cost.

Some related discussion on this topic: #7733

@jakmeier jakmeier added A-contract-runtime Area: contract compilation and execution, virtual machines, etc T-contract-runtime Team: issues relevant to the contract runtime team labels Sep 30, 2022
@jakmeier jakmeier self-assigned this Sep 30, 2022
@jakmeier
Copy link
Contributor Author

jakmeier commented Oct 4, 2022

progress update on this:

  • I have a tool on a branch that allows to check 50k blocks in 45min ( around 18 blocks/second) on a mainnet archival node against contract runtime gas parameter changes. Without stopping the node or slowing it down significantly. This is based on code in feat(state-viewer): simple gas counter extraction  #7733 and extra calculations to the actions fees right.
  • Sometime the tool fails with IO errors, not sure if somethings is wrong in how we usr RocksDB with multiple connections or what else the problem could be.
  • Getting the gas numbers exactly right is not trivial. Running with a no-diff parameter set, I would expect that the output gives me 100% unchanged costs. I am already at 99.99% but there seems to still be a problem with data receipt base costs when there are multiple data receipts and the generating receipt is already created from another function call, not a transaction.
  • I am first and foremost using this tool now to get some data I need for potential storage access cost changes. But I hope it will also give me a better understanding of what tools we need, whether we should get rid of ext_cost_counter (Get rid of with_ext_cost_counter #5719) and how to move forward with feat(state-viewer): simple gas counter extraction  #7733.

@jakmeier
Copy link
Contributor Author

#8033 will help with this effort as a positive side-effect :)

@jakmeier
Copy link
Contributor Author

Once again it appears we would love to have such a tool already implemented... Thus in this comment, I outline the required work to finish the tool.

The existing code is not fit to be merged but can certainly guide a proper implementation through all the corner cases to consider. Below are three possible ways to tackle the issue, with the third one being my personal recommendation.

The ideal solution

  • Having merged feat: gas profile by parameter #8148 it becomes possible to map from gas profiles (V3) back to parameters, which was a problem in the hacky implementation linked above. The hacky code bits should be easily adapted to the new profiles.
  • We still don't record profiles everywhere (Use gas profiles everywhere #8260) but we should add that to support action execution costs.
  • Both points above will only help us for blocks that have been applied by a node with the updated code. Old profiles won't contain enough data for clean a general-purpose tool. But the whole point of this tool would be to run analysis on an archival node's DB, which are full of profiles V2 and won't produce profiles V3 until the current master is released.
    To solve this, we could replay historical block ranges as far back as we need it for the analysis on one archival node and then use this node's DB that would only have profiles V3. (This could turn out tricky / slow based on prior attempts to replay from genesis.)

Quick and dirty solution

We can also try to clean up the hacky solution a bit. This involves more coding work and requires a deeper understanding of how gas is and has been charged.

The main pain points are:

  • We need to include some reverse-calculations to get the action fees that are not recorded in gas profiles. Coding this is verbose and error prone.
  • Per-byte and base costs are stored together in host function parameter. We might be able to "guess" x based on the linear constraint total = BASE_PARAM + x * PER_BYTE_PARAM. Not pretty but would probably work.

If we can make it work, this solution would work directly on a normal archival node's DB which we can get from a snapshot. But we effectively add tech-debt to our code base.

Incremental solution

We can also consider making this tool tailored to specific parameters that we care about. For example, to impact of increasing the 3 parameters for CreateAccount, we could simplify a lot by only implementing the reverse-calculations for this case. We could further constrain the cost for all 3 components (EXEC, SEND_SIR, SEND_NOT_SIR) to be the same number before and afterwards, which again simplifies the implementation.

I think this solution could still be considered tech-debt. But it would be much smaller in scope. And it would at least be a small step toward the ideal solution, if we ever wanted to move forward with that.

Conclusion

I think finishing a very dumb MVP according to the "incremental solution" description is possible in a few days. This could be done just for our needs today and serve as a basis for future additions. (Or if it turns out to be too hacky, we don't even have to merge it and use it as one-off.)

If the tool proves to be useful, we can consider spending more time to make it general-purpose. The more time passes, the more valuable that would be since the archival nodes will also contain more and more profile V3 data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contract-runtime Area: contract compilation and execution, virtual machines, etc T-contract-runtime Team: issues relevant to the contract runtime team
Projects
None yet
Development

No branches or pull requests

1 participant