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

'CheckWitness' executes failed during Nep5 Token transfer in Neo3 #1370

Closed
doubiliu opened this issue Dec 17, 2019 · 1 comment
Closed

'CheckWitness' executes failed during Nep5 Token transfer in Neo3 #1370

doubiliu opened this issue Dec 17, 2019 · 1 comment

Comments

@doubiliu
Copy link
Contributor

doubiliu commented Dec 17, 2019

Describe the bug
NGD try to complete the NEP5 Template contract in NEO3.In this process,we found the transfer method will execute failed when we try to transfer a Nep5 asset by invoking RPC method sendtoaddress.By testing, we found that the problem is in 'CheckWitness' method.

CheckWitness is a InteropService method.It provides protection for asset transfers

internal static bool CheckWitness(ApplicationEngine engine, UInt160 hash)
        {
            if (engine.ScriptContainer is Transaction tx)
            {
                Cosigner usage = tx.Cosigners.FirstOrDefault(p => p.Account.Equals(hash));
                if (usage is null) return false;
                if (usage.Scopes == WitnessScope.Global) return true;
                if (usage.Scopes.HasFlag(WitnessScope.CalledByEntry))
                {
                    if (engine.CallingScriptHash == engine.EntryScriptHash)
                        return true;
                }
                if (usage.Scopes.HasFlag(WitnessScope.CustomContracts))
                {
                    if (usage.AllowedContracts.Contains(engine.CurrentScriptHash))
                        return true;
                }
                if (usage.Scopes.HasFlag(WitnessScope.CustomGroups))
                {
                    var contract = engine.Snapshot.Contracts[engine.CallingScriptHash];
                    // check if current group is the required one
                    if (contract.Manifest.Groups.Select(p => p.PubKey).Intersect(usage.AllowedGroups).Any())
                        return true;
                }
                return false;
            }
            // only for non-Transaction types (Block, etc)
            var hashes_for_verifying = engine.ScriptContainer.GetScriptHashesForVerifying(engine.Snapshot);
            return hashes_for_verifying.Contains(hash);
        }

But when we try to transfer a Nep5 asset,we found it will execute failed.The bug logic happens in this part:

                if (usage.Scopes.HasFlag(WitnessScope.CalledByEntry))
                {
                    if (engine.CallingScriptHash == engine.EntryScriptHash)
                        return true;
                }

We found that if the asset is a global asset(NEO,GAS),CallingScriptHash is equal to EntryScriptHash.
But if the asset is a NEP5 asset,CallingScriptHash is not equal to EntryScriptHash.

The reason of this phenomenon is :
When we transfer a NEP5 asset,there exists at least 3 invoke contexts in InvocationStack .So the value of EntryScriptHash is script hash of Tx script,but the value of EntryScriptHash is script hash of contract.And if the scope of tx is CalledByEntry,it will return false.

Solution

  1. A simple way is change the code in MakeTransaction in Wallet.
    We can change the scope of tx to Global. Maybe it will have some security problem.
  2. Another way is that we can create a new scope type for Nep5 asset.
@erikzhang
Copy link
Member

It has been fixed by #1357.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants