feat(params-estimator): Increase deployment cost #6397
Conversation
jakmeier
requested review from
bowenwang1996,
matklad,
mm-near,
olonho and
Longarithm
as code owners
|
This is a protocol change. Since this is the first one I submit, I hope I got the technicalities for this right. Please let me know otherwise. For now I updated the cost for version 52 but |
| @@ -32,7 +32,7 @@ | |||
| "deploy_contract_cost_per_byte": { | |||
| "send_sir": 6812999, | |||
| "send_not_sir": 6812999, | |||
| "execution": 6812999 | |||
| "execution": 64572944 | |||
There was a problem hiding this comment.
I think that you might have to create version 53.. as 52 (AFAIK) was already pushed to testnet - but you can check with @mina86 who is doing that release.
There was a problem hiding this comment.
I created version 53 now and also updated STABLE_PROTOCOL_VERSION to 53, so that my change is actually tested. (Or would we first need to test this only in nightly protocol?)
In general, I am unsure when the right moment is to do these steps, it seems that the technical review of the change and the mapping to a protocol version should be mostly independent steps.
| assert_eq!(transaction_result.status, FinalExecutionStatus::SuccessValue(to_base64(&[]))); | ||
| assert_eq!(transaction_result.receipts_outcome.len(), 1); | ||
| assert!( | ||
| transaction_result.receipts_outcome[0].outcome.gas_burnt |
There was a problem hiding this comment.
how much gas does it burn now? how much is the headroom?
There was a problem hiding this comment.
(Okay, I actually forgot to add up TX conversion cost and receipt execution cost in this test, so the headroom was more than i expected. But now that I fixed that:)
Burnt gas is 299990074122595. This is 300Tgas - tx_overhead * new_deployment_per_gas_cost and some rounding. (TX overhead is 139B in this test.)
Motivation and calculation can be found in this document, which should also clarify what headroom we aim for:
https://docs.google.com/document/d/172ycoeDDhmeWwQPmWsxu5M16cSRc64NTCJMxPf9cImk/edit?usp=sharing
jakmeier
force-pushed
the
increase-deployment-cost
branch
from
bbcff19
to
8f6f797
Compare
Set the deplyoment cost per byte to the maximum that still allows deploying a 4MiB contract. This is to cover compilation costs.
(push missing `git add`)
jakmeier
force-pushed
the
increase-deployment-cost
branch
from
6b5f6bd
to
45d6c3c
Compare
|
From my point of view, this is ready for final review. I contacted everyone that I could identify to be affected by the change. And besides the test that I wrote already, I cannot think of anything that could be tested here. If possible, I would like to include this in NEAR TestNet Release - 1.26.0-rc.1, scheduled for Wed, March 23. |
When updating near#6397 to protocol v53 right before merging, I forgot to add the configuration JSON file to the config store. Test plan --------- Added a test to verify that cost indeed increases.
When updating #6397 to protocol v53 right before merging, I forgot to add the configuration JSON file to the config store. Test plan --------- Added a test to verify that cost indeed increases.
Set the deployment cost per byte to the maximum that still allows
deploying a 4MiB contract. This is to cover compilation costs.
Test plan
A new integration test checks that a 4MiB can still be deployed.