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

"Object reference not set to an instance of an object" after neo-express contract invoke when running Runtime.CheckWitness(owner) #12

Closed
johnhoj opened this issue Nov 8, 2019 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@johnhoj
Copy link

johnhoj commented Nov 8, 2019

Using this simple test contract:

using Neo.SmartContract.Framework;
using Neo.SmartContract.Framework.Services.Neo;

public class NEP5 : SmartContract
{
    private static readonly byte[] owner = "AHWz3eLZL2Ymc8PKhQKzq9xDczWJf5K2ac".ToScriptHash();
    public static void Main()
    {
        bool isOwner = Runtime.CheckWitness(owner);
        if (isOwner) 
            Storage.Put(Storage.CurrentContext, "Hello", "World from owner");
        else
            Storage.Put(Storage.CurrentContext, "Hello", "World NOT from owner");

    }
}`

with the standard setup of the neo-blockchain-toolkit works in debug mode (returns always true as defined in the default.neo-express.json) but throws an exception when running invoke from the terminal as follows:

C:...\NEP5>neo-express contract invoke NEP5 --account testWallet
Object reference not set to an instance of an object.
Usage: neo-express contract invoke [options]

Running the same contract after just changing the Runtime.CheckWitness line by:

bool isOwner = true;

works well. So I conclude is the call to CheckWitness that fails.

running main works in Debug NeoCompiler and in the de

NEP5.zip

@devhawk
Copy link
Contributor

devhawk commented Nov 14, 2019

Moving to the neo-express repo

@devhawk devhawk transferred this issue from neo-project/neo-blockchain-toolkit Nov 14, 2019
@devhawk devhawk self-assigned this Nov 14, 2019
@devhawk
Copy link
Contributor

devhawk commented Nov 15, 2019

This appears to be an issue with NeoVM. As you point out, if you hardcode the response isOwner assignment, the problem goes away.

Looking inside the neo-express code, after the invocation transaction is executed, a ContractParametersContext is created as part of the signing process. When calling runtime.checkWitness, the ContractParametersContext is invalid and we get an The given key 'Neo.Network.P2P.Payloads.CoinReference' was not present in the dictionary. in the latest release/v0.9 build of neo-express.

I'm going to transfer this bug to neo-vm repo and tag @lightszero and @lock9 for their attention

@devhawk devhawk assigned lightszero and lock9 and unassigned devhawk Nov 15, 2019
@devhawk
Copy link
Contributor

devhawk commented Nov 15, 2019

apparently I can't transfer to neo-vm repo, but I've tagged the appropriate folks

@shargon shargon transferred this issue from neo-project/neo-express Nov 15, 2019
@devhawk
Copy link
Contributor

devhawk commented Nov 15, 2019

I validated that the script executes on production neo-cli/neo-gui. I modified the script to return isOwner and it returns false which is not correct, but it doesn't fault the way neo-express does during signing. I've asked @shargon to move this back to neo-express repo.

@johnhoj I am not going to have time to troubleshoot this in the next week but I will try and fix whatever the problem is late next week

@devhawk
Copy link
Contributor

devhawk commented Nov 15, 2019

For anyone who wishes to duplicate my verification, this is what I did

  • I exported the consensus node and testWallet info from the provided neo-express.json file so I could use it with neo-cli/gui. I modified the generated protocol.json file to create a block every second.
  • I set up a new instance of neo-cli and neo-gui, using the exported config/protocol json files as appropriate. I installed the applicationlogs plugin in neo-cli
  • I started neo-cli and neo-gui.
  • using gui
    • I opened the consensus node wallet and transferred genesis neo to testWallet account, claimed gas and then transfered the gas to testWallet account
    • I opened the testWallet wallet, waited a bit, transferred all the neo to itself to unlock gas then claimed gas
  • I shut down neo-cli and neo-gui so I could save the blockchain state of neo-cli so I could easily rollback to this point (boy this is so much easier with neo-express checkpoints!)
  • I restarted neo-cli and gui, deployed the contract with gui and then invoked it, copying the txid to the clipboard
  • using Insomnia I hit the getapplicationlog rpc endpoint using the copied txid as a parameter to inspect the stack that was returned. THough I was expecting isOwner to be true, the stack indicated that the return value was false:
"stack": [
    {
    "type": "Boolean",
    "value": false
    }
],

@shargon shargon transferred this issue from neo-project/neo-vm Nov 15, 2019
@devhawk devhawk assigned devhawk and unassigned lightszero and lock9 Nov 20, 2019
@devhawk devhawk added the bug Something isn't working label Nov 21, 2019
@devhawk
Copy link
Contributor

devhawk commented Nov 21, 2019

There was a bug in neo-express - it was attaching coin references to the invocation transaction even though the transaction fee was below the free transaction threshold. commit a8752a3 resolves that issue. The fix is in v 0.9.85 of neo-express. There's another fix that needs to go into neo-express, so I'm not going to push this build to nuget.org, but there are instructions for installing from our azure artifacts feed in the repo readme if you want to test it right away.

However, what I've discovered is that neither neo-express nor neo-cli will sign the invocation transaction for this contract. @shargon suggested this might be the same issue as neo-project/neo-gui-2.x#305. Since there are no signatures, that means that I don't think CheckWitness will behave as expected. @johnhoj please feel free to open a new bug in the neo repo if you have further issues with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants