Allow smart contract verification by shargon · Pull Request #1800 · neo-project/neo

shargon · 2020-07-28T08:21:26Z

Required for neo-project/neo-node#628 (comment)

Allow CheckWitness during Verification.
Allow SmartContract Verification.
Allow static varibles during Verification

src/neo/SmartContract/ContractParametersContext.cs

meevee98 · 2020-07-28T21:43:37Z

Using the same test used in neo-project/neo-node#628 (comment):

I was able to sign and relay the transaction, but the transfer itself doesn't execute.

This only happened when transferring from smart contracts. I'm not sure what's causing the problem, but Transaction.Verify is returning Insufficient Funds even when the smart contract has the requested funds to transfer.

shargon · 2020-07-29T09:14:27Z

@meevee98 please take a look again.

I think that it's fixed

ProDog · 2020-07-29T09:40:50Z

@shargon ，I have tested and it's error, NLtxB5cz8kC3ir1JD4nrY1bq7YawCr7Gba is contract address added by import watchonly, then I transfer GAS from this address.

shargon · 2020-07-29T09:48:34Z

Could you show me your script and manifest?

I tested it with this code, PUSH1 and verify in offset 0

[ConsoleCommand("deploy-test")]
        private void DeployTest()
        {
            var nef = new NefFile()
            {
                Compiler = "",
                Script = new byte[] { (byte)OpCode.PUSH1 },
                Version = new Version(3, 1, 1, 1),
            };
            nef.ScriptHash = nef.Script.ToScriptHash();
            nef.CheckSum = NefFile.ComputeChecksum(nef);

            ContractManifest cf = new ContractManifest()
            {
                Abi = new ContractAbi()
                {
                    Events = new ContractEventDescriptor[0],
                    Hash = nef.ScriptHash,
                    Methods = new ContractMethodDescriptor[]
                     {
                         new ContractMethodDescriptor()
                         {
                              Name="verify",
                               Offset=0,
                                Parameters=new ContractParameterDefinition[0],
                                 ReturnType= ContractParameterType.Boolean
                         }
                     }
                },
                Extra = null,
                Features = ContractFeatures.Payable,
                SafeMethods = WildcardContainer<string>.Create(),
                Groups = new ContractGroup[0],
                Permissions = new ContractPermission[0],
                SupportedStandards = new string[0],
                Trusts = WildcardContainer<UInt160>.Create()
            };

            File.WriteAllBytes("C:\\temporal\\verify.nef", nef.ToArray());
            File.WriteAllBytes("C:\\temporal\\verify.json", Encoding.UTF8.GetBytes(cf.ToJson().ToString()));

            OnDeployCommand("C:\\temporal\\verify.nef", "C:\\temporal\\verify.json");
        }

ProDog · 2020-07-29T09:58:09Z

A simplified contract.

script(hex string):

0dce017b2267726f757073223a5b5d2c226665617475726573223a7b2273746f72616765223a747275652c2270617961626c65223a747275657d2c22737570706f727465647374616e6461726473223a5b5d2c22616269223a7b2268617368223a22307837316564626134656536653062306363613564346239626334333433623335613837353839666531222c226d6574686f6473223a5b7b226e616d65223a22766572696679222c22706172616d6574657273223a5b5d2c226f6666736574223a302c2272657475726e74797065223a22426f6f6c65616e227d2c7b226e616d65223a227465737431222c22706172616d6574657273223a5b5d2c226f6666736574223a31332c2272657475726e74797065223a22427974654172726179227d2c7b226e616d65223a225f696e697469616c697a65222c22706172616d6574657273223a5b5d2c226f6666736574223a32312c2272657475726e74797065223a22566f6964227d5d2c226576656e7473223a5b5d7d2c227065726d697373696f6e73223a5b7b22636f6e7472616374223a222a222c226d6574686f6473223a222a227d5d2c22747275737473223a5b5d2c22736166656d6574686f6473223a5b5d2c226578747261223a6e756c6c7d0c5d5701005f0041f827ec8c7068405701005f0170684056030c141c0357464b777ecf6b5f3ac3893ace1f8b1621f6600c14897720d8cd76f4f00abfa37c0edd889c208fde9b610c141c0357464b777ecf6b5f3ac3893ace1f8b1621f6624041ce352c85

manifest:

{"groups":[],"features":{"storage":true,"payable":true},"abi":{
    "hash":"0x71edba4ee6e0b0cca5d4b9bc4343b35a87589fe1",
    "methods":
    [
        {
            "name":"verify",
            "offset":"0",
            "parameters":
            [
            ],
            "returntype":"Boolean"
        },
        {
            "name":"test1",
            "offset":"13",
            "parameters":
            [
            ],
            "returntype":"ByteArray"
        },
        {
            "name":"_initialize",
            "offset":"21",
            "parameters":
            [
            ],
            "returntype":"Void"
        }
    ],
    "events":
    [
    ]
},"permissions":[{"contract":"*","methods":"*"}],"trusts":[],"safemethods":[],"supportedstandards":[],"extra":null}

shargon · 2020-07-29T10:52:10Z

I think that the problem could be that you use static variables and _initialize it's not called during verify. do you have the source?

shargon · 2020-07-29T11:04:35Z

Also _initialize offset points to STLOC5 instead of INITSLOT :S

shargon · 2020-07-29T11:07:12Z

@ProDog plase take a look again, now you should be able to use a static variable during verify, but check this error please #1800 (comment)

ProDog · 2020-07-29T13:13:00Z

I think that the problem could be that you use static variables and _initialize it's not called during verify. do you have the source?

Yes, I will check it.

My contract:

 [Features(ContractFeatures.HasStorage | ContractFeatures.Payable)]
    public partial class APITest : SmartContract
    {        
        private static byte[] Owner = "NNU67Fvdy3LEQTM374EJ9iMbCRxVExgM8Y".ToScriptHash();
        public static bool Verify()
        {
            return Runtime.CheckWitness(Owner);
        }
     }

ProDog · 2020-07-29T13:26:16Z

A new error:

System.InvalidOperationException: 'Cannot call this SYSCALL with the flag None.'

…erify

shargon · 2020-07-29T16:44:58Z

The problem was that verify can't call CheckWitness because require AllowStates, I changed to ReadOnly it should work, but @erikzhang we should reverify the transaction before persist? the states can be different.

ProDog · 2020-07-30T06:38:32Z

New error:

shargon · 2020-07-30T07:40:23Z

@ProDog it was signed by NNU67Fvdy3LEQTM374EJ9iMbCRxVExgM8Y? we should be able to sign with multiple accounts, but we only have one from ... maybe we can change this to be an array?

ProDog · 2020-07-30T07:42:51Z

@ProDog it was signed by NNU67Fvdy3LEQTM374EJ9iMbCRxVExgM8Y? we should be able to sign with multiple accounts, but we only have one from ... maybe we can change this to be an array?

Yes, so signers doesn't contains it.

shargon · 2020-07-30T07:53:32Z

Could you try with neo-project/neo-node@c186c36 or with invoke that allow multiple signers?

src/neo/Wallets/Wallet.cs

shargon · 2020-08-03T11:12:04Z

@ProDog could you do the last test?

ProDog · 2020-08-04T03:13:56Z

src/neo/Wallets/Wallet.cs

            {
                if (!Contains(from))
-                    throw new ArgumentException($"The address {from.ToString()} was not found in the wallet");
+                    throw new ArgumentException($"The address {from} was not found in the wallet");


So as neo-project/neo-node#628 (comment), we don't need this judgment.

ProDog · 2020-08-04T03:30:29Z

Another usage scenario, we use invoke to transfer asset from a verify contract, and contract is sender, like:

invoke nep5Hash transfer [{"type":"Hash160","value":contractHash},{"type":"Hash160","value":toAddress},{"type":"Integer","value":"10"}] contractHash verifyficationAccount

So it shouldn't be WitnessScope.FeeOnly:

neo/src/neo/Wallets/Wallet.cs

Line 212 in 47e8b52

Scopes = WitnessScope.FeeOnly

Now the CLI can't specified the sender to invoke command, I think we can add.

ProDog

Tested OK.

* Allow sc verify * Create DeployedContract * Fix fee * Fix * Change to exceptions * Allow static variables during verification * Simplify DeployedContract * Change verify call flags * Change to ReadOnly * Add error description * Allow cosigners in makeTransaction * Verify as APPCALL * Fix fee * Revert "Fix fee" This reverts commit e8d59a9. * Revert "Verify as APPCALL" This reverts commit d8bf588. * Auto stash before revert of "Allow cosigners in makeTransaction" * None * Add comment * Fix CustomGroups * Fix UT * Update DeployedContract.cs * Optimize MakeTransaction() * Update Wallet.cs * Fix * Fix fee calculation * Optimize CalculateNetworkFee() * Try sc verification when the account it's null * Update Wallet.cs * Update Wallet.cs * Add true verify test * Update Wallet.cs * Remove check account in wallet * Fix UT Co-authored-by: erikzhang <erik@neo.org>

Allow sc verify

0ce0b58

shargon mentioned this pull request Jul 28, 2020

Allow smart contract verification neo-project/neo-node#628

Merged

Tommo-L reviewed Jul 28, 2020

View reviewed changes

src/neo/SmartContract/ContractParametersContext.cs Outdated Show resolved Hide resolved

shargon added 3 commits July 29, 2020 09:18

Create DeployedContract

83e2850

Merge branch 'master' into allow-sc-verify

19a8c94

Fix fee

4668955

shargon added 2 commits July 29, 2020 11:15

Fix

21536a4

Change to exceptions

26905d4

Allow static variables during verification

4ce99fb

Simplify DeployedContract

37f6670

shargon added 3 commits July 29, 2020 18:32

Change verify call flags

898f1c6

Change to ReadOnly

d541f80

Merge remote-tracking branch 'origin/allow-sc-verify' into allow-sc-v…

b1c9772

…erify

Add error description

467c47b

Allow cosigners in makeTransaction

51b8352

shargon and others added 2 commits August 1, 2020 12:44

Try sc verification when the account it's null

ab971ce

Update Wallet.cs

c98d61d

erikzhang reviewed Aug 2, 2020

View reviewed changes

src/neo/Wallets/Wallet.cs Outdated Show resolved Hide resolved

Update Wallet.cs

ce30cd7

shargon commented Aug 2, 2020

View reviewed changes

src/neo/Wallets/Wallet.cs Show resolved Hide resolved

Add true verify test

df0d29c

shargon mentioned this pull request Aug 2, 2020

Add true UT for VerifyWitnesses #1779

Closed

Update Wallet.cs

47e8b52

erikzhang previously approved these changes Aug 3, 2020

View reviewed changes

shargon requested a review from ProDog August 3, 2020 11:12

ProDog reviewed Aug 4, 2020

View reviewed changes

shargon added 2 commits August 4, 2020 09:46

Merge branch 'master' into allow-sc-verify

d972416

Remove check account in wallet

b495fc4

shargon dismissed erikzhang’s stale review via b495fc4 August 4, 2020 07:50

shargon requested review from ProDog and erikzhang August 4, 2020 07:51

Fix UT

c1c2fa4

erikzhang approved these changes Aug 4, 2020

View reviewed changes

ProDog approved these changes Aug 4, 2020

View reviewed changes

shargon merged commit 64189a9 into neo-project:master Aug 4, 2020

shargon deleted the allow-sc-verify branch August 4, 2020 10:29

roman-khimov mentioned this pull request Aug 14, 2020

Add tests for verifyTx and verifyHeader nspcc-dev/neo-go#1318

Merged

roman-khimov mentioned this pull request Aug 23, 2020

Implement and test smart contract verification nspcc-dev/neo-go#1349

Closed

This was referenced Sep 10, 2020

Allow calling script hash to pass CheckWitness #1924

Closed

Remove exception throwing from System.Runtime.GetInvocationCounter #1930

Closed

ixje mentioned this pull request Sep 29, 2020

REQ: RPC call with Verification trigger type #1977

Closed

ixje added a commit to CityOfZion/neo-mamba that referenced this pull request Jan 15, 2021

https://github.com/neo-project/neo/pull/1800

8da2d45

Conversation

shargon commented Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

meevee98 commented Jul 28, 2020

Uh oh!

shargon commented Jul 29, 2020

Uh oh!

ProDog commented Jul 29, 2020

Uh oh!

shargon commented Jul 29, 2020

Uh oh!

ProDog commented Jul 29, 2020

Uh oh!

shargon commented Jul 29, 2020

Uh oh!

shargon commented Jul 29, 2020

Uh oh!

shargon commented Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ProDog commented Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ProDog commented Jul 29, 2020

Uh oh!

shargon commented Jul 29, 2020

Uh oh!

ProDog commented Jul 30, 2020

Uh oh!

shargon commented Jul 30, 2020

Uh oh!

ProDog commented Jul 30, 2020

Uh oh!

shargon commented Jul 30, 2020

Uh oh!

Uh oh!

Uh oh!

shargon commented Aug 3, 2020

Uh oh!

ProDog Aug 4, 2020

Choose a reason for hiding this comment

Uh oh!

shargon Aug 4, 2020

Choose a reason for hiding this comment

Uh oh!

ProDog commented Aug 4, 2020

Uh oh!

ProDog left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shargon commented Jul 28, 2020 •

edited

Loading

shargon commented Jul 29, 2020 •

edited

Loading

ProDog commented Jul 29, 2020 •

edited

Loading