ref-finance / ref-contracts

Smart contracts for Ref Finance
MIT License
96 stars 54 forks source link

Potentially required contract updates due to increased deployment cost #71

Open jakmeier opened 2 years ago

jakmeier commented 2 years ago

Deployment costs per byte of code are going to be increased by a factor of ~5 with this change to NEAR protocol: https://github.com/near/nearcore/pull/6397 I believe this does not break any of your code but I want to raise awareness so that you can double-check before this is deployed.

Probably this will be included with protocol version 53, which would be released on testnet by March 23 and on mainnet by April 20.

Why am I even contacting you? To find everyone affected by this change, I listed all the indirect deployments from the past that would fail with the new cost, if all gas attachments stay the same. One deployment from v2.ref-finance.near and one from v2.ref-farming.near showed up in this list. Meaning that the mechanism used to deploy them in the past might no longer work after the new release.

I am guessing ref-exchange is deployed on v2.ref-finance.near and ref-farming on v2.ref-farming.near, right? Both seem to follow a similar pattern for upgrades. Here is the code I found for ref-farming.

https://github.com/ref-finance/ref-contracts/blob/2ecbe5c7897e9ad1783fb5378d17eab66bad7436/ref-farming/src/owner.rs#L57-L102

First of all, props to you for let attached_gas = env::prepaid_gas() - env::used_gas() - GAS_FOR_MIGRATE_CALL;. Using this dynamic gas value for the following function call means that you are probably fine with the upgrade even without any changes. (Other contracts use hard-coded values and they are breaking with the increased cost.)

What I want to tell you is that the cost to deploy ref_farming_release.wasm, or any contract of size 416KB, will go from ~6.04TGas to ~30.07Tgas. (That is pure deployment cost, without function call costs or storage costs.) After a quick scan over your code, I see nothing that would break because of this increase. But please review this for yourself.

fercargo commented 2 years ago

@jakmeier, question, at line 86, env::used_gas() already includes the gas required for deployment on line 82? but line 82 seems to be a promise preparation, not the actual deployment

jakmeier commented 2 years ago

Line 82 calls promise_batch_action_deploy_contract, which asks the runtime to create a promise for deployment. You are right, it is not executed, yet. However, the protocol follows the idea that whenever possible, gas costs are charged upfront. So what happens is, calling promise_batch_action_deploy_contract burns the gas to create the promise and uses all gas necessary to execute the promise. (The difference between burning and using is only important if something fails and therefore the promise never executes. In that case, used but not burned gas is refunded.) Calls to env::used_gas() will give you the updated value, after deducting promise creation and execution fees. This is possible since we know how large the contract is, we know exactly how much executing the deployment promise will cost. For function call promises, the exact cost to execute is not known before, thus we have to guess how much it will use and manually attach gas that will be used later. But even for function call promises, there is a fixed amount that is charged upfront.