Some small optimization

[Qiao-Jin]

hashes.OrderBy(p => p) should be deleted as this sorting is not used by any code.
Besides func UnionWith's complexity is O(n) where n is the length of its parameter.

[shargon]

Could you provide some benchmarks? linq it's slower.

[vncoelho]

In the past I remember that we needed to order it for verifying multisig.
If it is not used and needed I agree with the improvement.

[erikzhang]

The order cannot be removed.

neo/src/neo/SmartContract/Helper.cs

Lines 126 to 168 in 3de1646

	internal static bool VerifyWitnesses(this IVerifiable verifiable, StoreView snapshot, long gas)
	{
	if (gas < 0) return false;
	UInt160[] hashes;
	try
	{
	hashes = verifiable.GetScriptHashesForVerifying(snapshot);
	}
	catch (InvalidOperationException)
	{
	return false;
	}
	if (hashes.Length != verifiable.Witnesses.Length) return false;
	for (int i = 0; i < hashes.Length; i++)
	{
	int offset;
	byte[] verification = verifiable.Witnesses[i].VerificationScript;
	if (verification.Length == 0)
	{
	ContractState cs = snapshot.Contracts.TryGet(hashes[i]);
	if (cs is null) return false;
	ContractMethodDescriptor md = cs.Manifest.Abi.GetMethod("verify");
	if (md is null) return false;
	verification = cs.Script;
	offset = md.Offset;
	}
	else
	{
	if (hashes[i] != verifiable.Witnesses[i].ScriptHash) return false;
	offset = 0;
	}
	using (ApplicationEngine engine = new ApplicationEngine(TriggerType.Verification, verifiable, snapshot, gas))
	{
	engine.LoadScript(verification, CallFlags.ReadOnly).InstructionPointer = offset;
	engine.LoadScript(verifiable.Witnesses[i].InvocationScript, CallFlags.None);
	if (engine.Execute() == VMState.FAULT) return false;
	if (!engine.ResultStack.TryPop(out StackItem result) || !result.ToBoolean()) return false;
	gas -= engine.GasConsumed;
	}
	}
	return true;
	}

[Qiao-Jin]

But the order of witness is also by func GetScriptHashesForVerifying. So whether we order hashes or not, witnesses and hashes are one-to-one correspondence.

[erikzhang]

You are modifying GetScriptHashesForVerifying(), aren't you?

[Qiao-Jin]

neo/src/neo/SmartContract/ContractParametersContext.cs

Line 235 in 1451f84

Witness[] witnesses = new Witness[ScriptHashes.Count];

and

neo/src/neo/SmartContract/ContractParametersContext.cs

Line 93 in 1451f84

_ScriptHashes = Verifiable.GetScriptHashesForVerifying(null);

Actually the order of witness depends on func GetScriptHashesForVerifying, so witnesses and hashes are one-to-one correspondence whether ordered or not.

[erikzhang]

You are talking ablout this implementation. But NEO is a protocol. You need to find a way to ensure the order between different implementations.

[Qiao-Jin]

Closed due to conflict with implementation design.