From 709c90f8835dd73c82e8f79aba86d308d50df1d6 Mon Sep 17 00:00:00 2001 From: nan01ab Date: Thu, 20 Feb 2025 12:00:43 +0800 Subject: [PATCH] Optimization: remove some unsafes (#3767) Co-authored-by: Christopher Schuchardt Co-authored-by: Shargon Co-authored-by: NGD Admin <154295625+NGDAdmin@users.noreply.github.com> --- src/Neo/BigDecimal.cs | 48 ++++++++++--------- src/Neo/SmartContract/ApplicationEngine.cs | 32 +++++++++---- src/Neo/UInt160.cs | 11 ++--- src/Neo/UInt256.cs | 17 +++---- .../UT_ApplicationEngineProvider.cs | 28 +++++++++-- 5 files changed, 83 insertions(+), 53 deletions(-) diff --git a/src/Neo/BigDecimal.cs b/src/Neo/BigDecimal.cs index 575e9e33da..17e132f085 100644 --- a/src/Neo/BigDecimal.cs +++ b/src/Neo/BigDecimal.cs @@ -13,6 +13,7 @@ using System; using System.Numerics; +using System.Runtime.InteropServices; namespace Neo { @@ -54,17 +55,19 @@ public BigDecimal(BigInteger value, byte decimals) /// Initializes a new instance of the struct with the value of . /// /// The value of the number. - public unsafe BigDecimal(decimal value) + public BigDecimal(decimal value) { +#if NET5_0_OR_GREATER Span span = stackalloc int[4]; - span = decimal.GetBits(value); - fixed (int* p = span) - { - ReadOnlySpan 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)span); + _value = new BigInteger(buffer[..12], isUnsigned: true); + + if (buffer[15] != 0) _value = -_value; + _decimals = buffer[14]; } /// @@ -72,21 +75,22 @@ public unsafe BigDecimal(decimal value) /// /// The value of the number. /// The number of decimal places for this number. - public unsafe BigDecimal(decimal value, byte decimals) + public BigDecimal(decimal value, byte decimals) { +#if NET5_0_OR_GREATER Span span = stackalloc int[4]; - span = decimal.GetBits(value); - fixed (int* p = span) - { - ReadOnlySpan 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)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; } diff --git a/src/Neo/SmartContract/ApplicationEngine.cs b/src/Neo/SmartContract/ApplicationEngine.cs index a68638cbcf..353cfe3258 100644 --- a/src/Neo/SmartContract/ApplicationEngine.cs +++ b/src/Neo/SmartContract/ApplicationEngine.cs @@ -175,12 +175,18 @@ public virtual UInt160 CallingScriptHash /// The trigger of the execution. /// The container of the script. /// The snapshot used by the engine during execution. - /// The block being persisted. It should be if the is . + /// + /// The block being persisted. + /// It should be if the is . + /// /// The used by the engine. - /// The maximum gas, in the unit of datoshi, used in this execution. The execution will fail when the gas is exhausted. + /// + /// The maximum gas, in the unit of datoshi, used in this execution. + /// The execution will fail when the gas is exhausted. + /// /// The diagnostic to be used by the . /// The jump table to be used by the . - 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(ref nonceData[0]); + nonce ^= persistingBlock.Nonce; } diagnostic?.Initialized(this); } @@ -405,14 +410,21 @@ internal override void UnloadContext(ExecutionContext context) } /// - /// Use the loaded to create a new instance of the class. If no is loaded, the constructor of will be called. + /// Use the loaded to create a new instance of the class. + /// If no is loaded, the constructor of will be called. /// /// The trigger of the execution. /// The container of the script. /// The snapshot used by the engine during execution. - /// The block being persisted. It should be if the is . + /// + /// The block being persisted. + /// It should be if the is . + /// /// The used by the engine. - /// The maximum gas used in this execution, in the unit of datoshi. The execution will fail when the gas is exhausted. + /// + /// The maximum gas used in this execution, in the unit of datoshi. + /// The execution will fail when the gas is exhausted. + /// /// The diagnostic to be used by the . /// The engine instance created. public static ApplicationEngine Create(TriggerType trigger, IVerifiable container, DataCache snapshot, Block persistingBlock = null, ProtocolSettings settings = null, long gas = TestModeGas, IDiagnostic diagnostic = null) diff --git a/src/Neo/UInt160.cs b/src/Neo/UInt160.cs index 4a9d965dee..cb697e0582 100644 --- a/src/Neo/UInt160.cs +++ b/src/Neo/UInt160.cs @@ -49,16 +49,13 @@ public UInt160() { } /// Initializes a new instance of the class. /// /// The value of the . - public unsafe UInt160(ReadOnlySpan value) + public UInt160(ReadOnlySpan value) { if (value.Length != Length) - throw new FormatException(); + throw new FormatException($"Invalid length: {value.Length}"); - fixed (void* p = &_value1) - { - Span dst = new(p, Length); - value[..Length].CopyTo(dst); - } + var span = MemoryMarshal.CreateSpan(ref Unsafe.As(ref _value1), Length); + value[..Length].CopyTo(span); } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/Neo/UInt256.cs b/src/Neo/UInt256.cs index 077fe0358c..581e2f95eb 100644 --- a/src/Neo/UInt256.cs +++ b/src/Neo/UInt256.cs @@ -44,22 +44,19 @@ public class UInt256 : IComparable, IEquatable, ISerializable, /// /// Initializes a new instance of the class. /// - public UInt256() - { - } + public UInt256() { } /// /// Initializes a new instance of the class. /// /// The value of the . - public unsafe UInt256(ReadOnlySpan value) + public UInt256(ReadOnlySpan value) { - if (value.Length != Length) throw new FormatException(); - fixed (ulong* p = &value1) - { - Span 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(ref value1), Length); + value[..Length].CopyTo(span); } public int CompareTo(UInt256 other) diff --git a/tests/Neo.UnitTests/SmartContract/UT_ApplicationEngineProvider.cs b/tests/Neo.UnitTests/SmartContract/UT_ApplicationEngineProvider.cs index 67ca760715..7ba64d108d 100644 --- a/tests/Neo.UnitTests/SmartContract/UT_ApplicationEngineProvider.cs +++ b/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,7 +43,8 @@ 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); } @@ -49,13 +52,29 @@ public void TestSetAppEngineProvider() 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); } @@ -63,7 +82,8 @@ public ApplicationEngine Create(TriggerType trigger, IVerifiable container, Data 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) { }