Skip to content
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.

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

add logs to execution #737

Closed
wants to merge 32 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
a634f59
add logs to execution
Jim8y Jun 25, 2022
042db65
fix issue
Jim8y Jun 25, 2022
86a68cc
optimise code style
Jim8y Jun 25, 2022
a89b625
optimise code
Jim8y Jun 25, 2022
f7eefd3
fix log message issue
Jim8y Jun 25, 2022
5b731eb
add log to transaction log
Jim8y Jul 1, 2022
724fc54
remove unused import
Jim8y Jul 1, 2022
4546018
Update LogReader.cs
erikzhang Jul 2, 2022
ac65c60
Merge branch 'master' into log-in-execution
Jim8y Jul 2, 2022
ea6d01a
make initial more clear
Jim8y Jul 3, 2022
e23f226
revert change to rpc client
Jim8y Jul 3, 2022
c45f703
add logs to the rpcserver plugin
Jim8y Jul 3, 2022
8888011
use queue to manage logs
Jim8y Jul 3, 2022
46aa190
update JArray
Jim8y Jul 4, 2022
7b9723e
remove log event on dispose
Jim8y Jul 5, 2022
397458c
revert _db change and move log to rpcserver
Jim8y Jul 11, 2022
391abe0
fix error
Jim8y Jul 11, 2022
c4b093d
update the function name
Jim8y Jul 11, 2022
f7afd69
Merge branch 'master' into log-in-execution
Jim8y Jul 13, 2022
12ded50
revert name change
Jim8y Jul 13, 2022
0bd5c11
Merge branch 'log-in-execution' of github.com:Liaojinghui/neo-modules…
Jim8y Jul 13, 2022
0aa880b
revert csproj change
Jim8y Jul 13, 2022
c0f35ec
revert order change
Jim8y Jul 13, 2022
8a83e5c
remove unused use
Jim8y Jul 13, 2022
6d15ff0
Merge branch 'master' into log-in-execution
Jim8y Sep 3, 2022
c0e72ac
Merge branch 'master' into log-in-execution
shargon Sep 12, 2022
9c724dd
Merge branch 'master' into log-in-execution
superboyiii Oct 9, 2022
68f35f6
Update RpcServerPlugin.cs
shargon Oct 10, 2022
5783063
make log size configurable
Jim8y Oct 10, 2022
0e3ccb6
Merge branch 'master' into log-in-execution
Jim8y Oct 26, 2022
12a36c6
Merge branch 'master' into log-in-execution
superboyiii Nov 7, 2022
b899935
Merge branch 'master' into log-in-execution
Jim8y Feb 21, 2023
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
5 changes: 5 additions & 0 deletions src/ApplicationLogs/LogReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,13 @@ private void OnCommitting(NeoSystem system, Block block, DataCache snapshot, IRe
foreach (var appExec in applicationExecutedList.Where(p => p.Transaction != null))
{
var txJson = TxLogToJson(appExec);
if (RpcServer.LogEvents.TryGetValue(appExec.Transaction.Hash, out var list))
{
txJson["logs"] = new JArray(list.Select(q => new JString(q.Message)));
}
Put(appExec.Transaction.Hash.ToArray(), Neo.Utility.StrictUTF8.GetBytes(txJson.ToString()));
}
RpcServer.LogEvents.Clear();
Copy link
Member

Choose a reason for hiding this comment

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

Clear() has been called in RpcServer.

Copy link
Contributor Author

@Jim8y Jim8y Jul 13, 2022

Choose a reason for hiding this comment

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

This is to make sure logs being removed from the memory as soon as possible. Add nother one in the server to ensure logs being removed even if there is no log plugin.


//processing log for block
var blockJson = BlockLogToJson(block, applicationExecutedList);
Expand Down
18 changes: 10 additions & 8 deletions src/RpcServer/RpcServer.SmartContract.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ private JObject GetInvokeResult(byte[] script, Signer[] signers = null, Witness[
lock (sessions)
sessions.Add(id, session);
}
if (session.Engine.ScriptContainer is Transaction tx && LogEvents.ContainsKey(tx.Hash))
json["logs"] = new JArray(LogEvents[tx.Hash].Select(p => new JString(p.Message)));
return json;
}

Expand Down Expand Up @@ -197,7 +199,7 @@ private static Witness[] WitnessesFromJson(JArray _params)
[RpcMethod]
protected virtual JObject InvokeFunction(JArray _params)
{
UInt160 script_hash = UInt160.Parse(_params[0].AsString());
UInt160 scriptHash = UInt160.Parse(_params[0].AsString());
string operation = _params[1].AsString();
ContractParameter[] args = _params.Count >= 3 ? ((JArray)_params[2]).Select(p => ContractParameter.FromJson(p)).ToArray() : System.Array.Empty<ContractParameter>();
Signer[] signers = _params.Count >= 4 ? SignersFromJson((JArray)_params[3], system.Settings) : null;
Expand All @@ -207,7 +209,7 @@ protected virtual JObject InvokeFunction(JArray _params)
byte[] script;
using (ScriptBuilder sb = new())
{
script = sb.EmitDynamicCall(script_hash, operation, args).ToArray();
script = sb.EmitDynamicCall(scriptHash, operation, args).ToArray();
}
return GetInvokeResult(script, signers, witnesses, useDiagnostic);
}
Expand Down Expand Up @@ -260,20 +262,20 @@ protected virtual JObject GetUnclaimedGas(JArray _params)
{
string address = _params[0].AsString();
JObject json = new();
UInt160 script_hash;
UInt160 scriptHash;
try
{
script_hash = AddressToScriptHash(address, system.Settings.AddressVersion);
scriptHash = AddressToScriptHash(address, system.Settings.AddressVersion);
}
catch
{
script_hash = null;
scriptHash = null;
}
if (script_hash == null)
if (scriptHash == null)
throw new RpcException(-100, "Invalid address");
var snapshot = system.StoreView;
json["unclaimed"] = NativeContract.NEO.UnclaimedGas(snapshot, script_hash, NativeContract.Ledger.CurrentIndex(snapshot) + 1).ToString();
json["address"] = script_hash.ToAddress(system.Settings.AddressVersion);
json["unclaimed"] = NativeContract.NEO.UnclaimedGas(snapshot, scriptHash, NativeContract.Ledger.CurrentIndex(snapshot) + 1).ToString();
json["address"] = scriptHash.ToAddress(system.Settings.AddressVersion);
return json;
}

Expand Down
41 changes: 40 additions & 1 deletion src/RpcServer/RpcServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
using Microsoft.AspNetCore.ResponseCompression;
using Microsoft.AspNetCore.Server.Kestrel.Https;
using Microsoft.Extensions.DependencyInjection;
using Neo.IO;
using Neo.IO.Json;
using Neo.Network.P2P;
using System;
Expand All @@ -28,6 +27,9 @@
using System.Security.Cryptography.X509Certificates;
using System.Text;
using System.Threading.Tasks;
using Neo.Ledger;
using Neo.Network.P2P.Payloads;
using Neo.SmartContract;

namespace Neo.Plugins
{
Expand All @@ -39,6 +41,15 @@ public partial class RpcServer : IDisposable
private RpcServerSettings settings;
private readonly NeoSystem system;
private readonly LocalNode localNode;
/// <summary>
/// Public interface of _logEvents.
/// </summary>
public static Dictionary<UInt256, Queue<LogEventArgs>> LogEvents { get; } = new();

/// <summary>
/// Maximum number of events to be logged per contract
/// </summary>
private const int MaxLogEvents = 50;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be configurable?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 @Liaojinghui

Copy link
Member

Choose a reason for hiding this comment

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

Agree with @devhawk


public RpcServer(NeoSystem system, RpcServerSettings settings)
{
Expand All @@ -47,6 +58,14 @@ public RpcServer(NeoSystem system, RpcServerSettings settings)
localNode = system.LocalNode.Ask<LocalNode>(new LocalNode.GetInstance()).Result;
RegisterMethods(this);
Initialize_SmartContract();

ApplicationEngine.Log += OnContractLogEvent;
Blockchain.Committed += OnCommitted;
}

private void OnCommitted(NeoSystem system, Block block)
{
LogEvents.Clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be clearing all the events or just the ones for the transactions in block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should clear all log events, otherwise some logs will be there forever, which will be a memory leak.

Copy link
Member

Choose a reason for hiding this comment

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

If you clear them in OnCommitting, other plugins that depend on this data may not work. I think it should be cleaned up in OnCommitted by a certain plugin.

}

Copy link
Contributor

Choose a reason for hiding this comment

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

Like the variable name change in LogReader.cs, these kind of changes make it harder to review PRs. We should do changes like this in a dedicated PR like neo-project/neo#2785

private bool CheckAuth(HttpContext context)
Expand Down Expand Up @@ -92,8 +111,28 @@ private static JObject CreateResponse(JObject id)
return response;
}

// It is potentially possible to have dos attack by sending a lot of transactions and logs.
// To prevent this, we limit the number of logs to be logged per contract.
// If the number of logs is greater than MAX_LOG_EVENTS, we remove the oldest log.
private static void OnContractLogEvent(object _, LogEventArgs e)
{
if (e.ScriptContainer is not Transaction tx) return;
if (!LogEvents.TryGetValue(tx.Hash, out var _logs))
{
_logs = new Queue<LogEventArgs>();
LogEvents.Add(tx.Hash, _logs);
}
if (LogEvents[tx.Hash].Count >= MaxLogEvents)
{
_logs.Dequeue();
}
_logs.Enqueue(e);
}

public void Dispose()
{
Blockchain.Committed -= OnCommitted;
ApplicationEngine.Log -= OnContractLogEvent;
Dispose_SmartContract();
if (host != null)
{
Expand Down
4 changes: 4 additions & 0 deletions src/RpcServer/RpcServer.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,9 @@
<PackageId>Neo.Plugins.RpcServer</PackageId>
<RootNamespace>Neo.Plugins</RootNamespace>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\RpcClient\RpcClient.csproj" />
Jim8y marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>

</Project>
17 changes: 10 additions & 7 deletions src/RpcServer/RpcServerPlugin.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@

using System.Collections.Generic;
using System.Linq;
using Neo.Ledger;
using Neo.Network.P2P.Payloads;
using Neo.SmartContract;

namespace Neo.Plugins
{
Expand All @@ -30,13 +33,6 @@ protected override void Configure()
server.UpdateSettings(s);
}

public override void Dispose()
{
foreach (var (_, server) in servers)
server.Dispose();
base.Dispose();
}

protected override void OnSystemLoaded(NeoSystem system)
{
RpcServerSettings s = settings.Servers.FirstOrDefault(p => p.Network == system.Settings.Network);
Expand All @@ -56,6 +52,13 @@ protected override void OnSystemLoaded(NeoSystem system)
servers.TryAdd(s.Network, server);
}

public override void Dispose()
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we could revert all the changes to this file in this PR

{
foreach (var (_, server) in servers)
server.Dispose();
base.Dispose();
}

public static void RegisterMethods(object handler, uint network)
{
if (servers.TryGetValue(network, out RpcServer server))
Expand Down