Storage payback

[lock9]

Extending VM Gas when user reuses or releases storage. Related to #278 .

Another option would be evaluating previously the amount of data that would be released, allowing this GAS to be used before in any part of the transaction, however, this has a greater performance impact and this would be required to occur during the verification and would require us to run the application trigger twice.

@shargon when possible, could you please contact me on Discord to help me improve the tests? Thanks.

[lock9]

Again, I would like to ask you guys to think about the possibility of allowing this credit GAS to be used since the first instruction.
Do you guys have any ideas if this is possible to achieve? Maybe if we add a more expensive fee. I don't know. I'm trying to figure this out. Please help 😅

We need a way to know the storage changes beforehand. Maybe this is not possible in the current situation.

[erikzhang]

neo/src/neo/SmartContract/InteropService.cs

Line 21 in 8eef1ac

public const long GasPerByte = 100000;

Use GasPerByte?

[lock9]

Do you mean using this property directly? (don't get it from the policy)
Shouldn't this GasPerByte be on the network policy? Otherwise, it may break the chain if we change it.

[erikzhang]

Will we change it?

[lock9]

Hmm, but if the fees are given to the consensus nodes, shouldn't they be responsible for updating this fee?
Maybe this is a little out of the scope of this PR. My concern is that we will never be allowed to change the fee.

About the PR itself, I can move to variables, I think that we can pay back 1:1 when users release storage, but when they reuse storage too? What if we can pay back a little more when the storage is being released since the user is also charged by the opcode?
How "attractive" should be this storage release?

I added the values to the policy because I think it will be hard to come up with value easily. I don't really know the best ratio for this payback.

[erikzhang]

The fee is pay to the neo holders, not the consensus nodes.

[lock9]

Yes, sorry. Reading NetworkPolicy everywhere that I thought this was a network fee value.

About the difference between reuse and release, do you still prefer one single variable (GasPerByte) or should I add one or two more variables?

[erikzhang]

I don't know. The core developers should discuss it again.

[lock9]

This PR has a bug that needs to be solved. I'm not sure how yet. The ConsumedGas cannot be the final result, but rather the "highest amount" during execution.

Gas consumption variation (t = time):
t0 - 1 GAS
t2 - 2 GAS
t3 - 3 GAS (free storage)
t4 - 1 GAS (storage released)
t5 - Execution finished

Now: ConsumedGas = 1
Should be: ConsumedGas = 3

If the user sends 1 GAS instead of 3, it won't be enough to reach the "free storage" instruction.

I was thinking about having an additional callback to be called after the method Execute on the VM, but I don't know if this will be accepted.
Without that, we can either add "RecalculateUsedGas" after every Execute() or manage this value for each OP Code executed.
If we add the callback, I would call "RecalculateUsedGas" after the VM finishes executing (including step instructions).

Pseudocode:

ReleaseStorage(){
    if(ConsumedGas > MaxConsumedGas) MaxConsumedGas = ConsumedGas
    ConsumedGas -= Discount
}

//After TX execution
RecaulculateGas()
{
    if (ConsumedGas < MaxConsumedGas) ConsumedGas = MaxConsumedGas
}

Any suggestions? Should I add this optional callback to the VM?
Edit: A much easier way is to make Execute virtual

[lock9]

@shargon forget about the callback thing.
Do you think we shuold pre-calculate the released storage? The problem is that either need the client provide these changes for us in a secure way or run the transaction twice.

I'm not very satisfied with the solution too, I'm still figuring out all the problems 😅

[lock9]

I will try to make a different solution using a pre-computed state hash. I don't know if it will work.

This PR has a problem that Shargon identified (besides not being the best final solution).
We should only pay back once per key and only for pre-existing keys.

This fixes this scenario:

while true
 put(key, value)
 delete(key)

[igormcoelho]

Interesting issue... indeed if user only gains back "after", then it needs enough gas, at least to reach that execution point. Obviously, payback should never equals or exceed expended amount, otherwise user can loop it.
To be honest, now that we can easily control GAS as Native NEP5, one possibility is to simply give back gas to user after execution, if not faulted. This way, it can easily reuse it, but only on next transaction.

[shargon]

Agree with igor, they can send all the fee and payback later

[lock9]

I updated the PR, but I still have to wait for #1357.
There are some other tests that I have to add too, but I believe many problems were fixed.

Do you guys think we can return 100% of the fee?

[Tommo-L]

Do you guys think we can return 100% of the fee?

Considering the competitors, I think return 100% is desirable. and also we need to keep fees low for developers.

[Tommo-L]

Could we set the discount in PolicyContract?

[lock9]

Hello @Tommo-L,
I initially added the discount on the PolicyContract, however, this Gas is distributed to Neo owners, and not consensus nodes, so it is not reasonable to allow ConsensusNode to update a fee that is not related to them.
You can check the discussion here:
#1356 (comment)

[Tommo-L]

Why is it closed? It's a pretty useful PR.

[lock9]

Hello @Tommo-L , I'm refactoring it, I had some issues to rebase it. I need to reopen it as a draft.