Inconsistent UInt160 hashes

[lock9]

Hi.

The script hash shown by Neo Express / VS Code extensions seems incorrect.

This is not the correct hash for the contract. The value should be 0x72a951129962cc3dec0220558c7f1064367d395b instead.

[gsmachado]

@devhawk do we have a unified view of how Hashes should be represented in a readable way? I mean, big- versus little-endian-kind-of-thing.

[gsmachado]

@lock9 in my point there's no right or wrong way. This is just representation, and the ecosystem should just agree (standardize) on a way how to display or input these.

Or am I wrong?

[gsmachado]

IMHO, this directly relates to neo-project/neo#938.

FYI, @roman-khimov

[lock9]

@lock9 in my point there's no right or wrong way. This is just representation, and the ecosystem should just agree (standardize) on a way how to display or input these.

Or am I wrong?

It must be consistent across the applications. The problem here is that it even suggests you to copy the script hash - but the value is not usable as is, you need to revert it. Who could possibly imagine such a thing? It's not even 'explainable'

[gsmachado]

It must be consistent across the applications.

Yes, agree.

The problem here is that it even suggests you to copy the script hash

Yes, that's a usability problem.

but the value is not usable as is, you need to revert it

Depends on where you want to paste, and which representation the application you want to paste accepts it. Big-endian? Little-endian?

Who could possibly imagine such a thing?

Seniors could imagine such a thing (here), but definitely not newbies or people learning it. 😓

It's not even 'explainable'

It's explainable, but not usable. Two different things.

[gsmachado]

IMHO, this is not only a problem of neo-express.

It's a problem within our entire community on how to standardize hash inputs/types and things like that. It's just more visible in neo-express because it's a tool that which multiple tools interact.

If I get a hash output somewhere in neow3j and paste it to another tool, I'm almost certain that I'd need to reverse it in some scenarios. 😓

[lock9]

Depends on where you want to paste, and which representation the application you want to paste accepts it. Big-endian? Little-endian?

I want to paste it in my Neo express scripts. That is why it's not consistent.

It says that the hash of the contract is "A", but if I try to use "callContract(A)", it won't work.

[devhawk]

The UInt160 type in the C# Neo implementation explicitly reverses the hex string when converting to a string representation

public override string ToString()
{
    return "0x" + this.ToArray().ToHexString(reverse: true);
}

TryParse reverses this, by parsing character pairs from front to back but filling the array buffer from back to front

for (int i = 0; i < Length; i++)
     if (!byte.TryParse(s.Substring(i * 2, 2), NumberStyles.AllowHexSpecifier, null, out data[Length - i - 1]))

Since Neo Express builds on top of the C# Neo implementation, it uses these ToString and TryParse methods. There may be places across the toolkit where we don't reverse the order when converting between binary and string representations, but I would consider those bugs that need to be fixed in the toolkit.

I'm sympathetic to the view Roman expressed on this topic - we don't typically treat hash outputs as integers so it feels kind of strange to apply big or little endian encoding. However, at this point we can't change the core platform without breaking a lot of code. The toolkit builds directly on the C# implementation, so it will also continue to use the reversed byte string representation for 160 and 256 bit hashes.

Note, Express uses # as a hash signifier prefix when parsing contract parameters (in .neo-invoke files and on the command line for contract run). When decoding a string prefixed with '#', express first tries to parse the value as a 160 or 256 bit hash. If both of those conversions fail, it tries to find a contract with a matching name. So strings like #NeoToken get resolved to the contract hash for the Neo token contract.

[melanke]

I agree that a breaking change is something we should avoid, but I also agree that we need to be as clear and concise as possible, specially integrating our tools.

I suggest to introduce a new symbol as prefix for little-endian. NeoExpress could keep using little-endian as default when no prefix symbol is provided but show a warning message to ask the user to use the correct symbol and warn that it will use little-endian as default.

[devhawk]

I suggest to introduce a new symbol as prefix for little-endian. NeoExpress could keep using little-endian as default when no prefix symbol is provided but show a warning message to ask the user to use the correct symbol and warn that it will use little-endian as default.

This sounds like a reasonable idea, though I'd like to get more feedback from the folks on this thread. In particular, I'm interested in what @lock9 thinks. The suggestion here is to make the tools easier to use, but having a two different prefix characters - # for reversed and something else for non-reversed - seems as hard to explain as the current situation.

ContractParameterParser.ParseStringParameter (used across BCTK) already looks for the following prefixes:

• @: parse the string as an address. The string can be a wallet name from the .neo-express file (i.e. @alice) or an encoded address (i.e. @NZS7VMxxu9H9db1fPGQCWeEj4z7dagFoLm)
• #: parse the string as a hash. The string can be a reverse encoded UInt160 or UInt256 (using TryParse as described above) or it can be the name of contract (such as #NeoToken)
• file://: parse the string as a path to an on disc file
• `0x': parse the string as hex encoded

Note, adding a new prefix for "non reversed hex encoded hash" is a minor breaking change. Anyone already using that character in their invoke files or other invocation scripts runs the risk of their project breaking since a string that was previously treated as a simple string would now be treated as a hash. However, using a punctuation character such as $ or % - or even a multi character prefix such as ## would make the risk pretty low.

Furthermore, if a prefixed string can't be parsed into the expected type, it gets treated as a simple string. For example, #foo is parsed as a string parameter (assuming there is no foo contract). So ##5b397d3664107f8c552002ec3dcc62991251a972 would be treated as a non-reversed hex string but ##hello would still be treated as a simple string parameter.

[lock9]

TL;DR:
We need to be able to use the script hash inside the code without using an external tool or website.

Some context:
We made our smart contract development course using 2 contracts. The first one is the token contract, and the second one is the NFT contract. To mint an NFT, the user must send the tokens to the contract, triggering the onNEP17Transfer method. The user must recompile and deploy the contract several times, one for each step in the tutorial.

In this scenario:

• The user collects the script hash from the first contract and adds it to the transfer callback. To do this, the user would open the explorer tab, find his contract, copy the hash, and put it inside his contract.
• The user will send the funds from his account to the NFT contract hash, so he needs the contract hash again.
• The user can view the storage contents using the hash
• The user must repeat these steps when the script hash changes.

The problem:
If they use the script hash being shown by the extension, a few things will happen:
1 - The funds will be transferred to the incorrect account
2 - The callback won't be triggered (since the hash was incorrect)
3 - The storage is still visible using the incorrect key, but using the same key inside a contract or tool will not work.
4 - Since it's common for this script hash to change, the default response is to recompile the contract, reset neo express, do a lot of 'cleaning', and try again. This takes a lot of time and doesn't work.

Notes:
We had to change and rework several steps to make the development more 'acceptable'. For example: we started with simple methods like 'symbol'. However, we had to change it. The response is encoded in hex and is not human-readable. In general, there are several this we have to make up to avoid explaining complex topics right at the beginning.

We also use the tool in Python, Java, and other programming languages. I'm not sure if all of them have the same SDK features available or have similar names.

Urgent solution:
This problem of copying the script hash is extremely urgent. I propose that both the big-endian and little-endian versions of the script hash be displayed inside the extension.
Everything inside the extension drives you to use one value, while the code expects a different one. We should at least allow them to view the correct one. This won't fix the issue of 'deceiving' the user into thinking that the keys on the debugger and the storage views are correct.

For the future:
There are a ton of things that we need to improve. I've personally reworked the tutorial 5 times already due to inconsistencies in naming, conversions, features etc. Things won't work if the user walks 1 meter outside the trail.
I believe many of them were reported as issues on GitHub. I didn't open issues for everything but a few important ones.

My concerns about prefixes and others:
We need these values to be used inside the contracts and not only inside scripts. I think we need it, but at the same time, it's not the main issue.

The alternative for us is to use the command line, but this has also been a challenge because of inconsistencies in the command line tool.

[gsmachado]

@devhawk @JohndeVadoss any opinions/feedback on the previous comment (above)?

It seems it negatively affects AxLabs/grantshares#11, in specific if you read AxLabs/grantshares#11 (comment) and AxLabs/grantshares#11 (comment).

[gsmachado]

I propose that both the big-endian and little-endian versions of the script hash be displayed inside the extension.

About this ☝️, I believe the easiest is to change the https://github.com/ngdenterprise/neo3-visual-tracker.

Therefore, I created an issue for this: N3developertoolkit/neo3-visual-tracker#162

[devhawk]

I propose that both the big-endian and little-endian versions of the script hash be displayed inside the extension.

I'm sure this would work fine for instructor led tutorials, but how will a developer using these tools for the first time know which one to use?

Everything inside the extension drives you to use one value, while the code expects a different one

So can we simply standardize the toolkit to always display UInt160/256 values as reverse encoded hex?

[lock9]

I think the second option is better. I don't think we ever use script hashes in current format

[roman-khimov]

We know the issue. We know it can't be fixed. But let me share some NeoGo experience around it and throw in some small idea that maybe can help in mitigating this.

We have a CLI that can do contract invocations and many other things. And this CLI has a simple rule --- wherever a script hash can be accepted, an address can also be passed and vice versa. Of course it accepts hashes of one particular kind (reversed ones) and we still can't copy-paste a hash from NeoVM script dump into CLI script, but we can do this for addresses. For contract invocations the way this CLI parses parameters is:

   Arguments always do have regular Neo smart contract parameter types, either
   specified explicitly or being inferred from the value. To specify the type
   manually use "type:value" syntax where the type is one of the following:
   'signature', 'bool', 'int', 'hash160', 'hash256', 'bytes', 'key' or 'string'.
   Array types are also supported: use special space-separated '[' and ']'
   symbols around array values to denote array bounds. Nested arrays are also
   supported. Null parameter is supported via 'nil' keyword without additional
   type specification.

   There is ability to provide an argument of 'bytearray' type via file. Use a
   special 'filebytes' argument type for this with a filepath specified after
   the colon, e.g. 'filebytes:my_file.txt'.

   Given values are type-checked against given types with the following
   restrictions applied:
    * 'signature' type values should be hex-encoded and have a (decoded)
      length of 64 bytes.
    * 'bool' type values are 'true' and 'false'.
    * 'int' values are decimal integers that can be successfully converted
      from the string.
    * 'hash160' values are Neo addresses and hex-encoded 20-bytes long (after
      decoding) strings.
    * 'hash256' type values should be hex-encoded and have a (decoded)
      length of 32 bytes.
    * 'bytes' type values are any hex-encoded things.
    * 'filebytes' type values are filenames with the argument value inside.
    * 'key' type values are hex-encoded marshalled public keys.
    * 'string' type values are any valid UTF-8 strings. In the value's part of
      the string the colon looses it's special meaning as a separator between
      type and value and is taken literally.

Yeah, it has a lot of magic in it, but in the end what we have is that

./bin/neo-go contract testinvokefunction -r https://rpc10.n3.nspcc.ru:10331 0xd2a4cff31913016155e38e474a2c06d08be276cf balanceOf NV96QgerjXNmu4jLdMW4ZWkhySVMYX52Ex

and

./bin/neo-go contract testinvokefunction -r https://rpc10.n3.nspcc.ru:10331 NepwUjd9GhqgNkrfXaxj9mmsFhFzGoFuWM balanceOf 07b2a38ee4a6186522978129324736bfa5913465

are absolutely the same invocations.

So we have some problems with hashes, but we have zero problems with addresses, they can't be misinterpreted, even though they provide the same 20-byte value inside. And maybe we can leverage that if we're to use them more often, including using them for contract identification purposes. This means providing address representation of every hash wherever we have hashes and accepting addresses wherever hashes are normally accepted. Using NiHURyS83nX2mpxtA7xq84cGxVbHojj5Wc instead of 0xef4073a0f2b305a38ec4050e4d3d28bc40ea63f5 to identify NEO, NepwUjd9GhqgNkrfXaxj9mmsFhFzGoFuWM instead of 0xd2a4cff31913016155e38e474a2c06d08be276cf to identify GAS, etc.

This can't be done overnight, obviously. And this still requires some understanding of things going on, but maybe we can at least think about it, because neo-project/neo#938 can't be fixed at this stage.

[lock9]

Will it work inside smart-contracts? Can you paste 'NiHURyS83nX2mpxtA7xq84cGxVbHojj5Wc' inside the code? It can be a good solution if smart contracts can use this value inside it.

I think we need to revert it because it's inconsistent inside Neo Express. If you copy the value from the explorer and paste it inside a invoke file, it won't work. This must be fixed by replacing the existing hash or showing the reverted one.
The current value can't be used on Dora, One Gate, or invoke files, or as parameters.

[roman-khimov]

It works like this for us in smart contracts (see package docs as well):

	owner = address.ToHash160("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB")

This gets compiled into

2        PUSHDATA1    aed8db3b5d664ca04bdd7bed37992ff958560b41
24       CONVERT      Buffer (30)
26       STSFLD0      

Conversion is made just because Hash160 is in fact a []byte. And yes, in reality the hash is 410b5658f92f9937ed7bdd4ba04c665d3bdbd8ae, but we'll probably do an enhancement there.

[lock9]

I think that if everyone agrees, we can move to an address in the future. Before we have that, could we revert the hex inside the extension? Or at least have an option to view it reverted?

[csmuller]

I think that if everyone agrees, we can move to an address in the future. Before we have that, could we revert the hex inside the extension? Or at least have an option to view it reverted?

I agree with @lock9, we should quickly get a solution in the VSCode extension that provides the reverted hex, which can be used without modification (i.e., reverting). If the user can copy the script hash from the UI and paste it back somewhere in the same tool, it should just work. This might seem like a minor thing in the context of the extension, but I think it's a big pain point if you have to tell the user "copy the hash, then go to this converter page, reverse it and paste it back in the same tool, just because".

[gsmachado]

While we were all discussing, I took my fun time and already implemented it:

N3developertoolkit/neo3-visual-tracker#163

I hope this would ALLEVIATE the pain of this issue -- because it doesn't solve it for good. But it certainly helps.

[ixje]

Shouldn't we fix the all tools using the wrong order for input instead? Now the user is still in a guessing game because 99% don't have a clue what order should be provided (depending on the tool I'm part of that 99%)

[lock9]

@ixje I think we should. There are other places where we are showing the incorrect 'hash'.

[PazBazak]

Any updates regarding this ticket... its been a long time and as a newb getting in, the extension was confusing.

[cschuchardt88]

@PazBazak No update. We are working on something else to replace (in future). However there shouldn't be a problem in neoxp.exe (if you are using the command-line).

[lock9]

Hi,

This is not important anymore. We are using other tools. I'm going to close it.