Optimization: remove some unsafes (#3767)

Co-authored-by: Christopher Schuchardt <cschuchardt88@gmail.com>
Co-authored-by: Shargon <shargon@gmail.com>
Co-authored-by: NGD Admin <154295625+NGDAdmin@users.noreply.github.com>

src/Neo/BigDecimal.cs

@@ -13,6 +13,7 @@
 
 using System;
 using System.Numerics;
+using System.Runtime.InteropServices;
 
 namespace Neo
 {
@@ -54,39 +55,42 @@ public BigDecimal(BigInteger value, byte decimals)
         /// Initializes a new instance of the <see cref="BigDecimal"/> struct with the value of <see cref="decimal"/>.
         /// </summary>
         /// <param name="value">The value of the number.</param>
-        public unsafe BigDecimal(decimal value)
+        public BigDecimal(decimal value)
         {
+#if NET5_0_OR_GREATER
             Span<int> span = stackalloc int[4];
-            span = decimal.GetBits(value);
-            fixed (int* p = span)
-            {
-                ReadOnlySpan<byte> buffer = new(p, 16);
-                _value = new BigInteger(buffer[..12], isUnsigned: true);
-                if (buffer[15] != 0) _value = -_value;
-                _decimals = buffer[14];
-            }
+            decimal.GetBits(value, span);
+#else
+            var span = decimal.GetBits(value);
+#endif
+            var buffer = MemoryMarshal.AsBytes((ReadOnlySpan<int>)span);
+            _value = new BigInteger(buffer[..12], isUnsigned: true);
+
+            if (buffer[15] != 0) _value = -_value;
+            _decimals = buffer[14];
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="BigDecimal"/> struct with the value of <see cref="decimal"/>.
         /// </summary>
         /// <param name="value">The value of the number.</param>
         /// <param name="decimals">The number of decimal places for this number.</param>
-        public unsafe BigDecimal(decimal value, byte decimals)
+        public BigDecimal(decimal value, byte decimals)
         {
+#if NET5_0_OR_GREATER
             Span<int> span = stackalloc int[4];
-            span = decimal.GetBits(value);
-            fixed (int* p = span)
-            {
-                ReadOnlySpan<byte> buffer = new(p, 16);
-                _value = new BigInteger(buffer[..12], isUnsigned: true);
-                if (buffer[14] > decimals)
-                    throw new ArgumentException(null, nameof(value));
-                else if (buffer[14] < decimals)
-                    _value *= BigInteger.Pow(10, decimals - buffer[14]);
-                if (buffer[15] != 0)
-                    _value = -_value;
-            }
+            decimal.GetBits(value, span);
+#else
+            var span = decimal.GetBits(value);
+#endif
+            var buffer = MemoryMarshal.AsBytes((ReadOnlySpan<int>)span);
+            _value = new BigInteger(buffer[..12], isUnsigned: true);
+            if (buffer[14] > decimals)
+                throw new ArgumentException($"Invalid decimals: {buffer[14]}-{decimals}", nameof(value));
+            else if (buffer[14] < decimals)
+                _value *= BigInteger.Pow(10, decimals - buffer[14]);
+
+            if (buffer[15] != 0) _value = -_value;
             _decimals = decimals;
         }
 

src/Neo/SmartContract/ApplicationEngine.cs

@@ -175,12 +175,18 @@ public virtual UInt160 CallingScriptHash
         /// <param name="trigger">The trigger of the execution.</param>
         /// <param name="container">The container of the script.</param>
         /// <param name="snapshotCache">The snapshot used by the engine during execution.</param>
-        /// <param name="persistingBlock">The block being persisted. It should be <see langword="null"/> if the <paramref name="trigger"/> is <see cref="TriggerType.Verification"/>.</param>
+        /// <param name="persistingBlock">
+        /// The block being persisted.
+        /// It should be <see langword="null"/> if the <paramref name="trigger"/> is <see cref="TriggerType.Verification"/>.
+        /// </param>
         /// <param name="settings">The <see cref="Neo.ProtocolSettings"/> used by the engine.</param>
-        /// <param name="gas">The maximum gas, in the unit of datoshi, used in this execution. The execution will fail when the gas is exhausted.</param>
+        /// <param name="gas">
+        /// The maximum gas, in the unit of datoshi, used in this execution.
+        /// The execution will fail when the gas is exhausted.
+        /// </param>
         /// <param name="diagnostic">The diagnostic to be used by the <see cref="ApplicationEngine"/>.</param>
         /// <param name="jumpTable">The jump table to be used by the <see cref="ApplicationEngine"/>.</param>
-        protected unsafe ApplicationEngine(
+        protected ApplicationEngine(
             TriggerType trigger, IVerifiable container, DataCache snapshotCache, Block persistingBlock,
             ProtocolSettings settings, long gas, IDiagnostic diagnostic, JumpTable jumpTable = null)
             : base(jumpTable ?? DefaultJumpTable)
@@ -203,12 +209,11 @@ protected unsafe ApplicationEngine(
                 ExecFeeFactor = NativeContract.Policy.GetExecFeeFactor(snapshotCache);
                 StoragePrice = NativeContract.Policy.GetStoragePrice(snapshotCache);
             }
+
             if (persistingBlock is not null)
             {
-                fixed (byte* p = nonceData)
-                {
-                    *(ulong*)p ^= persistingBlock.Nonce;
-                }
+                ref ulong nonce = ref System.Runtime.CompilerServices.Unsafe.As<byte, ulong>(ref nonceData[0]);
+                nonce ^= persistingBlock.Nonce;
             }
             diagnostic?.Initialized(this);
         }
@@ -405,14 +410,21 @@ internal override void UnloadContext(ExecutionContext context)
         }
 
         /// <summary>
-        /// Use the loaded <see cref="IApplicationEngineProvider"/> to create a new instance of the <see cref="ApplicationEngine"/> class. If no <see cref="IApplicationEngineProvider"/> is loaded, the constructor of <see cref="ApplicationEngine"/> will be called.
+        /// Use the loaded <see cref="IApplicationEngineProvider"/> to create a new instance of the <see cref="ApplicationEngine"/> class.
+        /// If no <see cref="IApplicationEngineProvider"/> is loaded, the constructor of <see cref="ApplicationEngine"/> will be called.
         /// </summary>
         /// <param name="trigger">The trigger of the execution.</param>
         /// <param name="container">The container of the script.</param>
         /// <param name="snapshot">The snapshot used by the engine during execution.</param>
-        /// <param name="persistingBlock">The block being persisted. It should be <see langword="null"/> if the <paramref name="trigger"/> is <see cref="TriggerType.Verification"/>.</param>
+        /// <param name="persistingBlock">
+        /// The block being persisted.
+        /// It should be <see langword="null"/> if the <paramref name="trigger"/> is <see cref="TriggerType.Verification"/>.
+        /// </param>
         /// <param name="settings">The <see cref="Neo.ProtocolSettings"/> used by the engine.</param>
-        /// <param name="gas">The maximum gas used in this execution, in the unit of datoshi. The execution will fail when the gas is exhausted.</param>
+        /// <param name="gas">
+        /// The maximum gas used in this execution, in the unit of datoshi.
+        /// The execution will fail when the gas is exhausted.
+        /// </param>
         /// <param name="diagnostic">The diagnostic to be used by the <see cref="ApplicationEngine"/>.</param>
         /// <returns>The engine instance created.</returns>
         public static ApplicationEngine Create(TriggerType trigger, IVerifiable container, DataCache snapshot, Block persistingBlock = null, ProtocolSettings settings = null, long gas = TestModeGas, IDiagnostic diagnostic = null)

src/Neo/UInt160.cs

@@ -49,16 +49,13 @@ public UInt160() { }
         /// Initializes a new instance of the <see cref="UInt160"/> class.
         /// </summary>
         /// <param name="value">The value of the <see cref="UInt160"/>.</param>
-        public unsafe UInt160(ReadOnlySpan<byte> value)
+        public UInt160(ReadOnlySpan<byte> value)
         {
             if (value.Length != Length)
-                throw new FormatException();
+                throw new FormatException($"Invalid length: {value.Length}");
 
-            fixed (void* p = &_value1)
-            {
-                Span<byte> dst = new(p, Length);
-                value[..Length].CopyTo(dst);
-            }
+            var span = MemoryMarshal.CreateSpan(ref Unsafe.As<ulong, byte>(ref _value1), Length);
+            value[..Length].CopyTo(span);
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]

src/Neo/UInt256.cs

@@ -44,22 +44,19 @@ public class UInt256 : IComparable<UInt256>, IEquatable<UInt256>, ISerializable,
         /// <summary>
         /// Initializes a new instance of the <see cref="UInt256"/> class.
         /// </summary>
-        public UInt256()
-        {
-        }
+        public UInt256() { }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="UInt256"/> class.
         /// </summary>
         /// <param name="value">The value of the <see cref="UInt256"/>.</param>
-        public unsafe UInt256(ReadOnlySpan<byte> value)
+        public UInt256(ReadOnlySpan<byte> value)
         {
-            if (value.Length != Length) throw new FormatException();
-            fixed (ulong* p = &value1)
-            {
-                Span<byte> dst = new(p, Length);
-                value[..Length].CopyTo(dst);
-            }
+            if (value.Length != Length)
+                throw new FormatException($"Invalid length: {value.Length}");
+
+            var span = MemoryMarshal.CreateSpan(ref Unsafe.As<ulong, byte>(ref value1), Length);
+            value[..Length].CopyTo(span);
         }
 
         public int CompareTo(UInt256 other)

tests/Neo.UnitTests/SmartContract/UT_ApplicationEngineProvider.cs

@@ -10,10 +10,12 @@
 // modifications are permitted.
 
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Neo.Extensions;
 using Neo.Network.P2P.Payloads;
 using Neo.Persistence;
 using Neo.SmartContract;
 using Neo.VM;
+using System.Reflection;
 
 namespace Neo.UnitTests.SmartContract
 {
@@ -41,29 +43,47 @@ public void TestSetAppEngineProvider()
             ApplicationEngine.Provider = new TestProvider();
             var snapshot = _snapshotCache.CloneCache();
 
-            using var appEngine = ApplicationEngine.Create(TriggerType.Application, null, snapshot, gas: 0, settings: TestBlockchain.TheNeoSystem.Settings);
+            using var appEngine = ApplicationEngine.Create(TriggerType.Application,
+                null, snapshot, gas: 0, settings: TestBlockchain.TheNeoSystem.Settings);
             Assert.IsTrue(appEngine is TestEngine);
         }
 
         [TestMethod]
         public void TestDefaultAppEngineProvider()
         {
             var snapshot = _snapshotCache.CloneCache();
-            using var appEngine = ApplicationEngine.Create(TriggerType.Application, null, snapshot, gas: 0, settings: TestBlockchain.TheNeoSystem.Settings);
+            using var appEngine = ApplicationEngine.Create(TriggerType.Application,
+                null, snapshot, gas: 0, settings: TestBlockchain.TheNeoSystem.Settings);
             Assert.IsTrue(appEngine is ApplicationEngine);
         }
 
+        [TestMethod]
+        public void TestInitNonce()
+        {
+            var block = new Block { Header = new() { Nonce = 0x0102030405060708 } };
+            using var app = new TestEngine(TriggerType.Application,
+                null, null, block, TestBlockchain.TheNeoSystem.Settings, 0, null, null);
+
+            var nonceData = typeof(ApplicationEngine)
+                .GetField("nonceData", BindingFlags.NonPublic | BindingFlags.Instance)
+                .GetValue(app) as byte[];
+            Assert.IsNotNull(nonceData);
+            Assert.AreEqual(nonceData.ToHexString(), "08070605040302010000000000000000");
+        }
+
         class TestProvider : IApplicationEngineProvider
         {
-            public ApplicationEngine Create(TriggerType trigger, IVerifiable container, DataCache snapshot, Block persistingBlock, ProtocolSettings settings, long gas, IDiagnostic diagnostic, JumpTable jumpTable)
+            public ApplicationEngine Create(TriggerType trigger, IVerifiable container, DataCache snapshot,
+                Block persistingBlock, ProtocolSettings settings, long gas, IDiagnostic diagnostic, JumpTable jumpTable)
             {
                 return new TestEngine(trigger, container, snapshot, persistingBlock, settings, gas, diagnostic, jumpTable);
             }
         }
 
         class TestEngine : ApplicationEngine
         {
-            public TestEngine(TriggerType trigger, IVerifiable container, DataCache snapshotCache, Block persistingBlock, ProtocolSettings settings, long gas, IDiagnostic diagnostic, JumpTable jumpTable)
+            public TestEngine(TriggerType trigger, IVerifiable container, DataCache snapshotCache,
+                Block persistingBlock, ProtocolSettings settings, long gas, IDiagnostic diagnostic, JumpTable jumpTable)
                 : base(trigger, container, snapshotCache, persistingBlock, settings, gas, diagnostic, jumpTable)
             {
             }