-
Notifications
You must be signed in to change notification settings - Fork 100
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
fix verify api #564
fix verify api #564
Changes from all commits
2d467f1
ca8d87f
331e24a
ec221df
647f56b
108b15e
bf5034c
b79c6de
9f89979
1d4b121
1043c8f
2069218
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -333,7 +333,6 @@ protected virtual JObject SendToAddress(JArray _params) | |||||
[RpcMethod] | ||||||
protected virtual JObject InvokeContractVerify(JArray _params) | ||||||
{ | ||||||
CheckWallet(); | ||||||
UInt160 script_hash = UInt160.Parse(_params[0].AsString()); | ||||||
ContractParameter[] args = _params.Count >= 2 ? ((JArray)_params[1]).Select(p => ContractParameter.FromJson(p)).ToArray() : new ContractParameter[0]; | ||||||
Signers signers = _params.Count >= 3 ? SignersFromJson((JArray)_params[2], system.Settings) : null; | ||||||
|
@@ -348,22 +347,36 @@ private JObject GetVerificationResult(UInt160 scriptHash, ContractParameter[] ar | |||||
{ | ||||||
throw new RpcException(-100, "Unknown contract"); | ||||||
} | ||||||
var methodName = "verify"; | ||||||
var md = contract.Manifest.Abi.GetMethod("verify", -1); | ||||||
if (md is null) | ||||||
throw new RpcException(-101, $"The smart contract {contract.Hash} haven't got verify method."); | ||||||
if (md.ReturnType != ContractParameterType.Boolean) | ||||||
throw new RpcException(-102, "The verify method doesn't return boolean value."); | ||||||
|
||||||
Transaction tx = signers == null ? null : new Transaction | ||||||
Transaction tx = new Transaction | ||||||
{ | ||||||
Signers = signers.GetSigners(), | ||||||
Attributes = Array.Empty<TransactionAttribute>() | ||||||
Signers = signers == null ? new Signer[] { new() { Account = scriptHash } } : signers.GetSigners(), | ||||||
Attributes = Array.Empty<TransactionAttribute>(), | ||||||
Witnesses = signers?.Witnesses, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
where
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We know exactly the invocation script, why not to fill it in? By doing this we're getting closer to the real transaction witnesses. |
||||||
Script = new[] { (byte)OpCode.RET } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And the last issue: at the current moment, the transaction is completely missing witnesses. I would suggest to pass witnesses along with signers via RPC request parameters using the following structure:
And then parse the provided witnesses in And then fill the transaction like the following:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will help us to get rid of the private key dependency (because if we parse witnesses, then we don't need private keys in the RPC node wallet anymore). And that's the answer to the #552 (comment) question. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok |
||||||
}; | ||||||
ContractParametersContext context = new ContractParametersContext(snapshot, tx); | ||||||
wallet.Sign(context); | ||||||
tx.Witnesses = context.Completed ? context.GetWitnesses() : null; | ||||||
|
||||||
using ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, tx, snapshot.CreateSnapshot(), settings: system.Settings); | ||||||
engine.LoadScript(new ScriptBuilder().EmitDynamicCall(scriptHash, methodName, args).ToArray(), rvcount: 1); | ||||||
engine.LoadContract(contract, md, CallFlags.ReadOnly); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And create an invocation script from the given args (it will push all arguments on the stack). And load the script right after the contract script, just like in https://github.com/neo-project/neo/blob/master/src/neo/SmartContract/Helper.cs#L246. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated it. Please test it like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not very convenient to construct an invocation script on the client. I would prefer to pass simple standard parameters to the server as we do in the
And then we can construct an invocation script on the server by performing the following:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @AnnaShaleva There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||||||
|
||||||
var invocationScript = new byte[] { }; | ||||||
if (args.Length > 0) | ||||||
{ | ||||||
using ScriptBuilder sb = new ScriptBuilder(); | ||||||
for (int i = args.Length - 1; i >= 0; i--) | ||||||
sb.EmitPush(args[i]); | ||||||
|
||||||
invocationScript = sb.ToArray(); | ||||||
tx.Witnesses ??= new Witness[] { new() { InvocationScript = invocationScript } }; | ||||||
engine.LoadScript(new Script(invocationScript), configureState: p => p.CallFlags = CallFlags.None); | ||||||
} | ||||||
JObject json = new JObject(); | ||||||
json["script"] = Convert.ToBase64String(contract.Script); | ||||||
|
||||||
json["script"] = Convert.ToBase64String(invocationScript); | ||||||
json["state"] = engine.Execute(); | ||||||
json["gasconsumed"] = engine.GasConsumed.ToString(); | ||||||
json["exception"] = GetExceptionMessage(engine.FaultException); | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also fill the transaction script? I think that []byte{OpCode.RET} is enough.
This is needed for those
verify
methods that useSystem.Runtime.GetScriptContainer
interop, because transaction can't be serialized without a script.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.