Conversation
| json["state"] = engine.Execute(); | ||
| // Gas consumed in the unit of datoshi, 1 GAS = 1e8 datoshi | ||
| json["gasconsumed"] = engine.GasConsumed.ToString(); | ||
| json["gasconsumed"] = engine.FeeConsumed.ToString(); |
There was a problem hiding this comment.
Do you want me to change the json property as well. @roman-khimov @shargon
There was a problem hiding this comment.
no existing rpc should be updated.
There was a problem hiding this comment.
we should add both, and remove gas in the future
There was a problem hiding this comment.
I'd leave it as is, this API is used a lot, transitioning to feeconsumed would just bring a lot of pain for no real gain.
There was a problem hiding this comment.
I agree with roman, we should avoid change the rpc interface, its wildly used, should avoid any type of change. But maybe we can have a version 2 rpc.
There was a problem hiding this comment.
Vote up for keeping it as is. And do we really need v2? I doubt, at least until we have a set of significant changes to be made in the API
There was a problem hiding this comment.
A set of v2? Why not only those that are needed?
| /// <param name="datoshi">The amount of GAS, in the unit of datoshi, 1 datoshi = 1e-8 GAS, to be added.</param> | ||
| protected internal void AddFee(long datoshi) | ||
| { | ||
| #pragma warning disable CS0618 // Type or member is obsolete |
There was a problem hiding this comment.
Wont this disable the warning on user side as well?
| private string GetDirectoryPath(uint network, uint blockIndex) | ||
| { | ||
| uint folder = (blockIndex / Settings.Default.StoragePerFolder) * Settings.Default.StoragePerFolder; | ||
| uint folder = (blockIndex / Settings.Default!.StoragePerFolder) * Settings.Default.StoragePerFolder; |
There was a problem hiding this comment.
Setttings.Default is not nullable, isn't it?
There was a problem hiding this comment.
Yes, its nullable.
There was a problem hiding this comment.
Setttings.Defaultis not nullable, isn't it?
The default is very weird, cause its initialized when the load is called, no real default value for it.
| json["state"] = engine.Execute(); | ||
| // Gas consumed in the unit of datoshi, 1 GAS = 1e8 datoshi | ||
| json["gasconsumed"] = engine.GasConsumed.ToString(); | ||
| json["gasconsumed"] = engine.FeeConsumed.ToString(); |
There was a problem hiding this comment.
I'd leave it as is, this API is used a lot, transitioning to feeconsumed would just bring a lot of pain for no real gain.
| protected internal void AddFee(long datoshi) | ||
| { | ||
| #pragma warning disable CS0618 // Type or member is obsolete | ||
| FeeConsumed = GasConsumed = checked(FeeConsumed + datoshi); |
There was a problem hiding this comment.
Frankly, I don't understand why we have FeeConsumed here. It's perfectly fine to count GasConsumed in sdatoshis, that's what you use in VM/contract anyway.
There was a problem hiding this comment.
I don't know. I'm just fixing the warning. @Jim8y any comment?
There was a problem hiding this comment.
Frankly, I don't understand why we have
FeeConsumedhere. It's perfectly fine to countGasConsumedinsdatoshis, that's what you use in VM/contract anyway.
@roman-khimov No, its not necessarily to use FeeConsumed here, problem is the same as anywhere else, gas is gas, datoshi is datoshi, miss using it cause ambiguous. So i tried to correct gas as much as possible. But if the team thinks we should keep using GasConsumed, it can be corrected.
ChangeLog
Note: this updates the
GasConsumedtoFeeConsumedfororacleand other services.Type of change
Checklist: