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

[Neo Rpc Methods] fix contract rpc methods parameters #3485

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
106 changes: 106 additions & 0 deletions src/Plugins/RpcServer/Model/SignerWithWitness.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Copyright (C) 2015-2024 The Neo Project.
//
// SignerWithWitness.cs file belongs to the neo project and is free
// software distributed under the MIT software license, see the
// accompanying file LICENSE in the main directory of the
// repository or http://www.opensource.org/licenses/mit-license.php
// for more details.
//
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using Neo.Cryptography.ECC;
using Neo.Json;
using Neo.Network.P2P.Payloads;
using Neo.Wallets;
using System;
using System.Diagnostics.CodeAnalysis;
using System.Linq;

namespace Neo.Plugins.RpcServer.Model;

public class SignerWithWitness(Signer? signer, Witness? witness)

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 22 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
AnnaShaleva marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

SignerWithWitness ?
May be WitnessSigner is better?

{
public Signer? Signer { get; } = signer;

Check warning on line 24 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 24 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 24 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 24 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 24 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
public Witness? Witness { get; } = witness;

Check warning on line 25 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 25 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 25 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 25 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

public static bool TryParse(JToken value, ProtocolSettings settings, [NotNullWhen(true)] out SignerWithWitness? signerWithWitness)

Check warning on line 27 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 27 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 27 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 27 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
{
signerWithWitness = null;

if (value == null)
return false;

if (value is JObject jObject)
{
Signer? signer = null;

Check warning on line 36 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 36 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 36 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 36 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
Witness? witness = null;

Check warning on line 37 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 37 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 37 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 37 in src/Plugins/RpcServer/Model/SignerWithWitness.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

if (jObject.ContainsProperty("account"))
{
signer = SignerFromJson(jObject, settings);
}
if (jObject.ContainsProperty("invocation") || jObject.ContainsProperty("verification"))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (jObject.ContainsProperty("invocation") || jObject.ContainsProperty("verification"))
else if (jObject.ContainsProperty("invocation") || jObject.ContainsProperty("verification"))

Copy link
Member

Choose a reason for hiding this comment

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

The suggestion is not correct because both account and witness are allowed at the same time.

Copy link
Contributor

Choose a reason for hiding this comment

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

"invocation" and "verification" defined as constant values is better?

{
witness = WitnessFromJson(jObject);
}

if (signer != null || witness != null)
{
signerWithWitness = new SignerWithWitness(signer, witness);
return true;
}
}

return false;
}

private static Signer SignerFromJson(JObject jObject, ProtocolSettings settings)
{
return new Signer
{
Account = AddressToScriptHash(jObject["account"].AsString(), settings.AddressVersion),
Scopes = jObject.ContainsProperty("scopes")
? (WitnessScope)Enum.Parse(typeof(WitnessScope), jObject["scopes"].AsString())
Jim8y marked this conversation as resolved.
Show resolved Hide resolved
: WitnessScope.CalledByEntry,
Jim8y marked this conversation as resolved.
Show resolved Hide resolved
AllowedContracts = ((JArray)jObject["allowedcontracts"])?.Select(p => UInt160.Parse(p.AsString())).ToArray() ?? Array.Empty<UInt160>(),
AllowedGroups = ((JArray)jObject["allowedgroups"])?.Select(p => ECPoint.Parse(p.AsString(), ECCurve.Secp256r1)).ToArray() ?? Array.Empty<ECPoint>(),
Rules = ((JArray)jObject["rules"])?.Select(r => WitnessRule.FromJson((JObject)r)).ToArray() ?? Array.Empty<WitnessRule>()
};
Copy link
Member

Choose a reason for hiding this comment

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

We also need a check against the maximum number of parsed contracts, rules and groups, it's 16.

Copy link
Member

Choose a reason for hiding this comment

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

You may serialize and deserialize parsed items to ensure validity, exactly how the old code does it.

Copy link
Member

Choose a reason for hiding this comment

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

Still not resolved, we need validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is already copied from the original code, if its still not working, i would say it has never worked, and should not be a concern of this pr.

}

private static Witness WitnessFromJson(JObject jObject)
{
return new Witness
{
InvocationScript = Convert.FromBase64String(jObject["invocation"]?.AsString() ?? string.Empty),
VerificationScript = Convert.FromBase64String(jObject["verification"]?.AsString() ?? string.Empty)
};
}

public static SignerWithWitness[] ParseArray(JArray array, ProtocolSettings settings)
{
if (array == null)
throw new ArgumentNullException(nameof(array));

if (array.Count > Transaction.MaxTransactionAttributes)
throw new RpcException(RpcError.InvalidParams.WithData("Max allowed signers or witnesses exceeded."));

return array.Select(item =>
{
if (TryParse(item, settings, out var signerWithWitness))
return signerWithWitness;
throw new ArgumentException($"Invalid signer or witness format: {item}");
}).ToArray();
}

private static UInt160 AddressToScriptHash(string address, byte version)
Jim8y marked this conversation as resolved.
Show resolved Hide resolved
{
if (UInt160.TryParse(address, out var scriptHash))
{
return scriptHash;
}

return address.ToScriptHash(version);
}
}
99 changes: 98 additions & 1 deletion src/Plugins/RpcServer/ParameterConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@
// modifications are permitted.

using Neo.Json;
using Neo.Network.P2P.Payloads;
using Neo.Plugins.RpcServer.Model;
using Neo.SmartContract;
using Neo.Wallets;
using System;
using System.Collections.Generic;
using System.Linq;
using JToken = Neo.Json.JToken;

namespace Neo.Plugins.RpcServer;
Expand All @@ -39,7 +42,12 @@ static ParameterConverter()
{ typeof(bool), token => Result.Ok_Or(token.AsBoolean, CreateInvalidParamError<bool>(token)) },
{ typeof(UInt256), ConvertUInt256 },
{ typeof(ContractNameOrHashOrId), ConvertContractNameOrHashOrId },
{ typeof(BlockHashOrIndex), ConvertBlockHashOrIndex }
{ typeof(BlockHashOrIndex), ConvertBlockHashOrIndex },
{ typeof(Signer), ConvertSigner },
{ typeof(ContractParameter), ConvertContractParameter },
{ typeof(Signer[]), ConvertSignerArray },
{ typeof(ContractParameter[]), ConvertContractParameterArray },
{ typeof(Guid), ConvertGuid }
Jim8y marked this conversation as resolved.
Show resolved Hide resolved
};
}

Expand Down Expand Up @@ -107,6 +115,22 @@ internal static object ConvertUInt160(JToken token, byte addressVersion)
RpcError.InvalidParams.WithData($"Invalid UInt160 Format: {token}"));
}

internal static object ConvertSignerOrWitnessArray(JToken token, ProtocolSettings settings)
Jim8y marked this conversation as resolved.
Show resolved Hide resolved
{
if (token is JArray jArray)
{
return SignerWithWitness.ParseArray(jArray, settings);
}
else if (token is JObject jObject)
{
if (SignerWithWitness.TryParse(jObject, settings, out var signerOrWitness))
{
return new[] { signerOrWitness };
}
}
throw new RpcException(RpcError.InvalidParams.WithData($"Invalid SignerOrWitness format: {token}"));
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

}

private static object ConvertUInt256(JToken token)
{
if (UInt256.TryParse(token.AsString(), out var hash))
Expand Down Expand Up @@ -134,6 +158,79 @@ private static object ConvertBlockHashOrIndex(JToken token)
throw new RpcException(RpcError.InvalidParams.WithData($"Invalid block hash or index Format: {token}"));
}

private static object ConvertSigner(JToken token)
{
if (token is JObject jObject)
{
try
{
return Signer.FromJson(jObject);
}
catch (FormatException)
{
throw new RpcException(CreateInvalidParamError<Signer>(token));
}
}
throw new RpcException(CreateInvalidParamError<Signer>(token));
Copy link
Contributor

Choose a reason for hiding this comment

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

Would adding an else statement here make it clearer?

}

private static object ConvertContractParameter(JToken token)
{
if (token is JObject jObject)
{
try
{
return ContractParameter.FromJson(jObject);
}
catch (FormatException)
{
throw new RpcException(CreateInvalidParamError<ContractParameter>(token));
}
}
throw new RpcException(CreateInvalidParamError<ContractParameter>(token));
}

private static object ConvertSignerArray(JToken token)
{
if (token is JArray jArray)
{
try
{
return jArray.Select(t => Signer.FromJson(t as JObject)).ToArray();
}
catch (FormatException)
{
throw new RpcException(CreateInvalidParamError<Signer[]>(token));
}
}
throw new RpcException(CreateInvalidParamError<Signer[]>(token));
}
Jim8y marked this conversation as resolved.
Show resolved Hide resolved

private static object ConvertContractParameterArray(JToken token)
{
if (token is JArray jArray)
{
try
{
return jArray.Select(t => ContractParameter.FromJson(t as JObject)).ToArray();
}
catch (FormatException)
{
throw new RpcException(CreateInvalidParamError<ContractParameter[]>(token));
}
}
throw new RpcException(CreateInvalidParamError<ContractParameter[]>(token));
}

private static object ConvertGuid(JToken token)
{
if (Guid.TryParse(token.AsString(), out var guid))
{
return guid;
}
throw new RpcException(CreateInvalidParamError<Guid>(token));
}

private static RpcError CreateInvalidParamError<T>(JToken token)
{
return RpcError.InvalidParams.WithData($"Invalid {typeof(T)} value: {token}");
Expand Down
Loading
Loading