From a6b5118d11ff82066676818b6cc1be9ba97854df Mon Sep 17 00:00:00 2001 From: Christopher Schuchardt Date: Sat, 9 Nov 2024 16:03:50 -0500 Subject: [PATCH 1/8] Added IEquatable to Signer --- src/Neo/Network/P2P/Payloads/Signer.cs | 45 ++++++++++++++++++- .../Network/P2P/Payloads/UT_Signers.cs | 42 +++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/Neo/Network/P2P/Payloads/Signer.cs b/src/Neo/Network/P2P/Payloads/Signer.cs index 40496dfe74..d828f723bc 100644 --- a/src/Neo/Network/P2P/Payloads/Signer.cs +++ b/src/Neo/Network/P2P/Payloads/Signer.cs @@ -26,7 +26,7 @@ namespace Neo.Network.P2P.Payloads /// /// Represents a signer of a . /// - public class Signer : IInteroperable, ISerializable + public class Signer : IInteroperable, ISerializable, IEquatable { // This limits maximum number of AllowedContracts or AllowedGroups here private const int MaxSubitems = 16; @@ -66,6 +66,33 @@ public class Signer : IInteroperable, ISerializable /*AllowedGroups*/ (Scopes.HasFlag(WitnessScope.CustomGroups) ? AllowedGroups.GetVarSize() : 0) + /*Rules*/ (Scopes.HasFlag(WitnessScope.WitnessRules) ? Rules.GetVarSize() : 0); + public bool Equals(Signer other) + { + if (ReferenceEquals(this, other)) + return true; + return Account == other.Account && + Scopes == other.Scopes && + AllowedContracts.SequenceEqual(other.AllowedContracts) && + AllowedGroups.SequenceEqual(other.AllowedGroups) && + Rules.SequenceEqual(other.Rules); + } + + public override bool Equals(object obj) + { + if (obj == null) + return false; + + if (obj is not Signer signerObj) + return false; + else + return Equals(signerObj); + } + + public override int GetHashCode() + { + return HashCode.Combine(Account.GetHashCode(), Scopes); + } + public void Deserialize(ref MemoryReader reader) { Account = reader.ReadSerializable(); @@ -202,5 +229,21 @@ VM.Types.StackItem IInteroperable.ToStackItem(IReferenceCounter referenceCounter Scopes.HasFlag(WitnessScope.WitnessRules) ? new VM.Types.Array(referenceCounter, Rules.Select(u => u.ToStackItem(referenceCounter))) : new VM.Types.Array(referenceCounter) ]); } + + public static bool operator ==(Signer left, Signer right) + { + if (((object)left) == null || ((object)right) == null) + return Equals(left, right); + + return left.Equals(right); + } + + public static bool operator !=(Signer left, Signer right) + { + if (((object)left) == null || ((object)right) == null) + return !Equals(left, right); + + return !(left.Equals(right)); + } } } diff --git a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs index d2358e947c..73a93f9e74 100644 --- a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs +++ b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs @@ -23,6 +23,48 @@ namespace Neo.UnitTests.Network.P2P.Payloads [TestClass] public class UT_Signers { + [TestMethod] + public void Test_IEquatable() + { + var expected = new Signer() + { + Account = UInt160.Zero, + Scopes = WitnessScope.Global, + AllowedContracts = [], + AllowedGroups = [], + Rules = [] + }; + + var actual = new Signer() + { + Account = UInt160.Zero, + Scopes = WitnessScope.Global, + AllowedContracts = [], + AllowedGroups = [], + Rules = [] + }; + + var notEqual = new Signer() + { + Account = UInt160.Zero, + Scopes = WitnessScope.WitnessRules, + AllowedContracts = [], + AllowedGroups = [], + Rules = [] + }; + + Assert.IsTrue(expected.Equals(expected)); + + Assert.AreEqual(expected, actual); + Assert.IsTrue(expected == actual); + Assert.IsTrue(expected.Equals(actual)); + + Assert.AreNotEqual(expected, notEqual); + Assert.IsTrue(expected != notEqual); + Assert.IsFalse(expected.Equals(notEqual)); + } + + [TestMethod] public void Serialize_Deserialize_Global() { From ce2d4a7d42cece0fd2cae1cc444319f1b746375d Mon Sep 17 00:00:00 2001 From: Christopher Schuchardt Date: Sat, 9 Nov 2024 16:07:33 -0500 Subject: [PATCH 2/8] Added `null` check to unit tests --- tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs index 73a93f9e74..8f67437bd7 100644 --- a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs +++ b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs @@ -62,6 +62,9 @@ public void Test_IEquatable() Assert.AreNotEqual(expected, notEqual); Assert.IsTrue(expected != notEqual); Assert.IsFalse(expected.Equals(notEqual)); + + Assert.IsFalse(expected == null); + Assert.IsFalse(null == expected); } From ee6a7e950918320c5ac30a1505822f620ab5114d Mon Sep 17 00:00:00 2001 From: Christopher Schuchardt Date: Sun, 10 Nov 2024 22:27:23 -0500 Subject: [PATCH 3/8] Added better testing of Signer --- tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs index 8f67437bd7..dd2f591125 100644 --- a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs +++ b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs @@ -26,12 +26,13 @@ public class UT_Signers [TestMethod] public void Test_IEquatable() { + var ecPoint = ECPoint.Parse("03b209fd4f53a7170ea4444e0cb0a6bb6a53c2bd016926989cf85f9b0fba17a70c", ECCurve.Secp256r1); var expected = new Signer() { Account = UInt160.Zero, Scopes = WitnessScope.Global, - AllowedContracts = [], - AllowedGroups = [], + AllowedContracts = [UInt160.Zero], + AllowedGroups = [ecPoint], Rules = [] }; @@ -39,8 +40,8 @@ public void Test_IEquatable() { Account = UInt160.Zero, Scopes = WitnessScope.Global, - AllowedContracts = [], - AllowedGroups = [], + AllowedContracts = [UInt160.Zero], + AllowedGroups = [ecPoint], Rules = [] }; From ae24f0a2643fbd67dcc91d0d0f4a275d63a2c855 Mon Sep 17 00:00:00 2001 From: Shargon Date: Mon, 11 Nov 2024 08:47:30 +0100 Subject: [PATCH 4/8] Apply suggestions from code review --- src/Neo/Network/P2P/Payloads/Signer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Neo/Network/P2P/Payloads/Signer.cs b/src/Neo/Network/P2P/Payloads/Signer.cs index d828f723bc..4e23752acb 100644 --- a/src/Neo/Network/P2P/Payloads/Signer.cs +++ b/src/Neo/Network/P2P/Payloads/Signer.cs @@ -232,7 +232,7 @@ VM.Types.StackItem IInteroperable.ToStackItem(IReferenceCounter referenceCounter public static bool operator ==(Signer left, Signer right) { - if (((object)left) == null || ((object)right) == null) + if (left is null || right is null) return Equals(left, right); return left.Equals(right); @@ -240,7 +240,7 @@ VM.Types.StackItem IInteroperable.ToStackItem(IReferenceCounter referenceCounter public static bool operator !=(Signer left, Signer right) { - if (((object)left) == null || ((object)right) == null) + if (left is null || right is null) return !Equals(left, right); return !(left.Equals(right)); From 3a1ec392d7ba7635525f9b3b1b559ee60fd20f9d Mon Sep 17 00:00:00 2001 From: Christopher Schuchardt Date: Mon, 11 Nov 2024 18:37:15 -0500 Subject: [PATCH 5/8] Added WitnessRules checking to the unit tests --- .../Network/P2P/Payloads/UT_Signers.cs | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs index dd2f591125..db31ecaa71 100644 --- a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs +++ b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs @@ -33,7 +33,16 @@ public void Test_IEquatable() Scopes = WitnessScope.Global, AllowedContracts = [UInt160.Zero], AllowedGroups = [ecPoint], - Rules = [] + Rules = [ + new WitnessRule + { + Condition = new BooleanCondition + { + Expression = true, + }, + Action = WitnessRuleAction.Allow, + }, + ] }; var actual = new Signer() @@ -42,7 +51,16 @@ public void Test_IEquatable() Scopes = WitnessScope.Global, AllowedContracts = [UInt160.Zero], AllowedGroups = [ecPoint], - Rules = [] + Rules = [ + new WitnessRule + { + Condition = new BooleanCondition + { + Expression = true, + }, + Action = WitnessRuleAction.Allow, + }, + ] }; var notEqual = new Signer() From 115616c5d37657d22b9272f4b7e51b7758679d7c Mon Sep 17 00:00:00 2001 From: Christopher Schuchardt Date: Mon, 11 Nov 2024 18:58:01 -0500 Subject: [PATCH 6/8] Added `null` check to `Equals(other)` --- src/Neo/Network/P2P/Payloads/Signer.cs | 1 + tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/Neo/Network/P2P/Payloads/Signer.cs b/src/Neo/Network/P2P/Payloads/Signer.cs index 4e23752acb..ed1ca01f92 100644 --- a/src/Neo/Network/P2P/Payloads/Signer.cs +++ b/src/Neo/Network/P2P/Payloads/Signer.cs @@ -70,6 +70,7 @@ public bool Equals(Signer other) { if (ReferenceEquals(this, other)) return true; + if (other is null) return false; return Account == other.Account && Scopes == other.Scopes && AllowedContracts.SequenceEqual(other.AllowedContracts) && diff --git a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs index db31ecaa71..0ac918466f 100644 --- a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs +++ b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs @@ -84,6 +84,8 @@ public void Test_IEquatable() Assert.IsFalse(expected == null); Assert.IsFalse(null == expected); + Assert.AreNotEqual(expected, null); + Assert.IsFalse(expected.Equals(null)); } From 4e1bc8ce7cbd2f70c9a27f939e3f6a3f8f084884 Mon Sep 17 00:00:00 2001 From: Christopher Schuchardt Date: Mon, 11 Nov 2024 22:42:47 -0500 Subject: [PATCH 7/8] Added `AggressiveInlining` and Compacked `Equals` --- src/Neo/Network/P2P/Payloads/Signer.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Neo/Network/P2P/Payloads/Signer.cs b/src/Neo/Network/P2P/Payloads/Signer.cs index ed1ca01f92..75fb8d9e71 100644 --- a/src/Neo/Network/P2P/Payloads/Signer.cs +++ b/src/Neo/Network/P2P/Payloads/Signer.cs @@ -20,6 +20,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; namespace Neo.Network.P2P.Payloads { @@ -66,6 +67,7 @@ public class Signer : IInteroperable, ISerializable, IEquatable /*AllowedGroups*/ (Scopes.HasFlag(WitnessScope.CustomGroups) ? AllowedGroups.GetVarSize() : 0) + /*Rules*/ (Scopes.HasFlag(WitnessScope.WitnessRules) ? Rules.GetVarSize() : 0); + [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool Equals(Signer other) { if (ReferenceEquals(this, other)) @@ -78,15 +80,11 @@ public bool Equals(Signer other) Rules.SequenceEqual(other.Rules); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public override bool Equals(object obj) { - if (obj == null) - return false; - - if (obj is not Signer signerObj) - return false; - else - return Equals(signerObj); + if (obj == null) return false; + return obj is Signer signerObj && Equals(signerObj); } public override int GetHashCode() @@ -231,6 +229,7 @@ VM.Types.StackItem IInteroperable.ToStackItem(IReferenceCounter referenceCounter ]); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool operator ==(Signer left, Signer right) { if (left is null || right is null) @@ -239,6 +238,7 @@ VM.Types.StackItem IInteroperable.ToStackItem(IReferenceCounter referenceCounter return left.Equals(right); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool operator !=(Signer left, Signer right) { if (left is null || right is null) From 025033c430ddca3cc41ebbf613bd51ce7a99745d Mon Sep 17 00:00:00 2001 From: Christopher Schuchardt Date: Tue, 12 Nov 2024 17:55:44 -0500 Subject: [PATCH 8/8] Update src/Neo/Network/P2P/Payloads/Signer.cs --- src/Neo/Network/P2P/Payloads/Signer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Neo/Network/P2P/Payloads/Signer.cs b/src/Neo/Network/P2P/Payloads/Signer.cs index 75fb8d9e71..f5d9015131 100644 --- a/src/Neo/Network/P2P/Payloads/Signer.cs +++ b/src/Neo/Network/P2P/Payloads/Signer.cs @@ -244,7 +244,7 @@ VM.Types.StackItem IInteroperable.ToStackItem(IReferenceCounter referenceCounter if (left is null || right is null) return !Equals(left, right); - return !(left.Equals(right)); + return !left.Equals(right); } } }