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.

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

Improve ApplicationEngine #2796

Merged
merged 2 commits into from
Aug 2, 2022
Merged

Improve ApplicationEngine #2796

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4225331
Improve ApplicationEngine
erikzhang Jul 24, 2022
6321f86
Improve CalledByEntryCondition.Match()
erikzhang Aug 1, 2022
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
3 changes: 2 additions & 1 deletion src/Neo/Network/P2P/Payloads/Conditions/CalledByEntryCondition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ protected override void DeserializeWithoutType(ref MemoryReader reader, int maxN

public override bool Match(ApplicationEngine engine)
{
return engine.CallingScriptHash is null || engine.CallingScriptHash == engine.EntryScriptHash;
var state = engine.CurrentContext.GetState<ExecutionContextState>();
return state.CallingContext is null || state.CallingContext == engine.EntryContext;
}

protected override void SerializeWithoutType(BinaryWriter writer)
Expand Down
2 changes: 1 addition & 1 deletion src/Neo/SmartContract/ApplicationEngine.Contract.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected internal void CallContract(UInt160 contractHash, string method, CallFl
bool hasReturnValue = md.ReturnType != ContractParameterType.Void;

ExecutionContext context = CallContractInternal(contract, md, callFlags, hasReturnValue, args);
if (!hasReturnValue) context.GetState<ExecutionContextState>().PushNullWhenReturn = true;
context.GetState<ExecutionContextState>().IsDynamicCall = true;
}

/// <summary>
Expand Down
2 changes: 2 additions & 0 deletions src/Neo/SmartContract/ApplicationEngine.Runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,8 @@ protected internal void RuntimeLog(byte[] state)
protected internal void RuntimeNotify(byte[] eventName, Array state)
{
if (eventName.Length > MaxEventName) throw new ArgumentException(null, nameof(eventName));
if (CurrentContext.GetState<ExecutionContextState>().Contract is null)
throw new InvalidOperationException("Notifications are not allowed in dynamic scripts.");
using MemoryStream ms = new(MaxNotificationSize);
using BinaryWriter writer = new(ms, Utility.StrictUTF8, true);
BinarySerializer.Serialize(writer, state, MaxNotificationSize);
Expand Down
28 changes: 21 additions & 7 deletions src/Neo/SmartContract/ApplicationEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,15 @@ public partial class ApplicationEngine : ExecutionEngine
/// <summary>
/// The script hash of the calling contract. This field could be <see langword="null"/> if the current context is the entry context.
/// </summary>
public UInt160 CallingScriptHash => CurrentContext?.GetState<ExecutionContextState>().CallingScriptHash;
public UInt160 CallingScriptHash
{
get
{
if (CurrentContext is null) return null;
var state = CurrentContext.GetState<ExecutionContextState>();
return state.NativeCallingScriptHash ?? state.CallingContext?.GetState<ExecutionContextState>().ScriptHash;
}
}

/// <summary>
/// The script hash of the entry context. This field could be <see langword="null"/> if no context is loaded to the engine.
Expand Down Expand Up @@ -224,15 +232,15 @@ private ExecutionContext CallContractInternal(ContractState contract, ContractMe
invocationCounter[contract.Hash] = 1;
}

ExecutionContextState state = CurrentContext.GetState<ExecutionContextState>();
UInt160 callingScriptHash = state.ScriptHash;
ExecutionContext currentContext = CurrentContext;
ExecutionContextState state = currentContext.GetState<ExecutionContextState>();
CallFlags callingFlags = state.CallFlags;

if (args.Count != method.Parameters.Length) throw new InvalidOperationException($"Method {method} Expects {method.Parameters.Length} Arguments But Receives {args.Count} Arguments");
if (hasReturnValue ^ (method.ReturnType != ContractParameterType.Void)) throw new InvalidOperationException("The return value type does not match.");
ExecutionContext context_new = LoadContract(contract, method, flags & callingFlags);
state = context_new.GetState<ExecutionContextState>();
state.CallingScriptHash = callingScriptHash;
state.CallingContext = currentContext;

for (int i = args.Count - 1; i >= 0; i--)
context_new.EvaluationStack.Push(args[i]);
Expand All @@ -244,7 +252,7 @@ internal ContractTask CallFromNativeContract(UInt160 callingScriptHash, UInt160
{
ExecutionContext context_new = CallContractInternal(hash, method, CallFlags.All, false, args);
ExecutionContextState state = context_new.GetState<ExecutionContextState>();
state.CallingScriptHash = callingScriptHash;
state.NativeCallingScriptHash = callingScriptHash;
ContractTask task = new();
contractTasks.Add(context_new, task.GetAwaiter());
return task;
Expand All @@ -254,7 +262,7 @@ internal ContractTask<T> CallFromNativeContract<T>(UInt160 callingScriptHash, UI
{
ExecutionContext context_new = CallContractInternal(hash, method, CallFlags.All, true, args);
ExecutionContextState state = context_new.GetState<ExecutionContextState>();
state.CallingScriptHash = callingScriptHash;
state.NativeCallingScriptHash = callingScriptHash;
ContractTask<T> task = new();
contractTasks.Add(context_new, task.GetAwaiter());
return task;
Expand All @@ -273,7 +281,13 @@ protected override void ContextUnloaded(ExecutionContext context)
{
ExecutionContextState contextState = CurrentContext.GetState<ExecutionContextState>();
contextState.NotificationCount += state.NotificationCount;
if (state.PushNullWhenReturn) Push(StackItem.Null);
if (state.IsDynamicCall)
{
if (context.EvaluationStack.Count == 0)
Push(StackItem.Null);
else if (context.EvaluationStack.Count > 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this duplicating https://github.com/neo-project/neo-vm/blob/f48de7e595ade7c72be6ccd5cce0ab99708f0afd/src/Neo.VM/ExecutionEngine.cs#L427 (rvcount normally is 0 or 1 in this case:

neo/src/Neo/SmartContract/ApplicationEngine.cs

Line 331 in 8e68c3f

rvcount: method.ReturnType == ContractParameterType.Void ? 0 : 1,
)? Can this ever happen without #2756?

Copy link
Member Author

Choose a reason for hiding this comment

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

rvcount: method.ReturnType == ContractParameterType.Void ? 0 : 1,

This is in LoadContract(). For a dynamic script, we have no way of knowing in advance how many values it will return. So set rvcount to -1.

https://github.com/neo-project/neo-vm/blob/master/src/Neo.VM/ExecutionEngine.cs#L1580

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. I mean, this change probably can be a part of #2756, it doesn't seem to be useful without it.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can use it as a double check.

throw new NotSupportedException("Multiple return values are not allowed in cross-contract calls.");
}
}
}
else
Expand Down
11 changes: 8 additions & 3 deletions src/Neo/SmartContract/ExecutionContextState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@ public class ExecutionContextState
public UInt160 ScriptHash { get; set; }

/// <summary>
/// The script hash of the calling contract.
/// The calling context.
/// </summary>
public UInt160 CallingScriptHash { get; set; }
public ExecutionContext CallingContext { get; set; }

/// <summary>
/// The script hash of the calling native contract. Used in native contracts only.
/// </summary>
internal UInt160 NativeCallingScriptHash { get; set; }

/// <summary>
/// The <see cref="ContractState"/> of the current context.
Expand All @@ -42,6 +47,6 @@ public class ExecutionContextState

public int NotificationCount { get; set; }

public bool PushNullWhenReturn { get; set; }
public bool IsDynamicCall { get; set; }
}
}
4 changes: 2 additions & 2 deletions tests/Neo.UnitTests/Extensions/NativeContractExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public static void UpdateContract(this DataCache snapshot, UInt160 callingScript
// Fake calling script hash
if (callingScriptHash != null)
{
engine.CurrentContext.GetState<ExecutionContextState>().CallingScriptHash = callingScriptHash;
engine.CurrentContext.GetState<ExecutionContextState>().NativeCallingScriptHash = callingScriptHash;
engine.CurrentContext.GetState<ExecutionContextState>().ScriptHash = callingScriptHash;
}

Expand All @@ -65,7 +65,7 @@ public static void DestroyContract(this DataCache snapshot, UInt160 callingScrip
// Fake calling script hash
if (callingScriptHash != null)
{
engine.CurrentContext.GetState<ExecutionContextState>().CallingScriptHash = callingScriptHash;
engine.CurrentContext.GetState<ExecutionContextState>().NativeCallingScriptHash = callingScriptHash;
engine.CurrentContext.GetState<ExecutionContextState>().ScriptHash = callingScriptHash;
}

Expand Down
1 change: 1 addition & 0 deletions tests/Neo.UnitTests/SmartContract/UT_ApplicationEngine.Runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public void TestNotSupportedNotification()
{
using var engine = ApplicationEngine.Create(TriggerType.Application, null, null, TestBlockchain.TheNeoSystem.GenesisBlock, settings: TestBlockchain.TheNeoSystem.Settings, gas: 1100_00000000);
engine.LoadScript(Array.Empty<byte>());
engine.CurrentContext.GetState<ExecutionContextState>().Contract = new();

// circular

Expand Down
2 changes: 2 additions & 0 deletions tests/Neo.UnitTests/SmartContract/UT_InteropService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ public void Runtime_GetNotifications_Test()
// Execute

engine.LoadScript(script.ToArray());
engine.CurrentContext.GetState<ExecutionContextState>().Contract = new();
var currentScriptHash = engine.EntryScriptHash;

Assert.AreEqual(VMState.HALT, engine.Execute());
Expand Down Expand Up @@ -145,6 +146,7 @@ public void Runtime_GetNotifications_Test()
// Execute

engine.LoadScript(script.ToArray());
engine.CurrentContext.GetState<ExecutionContextState>().Contract = new();
var currentScriptHash = engine.EntryScriptHash;

Assert.AreEqual(VMState.HALT, engine.Execute());
Expand Down