use flat vm fee #4607
use flat vm fee #4607
Conversation
|
simtest failure is unrelated |
| if single_tx.uses_vm() { | ||
| // Charge gas for this VM execution | ||
| if let Err(e) = gas_status.charge_vm_gas() { | ||
| result = Err(e); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Why not just move this into the match arms for the items that call the VM? Are we also flat feeing publishing with the same flat fee? I imagine we don't want to but it looks like we are here?
There was a problem hiding this comment.
Why not just move this into the match arms for the items that call the VM?
Because code duplication. I also think it's clearer to flag that we're charging for VM use.
Are we also flat feeing publishing with the same flat fee? I imagine we don't want to but it looks like we are here?
Publishing can invoke VM as part of init
| break; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I would imagine calls for this inside the various match arms, where you can say
let move_gas = match gas_status.charge_vm_gas() {
Ok(ff_move_gas) => ff_move_gas,
Err(e) => { result = Err(e); break }
}
|
@tnowacki |
Moving PR to draft while we add estimation logic |
|
Another way to reduce dev friction is to ship estimation logic first by using the flat fee here. |
|
💳 Wallet Extension has been built, you can download the packaged extension here: https://github.com/MystenLabs/sui/actions/runs/3232781121#artifacts |
|
Tests are failing due to unrelated Narwhal issues. Waiting for fix. |
This is done |
| @@ -101,7 +101,7 @@ impl FaucetClient for LocalFaucetClient { | |||
| async fn request_sui_coins(&self, request_address: SuiAddress) -> FaucetResponse { | |||
| let receipt = self | |||
| .simple_faucet | |||
| .send(Uuid::new_v4(), request_address, &[50000; 5]) | |||
| .send(Uuid::new_v4(), request_address, &[100000; 5]) | |||
There was a problem hiding this comment.
is 50k insufficient in some case?
There was a problem hiding this comment.
Yes it was. But generally we increased the faucet amount in devnet so it makes sense here too
| @@ -25,7 +25,7 @@ use move_binary_format::{ | |||
| }; | |||
|
|
|||
| /// VM flat fee | |||
| pub const VM_FLAT_FEE: Gas = Gas::new(1_000); | |||
| pub const VM_FLAT_FEE: Gas = Gas::new(2_000); | |||
There was a problem hiding this comment.
as this might need to tuned, maybe its better to put the number in some config or env files?
There was a problem hiding this comment.
Well this leads to a potential for each validator to start with different values.
Also if we use cfg files here, why not do it for all values in the cost table?
There was a problem hiding this comment.
I think this constant is a good place to tune this.
In the future, we might need to change this as the software upgrades, but we will need to sort out that whole problem at a later date.
|
|
This adds the flat fee for VM gas logic
The fee itself is subject to change
Finding the right number took a few iterations and will probably take a lot more once folks start using the new feature