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

Jump to bottom

[VM] rename vm catchable and uncatchable exceptions #3538

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[VM] rename vm catchable and uncatchable exceptions #3538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
10c2d71
rename vm catchable and uncatchable exceptions
Jim8y Oct 17, 2024
b59cc87
format
Jim8y Oct 17, 2024
7e4e662
format
Jim8y Oct 17, 2024
3d8c357
update exception
Jim8y Oct 17, 2024
7fdc631
update exception
Jim8y Oct 17, 2024
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
14 changes: 7 additions & 7 deletions src/Neo.VM/EvaluationStack.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ IEnumerator IEnumerable.GetEnumerator()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void Insert(int index, StackItem item)
{
if (index > innerList.Count) throw new InvalidOperationException($"Insert out of bounds: {index}/{innerList.Count}");
if (index > innerList.Count) throw new VMUncatchableException($"Insert out of bounds: {index}/{innerList.Count}");
innerList.Insert(innerList.Count - index, item);
referenceCounter.AddStackReference(item);
}
Expand All @@ -92,11 +92,11 @@ internal void MoveTo(EvaluationStack stack, int count = -1)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public StackItem Peek(int index = 0)
{
if (index >= innerList.Count) throw new InvalidOperationException($"Peek out of bounds: {index}/{innerList.Count}");
if (index >= innerList.Count) throw new VMUncatchableException($"Peek out of bounds: {index}/{innerList.Count}");
if (index < 0)
{
index += innerList.Count;
if (index < 0) throw new InvalidOperationException($"Peek out of bounds: {index}/{innerList.Count}");
if (index < 0) throw new VMUncatchableException($"Peek out of bounds: {index}/{innerList.Count}");
}
return innerList[innerList.Count - index - 1];
}
Expand All @@ -118,7 +118,7 @@ public void Push(StackItem item)
internal void Reverse(int n)
{
if (n < 0 || n > innerList.Count)
throw new ArgumentOutOfRangeException(nameof(n));
throw new VMUncatchableException($"Reverse operation out of bounds: {n}/{innerList.Count}");
if (n <= 1) return;
innerList.Reverse(innerList.Count - n, n);
}
Expand Down Expand Up @@ -147,16 +147,16 @@ public T Pop<T>() where T : StackItem
internal T Remove<T>(int index) where T : StackItem
{
if (index >= innerList.Count)
throw new ArgumentOutOfRangeException(nameof(index));
throw new VMUncatchableException($"Remove out of bounds: {index}/{innerList.Count}");
if (index < 0)
{
index += innerList.Count;
if (index < 0)
throw new ArgumentOutOfRangeException(nameof(index));
throw new VMUncatchableException($"Remove out of bounds: {index}/{innerList.Count}");
}
index = innerList.Count - index - 1;
if (innerList[index] is not T item)
throw new InvalidCastException($"The item can't be casted to type {typeof(T)}");
throw new VMUncatchableException($"The item of type {innerList[index].Type} can't be casted to type {typeof(T)}");
innerList.RemoveAt(index);
referenceCounter.RemoveStackReference(item);
return item;
Expand Down
4 changes: 2 additions & 2 deletions src/Neo.VM/ExecutionEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public class ExecutionEngine : IDisposable
/// <summary>
/// The VM object representing the uncaught exception.
/// </summary>
public StackItem? UncaughtException { get; internal set; }
public StackItem? UncaughtVMCatchableException { get; internal set; }
Copy link
Member

Choose a reason for hiding this comment

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

I think that we don't need to rename this one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not a problem, if you think this pr is ok, will revert it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, you want to throw only VMXXExceptions, ins't it? maybe we should have and interface for valid exceptions

Copy link
Member

@shargon shargon Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

something like IVMException , And catcheable and UnCatcheable


/// <summary>
/// The current state of the VM.
Expand Down Expand Up @@ -148,7 +148,7 @@ protected internal void ExecuteNext()
{
JumpTable[instruction.OpCode](this, instruction);
}
catch (CatchableException ex) when (Limits.CatchEngineExceptions)
catch (VMCatchableException ex) when (Limits.CatchEngineExceptions)
{
JumpTable.ExecuteThrow(this, ex.Message);
}
Expand Down
12 changes: 6 additions & 6 deletions src/Neo.VM/JumpTable/JumpTable.Compound.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,14 +375,14 @@ public virtual void PickItem(ExecutionEngine engine, Instruction instruction)
{
var index = (int)key.GetInteger();
if (index < 0 || index >= array.Count)
throw new CatchableException($"The value {index} is out of range.");
throw new VMCatchableException($"The value {index} is out of range.");
engine.Push(array[index]);
break;
}
case Map map:
{
if (!map.TryGetValue(key, out var value))
throw new CatchableException($"Key not found in {nameof(Map)}");
throw new VMCatchableException($"Key not found in {nameof(Map)}");
engine.Push(value);
break;
}
Expand All @@ -391,15 +391,15 @@ public virtual void PickItem(ExecutionEngine engine, Instruction instruction)
var byteArray = primitive.GetSpan();
var index = (int)key.GetInteger();
if (index < 0 || index >= byteArray.Length)
throw new CatchableException($"The value {index} is out of range.");
throw new VMCatchableException($"The value {index} is out of range.");
engine.Push((BigInteger)byteArray[index]);
break;
}
case Types.Buffer buffer:
{
var index = (int)key.GetInteger();
if (index < 0 || index >= buffer.Size)
throw new CatchableException($"The value {index} is out of range.");
throw new VMCatchableException($"The value {index} is out of range.");
engine.Push((BigInteger)buffer.InnerBuffer.Span[index]);
break;
}
Expand Down Expand Up @@ -444,7 +444,7 @@ public virtual void SetItem(ExecutionEngine engine, Instruction instruction)
{
var index = (int)key.GetInteger();
if (index < 0 || index >= array.Count)
throw new CatchableException($"The value {index} is out of range.");
throw new VMCatchableException($"The value {index} is out of range.");
array[index] = value;
break;
}
Expand All @@ -457,7 +457,7 @@ public virtual void SetItem(ExecutionEngine engine, Instruction instruction)
{
var index = (int)key.GetInteger();
if (index < 0 || index >= buffer.Size)
throw new CatchableException($"The value {index} is out of range.");
throw new VMCatchableException($"The value {index} is out of range.");
if (value is not PrimitiveType p)
throw new InvalidOperationException($"Value must be a primitive type in {instruction.OpCode}");
var b = (int)p.GetInteger();
Expand Down
12 changes: 6 additions & 6 deletions src/Neo.VM/JumpTable/JumpTable.Control.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,10 +517,10 @@ public virtual void EndFinally(ExecutionEngine engine, Instruction instruction)
if (!engine.CurrentContext.TryStack.TryPop(out var currentTry))
throw new InvalidOperationException($"The corresponding TRY block cannot be found.");

if (engine.UncaughtException is null)
if (engine.UncaughtVMCatchableException is null)
engine.CurrentContext.InstructionPointer = currentTry.EndPointer;
else
ExecuteThrow(engine, engine.UncaughtException);
ExecuteThrow(engine, engine.UncaughtVMCatchableException);

engine.isJumping = true;
}
Expand Down Expand Up @@ -659,7 +659,7 @@ public virtual void ExecuteTry(ExecutionEngine engine, int catchOffset, int fina
/// <param name="ex">The exception to throw.</param>
public virtual void ExecuteThrow(ExecutionEngine engine, StackItem? ex)
{
engine.UncaughtException = ex;
engine.UncaughtVMCatchableException = ex;

var pop = 0;
foreach (var executionContext in engine.InvocationStack)
Expand All @@ -680,9 +680,9 @@ public virtual void ExecuteThrow(ExecutionEngine engine, StackItem? ex)
if (tryContext.State == ExceptionHandlingState.Try && tryContext.HasCatch)
{
tryContext.State = ExceptionHandlingState.Catch;
engine.Push(engine.UncaughtException!);
engine.Push(engine.UncaughtVMCatchableException!);
executionContext.InstructionPointer = tryContext.CatchPointer;
engine.UncaughtException = null;
engine.UncaughtVMCatchableException = null;
}
else
{
Expand All @@ -696,7 +696,7 @@ public virtual void ExecuteThrow(ExecutionEngine engine, StackItem? ex)
++pop;
}

throw new VMUnhandledException(engine.UncaughtException!);
throw new VMUnhandledException(engine.UncaughtVMCatchableException!);
}

#endregion
Expand Down
8 changes: 5 additions & 3 deletions src/Neo.VM/CatchableException.cs → src/Neo.VM/VMCatchableException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (C) 2015-2024 The Neo Project.
//
// CatchableException.cs file belongs to the neo project and is free
// VMCatchableException.cs file belongs to the neo project and is free
// software distributed under the MIT software license, see the
// accompanying file LICENSE in the main directory of the
// repository or http://www.opensource.org/licenses/mit-license.php
Expand All @@ -13,10 +13,12 @@

namespace Neo.VM
{
public class CatchableException : Exception
public class VMCatchableException : Exception
{
public CatchableException(string message) : base(message)
public VMCatchableException(string message) : base(message)
{
if (string.IsNullOrEmpty(message))
throw new ArgumentException("Message cannot be null or empty.", nameof(message));
Comment on lines +20 to +21
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to avoid this, this error will be throw during runtime, is not a warning for us

}
}
}
23 changes: 23 additions & 0 deletions src/Neo.VM/VMUncatchableException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (C) 2015-2024 The Neo Project.
//
// VMUncatchableException.cs file belongs to the neo project and is free
// software distributed under the MIT software license, see the
// accompanying file LICENSE in the main directory of the
// repository or http://www.opensource.org/licenses/mit-license.php
// for more details.
//
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using System;

namespace Neo.VM;

public class VMUncatchableException : Exception
{
public VMUncatchableException(string message) : base(message)
{
if (string.IsNullOrEmpty(message))
throw new ArgumentException("Message cannot be null or empty.", nameof(message));
Comment on lines +20 to +21
Copy link
Member

Choose a reason for hiding this comment

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

Same here

}
}
12 changes: 6 additions & 6 deletions src/Neo/SmartContract/ApplicationEngine.Contract.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ protected internal void CallContract(UInt160 contractHash, string method, CallFl
throw new ArgumentOutOfRangeException(nameof(callFlags));

ContractState contract = NativeContract.ContractManagement.GetContract(SnapshotCache, contractHash);
if (contract is null) throw new InvalidOperationException($"Called Contract Does Not Exist: {contractHash}.{method}");
if (contract is null) throw new VMUncatchableException($"Called Contract Does Not Exist: {contractHash}.{method}");
ContractMethodDescriptor md = contract.Manifest.Abi.GetMethod(method, args.Count);
if (md is null) throw new InvalidOperationException($"Method \"{method}\" with {args.Count} parameter(s) doesn't exist in the contract {contractHash}.");
if (md is null) throw new VMUncatchableException($"Method \"{method}\" with {args.Count} parameter(s) doesn't exist in the contract {contractHash}.");
bool hasReturnValue = md.ReturnType != ContractParameterType.Void;

ExecutionContext context = CallContractInternal(contract, md, callFlags, hasReturnValue, args);
Expand All @@ -95,9 +95,9 @@ protected internal void CallNativeContract(byte version)
{
NativeContract contract = NativeContract.GetContract(CurrentScriptHash);
if (contract is null)
throw new InvalidOperationException("It is not allowed to use \"System.Contract.CallNative\" directly.");
throw new VMUncatchableException("It is not allowed to use \"System.Contract.CallNative\" directly.");
if (!contract.IsActive(ProtocolSettings, NativeContract.Ledger.CurrentIndex(SnapshotCache)))
throw new InvalidOperationException($"The native contract {contract.Name} is not active.");
throw new VMUncatchableException($"The native contract {contract.Name} is not active.");
contract.Invoke(this, version);
}

Expand Down Expand Up @@ -154,7 +154,7 @@ protected internal async void NativeOnPersistAsync()
try
{
if (Trigger != TriggerType.OnPersist)
throw new InvalidOperationException();
throw new VMUncatchableException("Trigger type must be 'OnPersist'.");
foreach (NativeContract contract in NativeContract.Contracts)
{
if (contract.IsActive(ProtocolSettings, PersistingBlock.Index))
Expand All @@ -176,7 +176,7 @@ protected internal async void NativePostPersistAsync()
try
{
if (Trigger != TriggerType.PostPersist)
throw new InvalidOperationException();
throw new VMUncatchableException("Trigger type must be 'PostPersist'.");
foreach (NativeContract contract in NativeContract.Contracts)
{
if (contract.IsActive(ProtocolSettings, PersistingBlock.Index))
Expand Down
20 changes: 10 additions & 10 deletions src/Neo/SmartContract/ApplicationEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ protected internal void AddFee(long datoshi)
FeeConsumed = GasConsumed = checked(FeeConsumed + datoshi);
#pragma warning restore CS0618 // Type or member is obsolete
if (FeeConsumed > _feeAmount)
throw new InvalidOperationException("Insufficient GAS.");
throw new VMUncatchableException("Insufficient GAS.");
}

protected override void OnFault(Exception ex)
Expand All @@ -281,16 +281,16 @@ internal void Throw(Exception ex)
private ExecutionContext CallContractInternal(UInt160 contractHash, string method, CallFlags flags, bool hasReturnValue, StackItem[] args)
{
ContractState contract = NativeContract.ContractManagement.GetContract(SnapshotCache, contractHash);
if (contract is null) throw new InvalidOperationException($"Called Contract Does Not Exist: {contractHash}");
if (contract is null) throw new VMUncatchableException($"Called Contract Does Not Exist: {contractHash}");
ContractMethodDescriptor md = contract.Manifest.Abi.GetMethod(method, args.Length);
if (md is null) throw new InvalidOperationException($"Method \"{method}\" with {args.Length} parameter(s) doesn't exist in the contract {contractHash}.");
if (md is null) throw new VMUncatchableException($"Method \"{method}\" with {args.Length} parameter(s) doesn't exist in the contract {contractHash}.");
return CallContractInternal(contract, md, flags, hasReturnValue, args);
}

private ExecutionContext CallContractInternal(ContractState contract, ContractMethodDescriptor method, CallFlags flags, bool hasReturnValue, IReadOnlyList<StackItem> args)
{
if (NativeContract.Policy.IsBlocked(SnapshotCache, contract.Hash))
throw new InvalidOperationException($"The contract {contract.Hash} has been blocked.");
throw new VMUncatchableException($"The contract {contract.Hash} has been blocked.");

ExecutionContext currentContext = CurrentContext;
ExecutionContextState state = currentContext.GetState<ExecutionContextState>();
Expand All @@ -304,7 +304,7 @@ private ExecutionContext CallContractInternal(ContractState contract, ContractMe
? state.Contract // use executing contract state to avoid possible contract update/destroy side-effects, ref. https://github.com/neo-project/neo/pull/3290.
: NativeContract.ContractManagement.GetContract(SnapshotCache, CurrentScriptHash);
if (executingContract?.CanCall(contract, method.Name) == false)
throw new InvalidOperationException($"Cannot Call Method {method.Name} Of Contract {contract.Hash} From Contract {CurrentScriptHash}");
throw new VMUncatchableException($"Cannot Call Method {method.Name} Of Contract {contract.Hash} From Contract {CurrentScriptHash}");
}

if (invocationCounter.TryGetValue(contract.Hash, out var counter))
Expand Down Expand Up @@ -356,7 +356,7 @@ internal override void UnloadContext(ExecutionContext context)
if (context.Script != CurrentContext?.Script)
{
ExecutionContextState state = context.GetState<ExecutionContextState>();
if (UncaughtException is null)
if (UncaughtVMCatchableException is null)
{
state.SnapshotCache?.Commit();
if (CurrentContext != null)
Expand All @@ -368,7 +368,7 @@ internal override void UnloadContext(ExecutionContext context)
if (context.EvaluationStack.Count == 0)
Push(StackItem.Null);
else if (context.EvaluationStack.Count > 1)
throw new NotSupportedException("Multiple return values are not allowed in cross-contract calls.");
throw new VMUncatchableException("Multiple return values are not allowed in cross-contract calls.");
}
}
}
Expand All @@ -381,8 +381,8 @@ internal override void UnloadContext(ExecutionContext context)
Diagnostic?.ContextUnloaded(context);
if (contractTasks.Remove(context, out var awaiter))
{
if (UncaughtException is not null)
throw new VMUnhandledException(UncaughtException);
if (UncaughtVMCatchableException is not null)
throw new VMUnhandledException(UncaughtVMCatchableException);
awaiter.SetResult(this);
}
}
Expand Down Expand Up @@ -568,7 +568,7 @@ internal protected void ValidateCallFlags(CallFlags requiredCallFlags)
{
ExecutionContextState state = CurrentContext.GetState<ExecutionContextState>();
if (!state.CallFlags.HasFlag(requiredCallFlags))
throw new InvalidOperationException($"Cannot call this SYSCALL with the flag {state.CallFlags}.");
throw new VMUncatchableException($"Cannot call this SYSCALL with the flag {state.CallFlags}.");
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion tests/Neo.UnitTests/SmartContract/UT_InteropService.NEO.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public void TestContract_Create()
var manifest = TestUtils.CreateDefaultManifest();
Assert.ThrowsException<InvalidOperationException>(() => snapshotCache.DeployContract(null, nefFile, manifest.ToJson().ToByteArray(false)));
Assert.ThrowsException<ArgumentException>(() => snapshotCache.DeployContract(UInt160.Zero, nefFile, new byte[ContractManifest.MaxLength + 1]));
Assert.ThrowsException<InvalidOperationException>(() => snapshotCache.DeployContract(UInt160.Zero, nefFile, manifest.ToJson().ToByteArray(true), 10000000));
Assert.ThrowsException<VMUncatchableException>(() => snapshotCache.DeployContract(UInt160.Zero, nefFile, manifest.ToJson().ToByteArray(true), 10000000));

var script_exceedMaxLength = new NefFile()
{
Expand Down
Loading
Loading