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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Mar 17, 2021
Merged

fix verify api #564

Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2d467f1
fix verify api
Ashuaidehao Mar 16, 2021
ca8d87f
Merge branch 'master' into Fix-verifyfee
superboyiii Mar 16, 2021
331e24a
refac
Ashuaidehao Mar 16, 2021
ec221df
Merge branch 'Fix-verifyfee' of https://github.com/Ashuaidehao/neo-mo…
Ashuaidehao Mar 16, 2021
647f56b
default signer & parameter
Ashuaidehao Mar 17, 2021
108b15e
refac
Ashuaidehao Mar 17, 2021
bf5034c
add Witness
Ashuaidehao Mar 17, 2021
b79c6de
format
Ashuaidehao Mar 17, 2021
9f89979
update
Ashuaidehao Mar 17, 2021
1d4b121
update
Ashuaidehao Mar 17, 2021
1043c8f
update
Ashuaidehao Mar 17, 2021
2069218
Merge branch 'master' into Fix-verifyfee
superboyiii Mar 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions src/RpcServer/RpcServer.Wallet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -348,20 +347,20 @@ private JObject GetVerificationResult(UInt160 scriptHash, ContractParameter[] ar
{
throw new RpcException(-100, "Unknown contract");
}
var methodName = "verify";
var md = contract.Manifest.Abi.GetMethod("verify", 0);
Copy link
Member

Choose a reason for hiding this comment

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

Can we set pcount to -1 here in order to be able to call verify with arguments? Just like in https://github.com/neo-project/neo/blob/master/src/neo/SmartContract/Helper.cs#L222.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated it.

if (md is null)
throw new RpcException(-101, $"The smart contract {contract.Hash} haven't got verify method without arguments");
Copy link
Member

Choose a reason for hiding this comment

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

Exception message should also be updated:

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
Copy link
Member

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 when signers == null? We can do it because we know exactly at least one signer (it's the contract to be called). And we also know the witness for this signer: invocation script pushes args on the stack; verification script is empty because it's a contract witness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

{
Signers = signers.GetSigners(),
Attributes = Array.Empty<TransactionAttribute>()
Attributes = Array.Empty<TransactionAttribute>(),
Copy link
Member

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 use System.Runtime.GetScriptContainer interop, because transaction can't be serialized without a script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

Witnesses = signers.Witnesses,
Copy link
Member

Choose a reason for hiding this comment

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

Witnesses it wil lbe null, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, just like

neo-modules/src/RpcServer/RpcServer.SmartContract.cs

Line 69 in 93fb620

Witnesses = signers.Witnesses,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it since no use here.

Copy link
Member

Choose a reason for hiding this comment

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

Can we pass witnesses via RPC request parameters, just like signers are passed? And then fill Witnesses here.

};
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);
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated it. Please test it like:

{
    "jsonrpc": "2.0",
    "method": "invokecontractverify",
    "params": [
        "0x31f714e04766139c8f3705f5ce94518e1411ec07",
        [
            {
                "type": "ByteArray",
                "value": "DED2HMp00JZ2zcJYjKgTp4Tl0GODm5r14PKN6ip8i2+8ox3JYqN2xNHxQzCgzENqSD52GhVF4eGx2i405zxM1KTH"
            }
        ],
        [
            {
                "account": "NMA2FKN8up2cEwaJgtmAiDrZWB69ApnDfp",
                "scopes": "CalledByEntry"
            }
        ]
    ],
    "id": 1
}

Copy link
Member

Choose a reason for hiding this comment

The 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 InvokeFunction RPC method. Like the following:

{
    "jsonrpc": "2.0",
    "method": "invokecontractverify",
    "params": [
        "0x31f714e04766139c8f3705f5ce94518e1411ec07",
        [
            {
                "type": "Boolean",
                "value": "true"
            },
            {
                "type": "Integer",
                "value": 123
            }
        ],
        [
            {
                "account": "NMA2FKN8up2cEwaJgtmAiDrZWB69ApnDfp",
                "scopes": "CalledByEntry"
            }
        ]
    ],
    "id": 1
}

And then we can construct an invocation script on the server by performing the following:

using (ScriptBuilder sb = new ScriptBuilder())
{
        for (int i = args.Count - 1; i >= 0; i--)
                sb.EmitPush(args[i]);
}

Copy link
Member

Choose a reason for hiding this comment

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

Agree with @AnnaShaleva

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

JObject json = new JObject();
json["script"] = Convert.ToBase64String(contract.Script);
Copy link
Member

Choose a reason for hiding this comment

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

And let the resulting script be the InvocationScript (which is sb.ToArray()). Because 1) the user is aware of the contract being invoked and 2) the user should have the ability to check that all the parameters were converted as expected.

Copy link
Member

Choose a reason for hiding this comment

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

See nspcc-dev/neo-go#1825 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What script mean if sb is empty?

Copy link
Member

Choose a reason for hiding this comment

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

That no parameters were pushed on the stack before verify execution.

json["state"] = engine.Execute();
Expand Down