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

Struct containing Vec<u8> followed by other fields is decoded incorrectly. #1432

Closed
bddap opened this issue Oct 1, 2019 · 6 comments
Labels

Comments

@bddap
Copy link

@bddap bddap commented Oct 1, 2019

Given the the rust struct:

#[derive(Encode, Decode)]
pub struct Erc20Token {
    pub name: Vec<u8>,
    pub ticker: Vec<u8>,
    pub total_supply: u128,
}

and this custom type declared in the web-ui

{
    "Erc20Token": {
        "name": "Vec<u8>",
        "ticker": "Vec<u8>",
        "total_supply": "u128"
    }
}

when Erc20Token is queried over rpc (from substrate) the encoded form is
0x204e414d454649454c185449434b4552ffffffffffffffffffff.

0x20_4e414d454649454c_18_5449434b4552_ffffffffffffffffffff
  ^^ ^^^^^^^^^^^^^^^^ ^^ ^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
   |       |           |      |                |
   | Name field. Set   |   "TICKER"   total_supply
   | to "NAMEFIEL".    |                 = u128::max_value()
   |                   |
 Single byte length   Length prefix for
 prefix for name      ticker field.
 field.

Apparently, substrate encodes the length fields as variable width integers. That's why they only take up one byte in this case. https://substrate.dev/docs/en/overview/low-level-data-format#compact-general-integers

The encoded value was generated from this object:

Erc20Token {
    name: b"NAMEFIEL".to_vec(),
    ticker: b"TICKER".to_vec(),
    total_supply: u128::max_value(),
}

Problem

0x204e414d454649454c185449434b4552ffffffffffffffffffff should decode to something like:

{
    "name": "0x4e414d454649454c",
    "ticker": "0x5449434b4552",
    "total_supply": 938463463374607431768211455
}

The actual value is:

{
    "name": "0x204e414d454649454c185449434b4552ffffffffffffffffffffffffffffffff",
	"ticker": "0x",
	"total_supply": 0
}

It seems that when decoding the first field, "name", the entire input is consumed.

Side Note If `name` did in fact consume all the input. `ticker` and `total_supply` must have been the result of parsing empty input. When parsing, its usually best not to silently ignore end-of-input. I suggest reporting an error instead of returning incorrect values. 😊

After incorrectly decoding the struct the following warning is posted to the browser console.

{"name":"Bytes","ticker":"Bytes","total_supply":"u128"}:: Input doesn't match output, received 0x204e414d454649454c185449434b4552ffffffffffffffffffffffffffffffff, created 0x80204e414d454649454c185449434b4552ffffffffffffffffffffffffffffffff0000000000000000000000000000000000
@jacogr

This comment has been minimized.

Copy link
Contributor

@jacogr jacogr commented Oct 2, 2019

Yes, any variable-length structure is length-prefixed, where <= 64 is encoded with length << 2, so 0x20 is 8 bytes.

Taking your inputs and doing a quick test in the apps UI JS console, yields the following -

const createType = types.createType;
const getTypeRegistry = types.getTypeRegistry;

getTypeRegistry().register({
  "token": {
    "name": "Vec<u8>",
    "ticker": "Vec<u8>",
    "total_supply": "u128"
  }
});

console.log(
  createType('token', '0x204e414d454649454c185449434b4552ffffffffffffffffffff')
);

Output -

{name: 0x4e414d454649454c, ticker: 0x5449434b4552, total_supply: 1208925819614629174706175}

So how and you getting to your outputs? (If you are doing this via storage UI, it could very well be that there is a mismatch in the UI portions)

@jacogr jacogr added the <question> label Oct 2, 2019
@bddap

This comment has been minimized.

Copy link
Author

@bddap bddap commented Oct 2, 2019

Yes, the issue arises when using the storage ui.

Using the runtime defined here.
Querying the token_details getter.

@jacogr

This comment has been minimized.

Copy link
Contributor

@jacogr jacogr commented Oct 2, 2019

I normally don't pull external runtimes (since I have 5 branches of substrate/polkadot repos at any one time, which takes a toll in compiling), but was quite interested to see this. Steps -

  • add types (also included "TokenBalance": "u128" since that was is needed)
  • register via init (which I'm hoping is the correct call)
  • query via get

Result -

image

This matches up with the API-via-toolbox test done this morning as well, so not quite sure what you are seeing.

@bddap

This comment has been minimized.

Copy link
Author

@bddap bddap commented Oct 4, 2019

The original encoded value came from a genesis config in a chainspec declaration (I'm working to publish a version of the erc20 module that accepts genesis parameters).

The module is not yet published and the issue doesn't seem to be reproducible without it.

Thanks for applying the effort to reproduce the problem.

If you want to close the issue now I'll post an update if/when I have a simple way to reproduce.

@bddap

This comment has been minimized.

Copy link
Author

@bddap bddap commented Oct 4, 2019

For future reference, the problem seems related to either serialization of genesis parameters on the Rust side or "Vec<u8>" being replaced with "Bytes" in type declarations.

@jacogr

This comment has been minimized.

Copy link
Contributor

@jacogr jacogr commented Oct 4, 2019

“Bytes” on the JS side is the implementation of ”Vec” from Rust - it is a type alias on the JS side. (So it handles the length prefixed logic on a Uint8Array transparently) this has been in the API since poc-1, so the logic is quite battle tested by now ;)

Just make sure that where it is encoded on the genesis side, it apply the correct SCALE encoding logic. It should just transparently apply it if the right types are defined.

(Cannot think off hand of a Vec example in Polkadot or Substrate genesis for this)

@jacogr jacogr closed this Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.