Optimise gas usage during the upgrade process #147
Conversation
| @@ -7,13 +7,13 @@ use near_sdk::serde_json; | |||
| use near_sdk::{env, sys, AccountId, Balance, CryptoHash, Gas}; | |||
|
|
|||
| /// Gas spent on the call & account creation. | |||
| const CREATE_CALL_GAS: Gas = Gas(75_000_000_000_000); | |||
| const CREATE_CALL_GAS: Gas = Gas(50_000_000_000_000); | |||
There was a problem hiding this comment.
I think this can be decreased even further. Could probably go for 30-40.
There was a problem hiding this comment.
Im not sure i 100% agree here. Since the protocol refunds overspend on gas, its safer to attach more, in this case we dont know 100% of times will work given large policy from start.
There was a problem hiding this comment.
Are you not happy with 50 or with a value lower than 50?
I'm not sure if a larger policy would consume more gas? It makes sense to consume more storage, but is the gas affected as well? Shouldn't be and if it is, I think the impact is pretty low...
There was a problem hiding this comment.
I'm not 100% sure here either, you made a valid point
There was a problem hiding this comment.
Been having a solid think on this:
As I have deployed DAOs from other DAOs and within other batch calls, even the previous 75Tgas was plenty to get through. Even at 75Tgas, i have seen an avg of 5-25Tgas based on the policy tests i've done. This is not the maximum size a policy can be however - so maybe a little R&D is needed to find the absolute policy size at creation.
Here's what I'd like to see:
Can you create a test case with a large amount of groups AND/OR a large amount of council accounts?
That should give us a clear indication of how big the gas needs to be.
There was a problem hiding this comment.
I say all this, assuming your estimations on gas all relate to default policy
There was a problem hiding this comment.
We can increase the Tgas to 30-40. It's still low enough to cover the new deployment costs and big enough to cover most of the policies. If a policy consumes more than 30-40, the DAO creator can create the policy slimmer at first and then add new roles after the DAO is created. How does that sound to you?
There was a problem hiding this comment.
That sounds fair. Can you add a test that incorporates a large policy and update the gas to 40?
| @@ -7,13 +7,13 @@ use near_sdk::serde_json; | |||
| use near_sdk::{env, sys, AccountId, Balance, CryptoHash, Gas}; | |||
|
|
|||
| /// Gas spent on the call & account creation. | |||
| const CREATE_CALL_GAS: Gas = Gas(75_000_000_000_000); | |||
There was a problem hiding this comment.
This was absurd. A simple new call to create a DAO does not require more than 5_000_000_000_000 gas. However, I've doubled it to 10_000_000_000_000 just to be safe.
There was a problem hiding this comment.
Updated to 40TGas to cover large policies.
|
Thank you for this!! |
This PR is trying to address #135.