Fix RPC responses for Hash160 types
Describe the bug Nodes return ByteStrings when a method return/event type is Hash160 (probably Hash256 too). Due to this bug, developers must reverse the array before using it as a scripthash / address. Methods like 'ownerOf' will return the incorrect owner script hash.
To Reproduce Please try this script from Edge:
import { rpc, sc, wallet, u } from "@cityofzion/neon-js";
const client = new rpc.RPCClient("https://testnet1.neo.coz.io:443");
const ghostmarketContract = "0xc414275b3eca3969c4ca49f6a1fb67013fbe0544";
const res = await client.invokeFunction(ghostmarketContract, "ownerOf", [
sc.ContractParam.byteArray("JwE="),
]);
const b64 = res.stack[0].value;
const hex = u.base642hex(b64);
const address = wallet.getAddressFromScriptHash(hex);
console.log(address);
// NdfRZ4JvyKw4F2kK7K7ffSNUm5Y18F7ASw <-- wrong, but why?
The return type of ownerOf is Hash160, but the returned value is a byte string:
RPC Response Type:
Expected behavior The value must be reversed / parsed properly if the type is Hash160 or Hash256.
** Additional context** This is a major breaking change, but it also fixes an annoying issue on Neo. Most applications already deal with this bug as it was a feature, so changing this will require developers to make changes to their applications. This will be 'chaotic' for existing developers, but will make it much easier for new ones.
**I'm not 100% sure if this is something on the SDKs or on the RPC node, but it's probably on the RPC API.
That is not how it is done. There is no way in the VM to know what is hash160 or hash256
CSharp
var address = new UInt160(Convert.FromBase64String(byteStringValue));
Hello,
Maybe not the VM, but the node. The types are stored in the contract manifest.
That will change the interface and break all current apps using it.
Yes, and this is too dangerous to change. The best solution might be to add another RPC endpoint and change the SDKs first. We can argue that developers should use their own RPC nodes, but most applications use public RPC nodes. Then, it's probably ideal to introduce a new endpoint instead of changing the existing ones.
That's to much. When a developer can easily change the base64 to bytes and reverse bytes.
This can't be fixed, see https://github.com/neo-project/neo-modules/issues/609#issuecomment-875128591
Why it can't be fixed if it's an RPC endpoint?
The 'GetInvokeResults' method is not part of the VM. It could accept an additional parameter to parse types using the manifest.
If the conversion fails, it's because there is a bug in the smart contract. Most compilers won't allow you to return different types. This can only happen using "incorrect" storage calls.
If the conversion fails, the developer can either fix their contract or omit the "parse types" parameter.
- In C# contract compiler (and other compilers) it is allowed to return ByteString when the declared return type is UInt160. This saves GAS.
- At runtime in the VM, the type of returned value is not strictly enforced.
- ByteString is little-endian
- ScriptHash is big-endian.
const b64 = res.stack[0].value; // little-endian
const hex = u.base642hex(b64); // little-endian
const address = wallet.getAddressFromScriptHash(hex); // getAddressFromScriptHash accepts only big-endian, but `hex` is little-endian