[WIP] Storage payback #1372
[WIP] Storage payback #1372
Conversation
| bool keyAdded = false; | ||
| if (!updatedKeys.Contains(key)) | ||
| { | ||
| updatedKeys.Add(key); |
There was a problem hiding this comment.
It's better to return a value if it's added
There was a problem hiding this comment.
What do you mean? It returns true or false. Why should I return the added key if invoker already has it?
There was a problem hiding this comment.
not your method, updatedKeys.Add(key)
lock9
force-pushed
the
storage-payback
branch
from
2d984cc to
26c50fa
Compare
| foreach (Transaction tx in engine.Snapshot.PersistingBlock.Transactions) | ||
| Burn(engine, tx.Sender, tx.SystemFee + tx.NetworkFee); | ||
| { | ||
| var sysFee = CalculateSystemFeeWithPayback(tx); |
There was a problem hiding this comment.
In CalculateSystemFeeWithPayback, tx.SysFeeCredit = 0 ? As GasToken.OnPersist executes before tx.script.
There was a problem hiding this comment.
I will add more tests, so far I was only able to ensure that the payback is being calculated correctly. The next step is to ensure that we are charging the user the adequate fee.
|
|
||
| public TriggerType Trigger { get; } | ||
| public IVerifiable ScriptContainer { get; } | ||
| public StoreView Snapshot { get; } | ||
| public long GasConsumed { get; private set; } = 0; | ||
| public long GasCredit { get; private set; } = 0; | ||
| public long MinimumGasRequired { get { return Math.Max(GasConsumed, maxConsumedGas); } } |
There was a problem hiding this comment.
I think we could remove MinimumGasRequired and maxConsumedGas, but keep GasConsumed and rename GasCredit to RecyclingRewardGas.
Add reuse and release discount rewards to RecyclingRewardGas. In this way, we don't have to modify the Wallets.MakeTransaction.
After tx.script executed, we can pay back the RecyclingRewardGas to user.
There was a problem hiding this comment.
@Tommo-L if we remove maxConsumedGas the user will need the full-balance to send the TX. I think it is not possible to do it without updating the wallet, because the user can send less fees now.
There was a problem hiding this comment.
I think it is not possible to do it without updating the wallet, because the user can send less fees now.
If we allow user send less fees, we will face the contract halting problem. as the transaction's sysfee will loss the use for limitation of execution, is it right?
There was a problem hiding this comment.
This minimum is exactly for that. The minimum required for the transaction to not fault.
|
@lock9 conflicts |
|
@shargon Close? |
Recreating my previous PR as a draft. Please check the tests for more information.
@shargon @igormcoelho I faced some issues while trying to implement the payback.
What value will be stored on the SysFee property in a transaction that has pay-back? The value with or without the credit?
The issue of only paying back, in the end, is that this is going to demand users to have the full balance to call a transaction that may cost them 0. Doesn't feel right to me 😅