Skip to content

Remove Build Warnings #3283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shargon merged 1 commit into from
May 30, 2024
Merged

Remove Build Warnings #3283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Neo.GUI/Neo.GUI.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<UseWindowsForms>true</UseWindowsForms>
<Product>Neo.GUI</Product>
<ApplicationIcon>neo.ico</ApplicationIcon>
<GenerateResourceWarnOnBinaryFormatterUse>false</GenerateResourceWarnOnBinaryFormatterUse>
</PropertyGroup>

<ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions src/Neo/SmartContract/ApplicationEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,9 @@ protected static void OnSysCall(ExecutionEngine engine, Instruction instruction)
/// <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
Copy link
Copy Markdown
Contributor

@Jim8y Jim8y May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wont this disable the warning on user side as well?

Copy link
Copy Markdown
Member Author

@cschuchardt88 cschuchardt88 May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No

FeeConsumed = GasConsumed = checked(FeeConsumed + datoshi);
Copy link
Copy Markdown
Contributor

@roman-khimov roman-khimov May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

@cschuchardt88 cschuchardt88 May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. I'm just fixing the warning. @Jim8y any comment?

Copy link
Copy Markdown
Contributor

@Jim8y Jim8y May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@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.

#pragma warning restore CS0618 // Type or member is obsolete
if (FeeConsumed > _feeAmount)
throw new InvalidOperationException("Insufficient GAS.");
}
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/OracleService/OracleService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ public static Transaction CreateResponseTx(DataCache snapshot, OracleRequest req
ContractMethodDescriptor md = oracleContract.Manifest.Abi.GetMethod("verify", -1);
engine.LoadContract(oracleContract, md, CallFlags.None);
if (engine.Execute() != VMState.HALT) return null;
tx.NetworkFee += engine.GasConsumed;
tx.NetworkFee += engine.FeeConsumed;

var executionFactor = NativeContract.Policy.GetExecFeeFactor(snapshot);
var networkFee = executionFactor * SmartContract.Helper.MultiSignatureContractCost(m, n);
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/RpcServer/RpcServer.SmartContract.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private JObject GetInvokeResult(byte[] script, Signer[] signers = null, Witness[
json["script"] = Convert.ToBase64String(script);
json["state"] = session.Engine.State;
// Gas consumed in the unit of datoshi, 1 GAS = 10^8 datoshi
json["gasconsumed"] = session.Engine.GasConsumed.ToString();
json["gasconsumed"] = session.Engine.FeeConsumed.ToString();
json["exception"] = GetExceptionMessage(session.Engine.FaultException);
json["notifications"] = new JArray(session.Engine.Notifications.Select(n =>
{
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/RpcServer/RpcServer.Wallet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ private JObject GetVerificationResult(UInt160 scriptHash, ContractParameter[] ar
json["script"] = Convert.ToBase64String(invocationScript);
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();
Copy link
Copy Markdown
Member Author

@cschuchardt88 cschuchardt88 May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want me to change the json property as well. @roman-khimov @shargon

Copy link
Copy Markdown
Contributor

@Jim8y Jim8y May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no existing rpc should be updated.

Copy link
Copy Markdown
Member

@shargon shargon May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add both, and remove gas in the future

Copy link
Copy Markdown
Contributor

@roman-khimov roman-khimov May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@Jim8y Jim8y May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@AnnaShaleva AnnaShaleva May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

@Jim8y Jim8y May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A set of v2? Why not only those that are needed?

json["exception"] = GetExceptionMessage(engine.FaultException);
try
{
Expand Down
4 changes: 2 additions & 2 deletions src/Plugins/StorageDumper/Settings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ internal class Settings

private Settings(IConfigurationSection section)
{
/// Geting settings for storage changes state dumper
// Geting settings for storage changes state dumper
BlockCacheSize = section.GetValue("BlockCacheSize", 1000u);
HeightToBegin = section.GetValue("HeightToBegin", 0u);
StoragePerFolder = section.GetValue("StoragePerFolder", 100000u);
Exclude = section.GetSection("Exclude").Exists()
? section.GetSection("Exclude").GetChildren().Select(p => int.Parse(p.Value)).ToArray()
? section.GetSection("Exclude").GetChildren().Select(p => int.Parse(p.Value!)).ToArray()
: new[] { NativeContract.Ledger.Id };
}

Expand Down
18 changes: 9 additions & 9 deletions src/Plugins/StorageDumper/StorageDumper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ public class StorageDumper : Plugin
{
private readonly Dictionary<uint, NeoSystem> systems = new Dictionary<uint, NeoSystem>();

private StreamWriter _writer;
private StreamWriter? _writer;
/// <summary>
/// _currentBlock stores the last cached item
/// </summary>
private JObject _currentBlock;
private string _lastCreateDirectory;
private JObject? _currentBlock;
private string? _lastCreateDirectory;


public override string Description => "Exports Neo-CLI status data";
Expand Down Expand Up @@ -73,7 +73,7 @@ private void OnDumpStorage(uint network, UInt160? contractHash = null)
prefix = BitConverter.GetBytes(contract.Id);
}
var states = systems[network].StoreView.Find(prefix);
JArray array = new JArray(states.Where(p => !Settings.Default.Exclude.Contains(p.Key.Id)).Select(p => new JObject
JArray array = new JArray(states.Where(p => !Settings.Default!.Exclude.Contains(p.Key.Id)).Select(p => new JObject
{
["key"] = Convert.ToBase64String(p.Key.ToArray()),
["value"] = Convert.ToBase64String(p.Value.ToArray())
Expand All @@ -94,7 +94,7 @@ private void OnCommitting(NeoSystem system, Block block, DataCache snapshot, IRe
private void OnPersistStorage(uint network, DataCache snapshot)
{
uint blockIndex = NativeContract.Ledger.CurrentIndex(snapshot);
if (blockIndex >= Settings.Default.HeightToBegin)
if (blockIndex >= Settings.Default!.HeightToBegin)
{
JArray stateChangeArray = new JArray();

Expand Down Expand Up @@ -139,7 +139,7 @@ private void OnCommitted(NeoSystem system, Block block)

void OnCommitStorage(uint network, DataCache snapshot)
{
if (_currentBlock != null)
if (_currentBlock != null && _writer != null)
{
_writer.WriteLine(_currentBlock.ToString());
_writer.Flush();
Expand All @@ -150,10 +150,10 @@ private void InitFileWriter(uint network, DataCache snapshot)
{
uint blockIndex = NativeContract.Ledger.CurrentIndex(snapshot);
if (_writer == null
|| blockIndex % Settings.Default.BlockCacheSize == 0)
|| blockIndex % Settings.Default!.BlockCacheSize == 0)
{
string path = GetOrCreateDirectory(network, blockIndex);
var filepart = (blockIndex / Settings.Default.BlockCacheSize) * Settings.Default.BlockCacheSize;
var filepart = (blockIndex / Settings.Default!.BlockCacheSize) * Settings.Default.BlockCacheSize;
path = $"{path}/dump-block-{filepart}.dump";
if (_writer != null)
{
Expand All @@ -176,7 +176,7 @@ private string GetOrCreateDirectory(uint network, uint blockIndex)

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;
Copy link
Copy Markdown
Member

@shargon shargon May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setttings.Default is not nullable, isn't it?

Copy link
Copy Markdown
Member Author

@cschuchardt88 cschuchardt88 May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, its nullable.

Copy link
Copy Markdown
Contributor

@Jim8y Jim8y May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setttings.Default is not nullable, isn't it?

The default is very weird, cause its initialized when the load is called, no real default value for it.

return $"./StorageDumper_{network}/BlockStorage_{folder}";
}

Expand Down