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.

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

[Add] IEquatable to Signer #3571

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
46 changes: 45 additions & 1 deletion src/Neo/Network/P2P/Payloads/Signer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.CompilerServices;

namespace Neo.Network.P2P.Payloads
{
/// <summary>
/// Represents a signer of a <see cref="Transaction"/>.
/// </summary>
public class Signer : IInteroperable, ISerializable
public class Signer : IInteroperable, ISerializable, IEquatable<Signer>
{
// This limits maximum number of AllowedContracts or AllowedGroups here
private const int MaxSubitems = 16;
Expand Down Expand Up @@ -66,6 +67,31 @@ public class Signer : IInteroperable, ISerializable
/*AllowedGroups*/ (Scopes.HasFlag(WitnessScope.CustomGroups) ? AllowedGroups.GetVarSize() : 0) +
/*Rules*/ (Scopes.HasFlag(WitnessScope.WitnessRules) ? Rules.GetVarSize() : 0);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool Equals(Signer other)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a practical need for this API?

Copy link
Contributor

Choose a reason for hiding this comment

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

those of being nulls will also be compared, i have checked, null == null return true. Not sure if this can be of any problem, looks right to me.

Copy link
Member

Choose a reason for hiding this comment

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

Is not bad to me also, == and != not required to me, but it's ok

{
if (ReferenceEquals(this, other))
return true;
if (other is null) return false;
return Account == other.Account &&
Scopes == other.Scopes &&
AllowedContracts.SequenceEqual(other.AllowedContracts) &&
Copy link
Member

Choose a reason for hiding this comment

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

Some of these values are null depending of the Scope

Copy link
Contributor

Choose a reason for hiding this comment

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

scope can not be null, its a enum, can not be null.

Copy link
Member

Choose a reason for hiding this comment

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

scope can not be null,

But AllowedContracts/AllowedGroups/Rules can be null sometimes, that's what Shargon says.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, some of the values can be null, we should make unit tests with real signers with different scopes, depending of the scope, you will find nulls entries

AllowedGroups.SequenceEqual(other.AllowedGroups) &&
Rules.SequenceEqual(other.Rules);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public override bool Equals(object obj)
{
if (obj == null) return false;
return obj is Signer signerObj && Equals(signerObj);
}

public override int GetHashCode()
{
return HashCode.Combine(Account.GetHashCode(), Scopes);
}

public void Deserialize(ref MemoryReader reader)
{
Account = reader.ReadSerializable<UInt160>();
Expand Down Expand Up @@ -202,5 +228,23 @@ 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)
]);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool operator ==(Signer left, Signer right)
{
if (left is null || right is null)
return Equals(left, right);

return left.Equals(right);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool operator !=(Signer left, Signer right)
{
if (left is null || right is null)
return !Equals(left, right);

return !left.Equals(right);
}
}
}
66 changes: 66 additions & 0 deletions tests/Neo.UnitTests/Network/P2P/Payloads/UT_Signers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,72 @@ namespace Neo.UnitTests.Network.P2P.Payloads
[TestClass]
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 = [UInt160.Zero],
AllowedGroups = [ecPoint],
Rules = [
new WitnessRule
{
Condition = new BooleanCondition
{
Expression = true,
},
Action = WitnessRuleAction.Allow,
},
]
};

var actual = new Signer()
{
Account = UInt160.Zero,
Scopes = WitnessScope.Global,
AllowedContracts = [UInt160.Zero],
AllowedGroups = [ecPoint],
Rules = [
new WitnessRule
{
Condition = new BooleanCondition
{
Expression = true,
},
Action = WitnessRuleAction.Allow,
},
]
};

var notEqual = new Signer()
{
Account = UInt160.Zero,
Scopes = WitnessScope.WitnessRules,
AllowedContracts = [],
shargon marked this conversation as resolved.
Show resolved Hide resolved
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));

Assert.IsFalse(expected == null);
Assert.IsFalse(null == expected);
Assert.AreNotEqual(expected, null);
Assert.IsFalse(expected.Equals(null));
}


[TestMethod]
public void Serialize_Deserialize_Global()
{
Expand Down
Loading