-
Notifications
You must be signed in to change notification settings - Fork 100
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
Changes from 24 commits
a634f59
042db65
86a68cc
a89b625
f7eefd3
5b731eb
724fc54
4546018
ac65c60
ea6d01a
e23f226
c45f703
8888011
46aa190
7b9723e
397458c
391abe0
c4b093d
f7afd69
12ded50
0bd5c11
0aa880b
c0f35ec
8a83e5c
6d15ff0
c0e72ac
9c724dd
68f35f6
5783063
0e3ccb6
12a36c6
b899935
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 | ||
{ | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be configurable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 @Liaojinghui There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @devhawk |
||
|
||
public RpcServer(NeoSystem system, RpcServerSettings settings) | ||
{ | ||
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you clear them in |
||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -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) | ||
{ | ||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.