Update Type Definitions to latest PolkadotJS #34

insipx · 2020-12-03T18:25:02Z

Latest PolkadotJS includes two new types UInt<size, type> and Int<size, type>. There were a few ways to handle this:

with nothing done, desub recognizes this as a generic and tries to parse/decode it as such, but runs into name resolution issues because it won't be able to resolve UInt/Int (those definitions are internal to polkadot.js and not in the JSON). So we can instead include a special case to match on UInt and Int in the generic matching code and redirect it to instead decode this as a primitive integer
create new regex to parse UInt and Int as u{8-128} and Byte-arrays

I went with the second option as it seem the most flexible with future changes, and requires less code added to decoder.rs (which could be confusing as a generic redirects to a primitive)

Includes changes to update to latest master from #33

- Remove TYPES_META (it's not used, and not in the latest polkadot.js definitions) - Revise Generic Parsing regex

dvdplm

LGTM modulo nits

core/src/decoder.rs

Co-authored-by: David <dvdplm@gmail.com>

…h/desub into insipx/update-type-definitions

core/src/decoder.rs

Co-authored-by: David <dvdplm@gmail.com>

niklasad1 · 2020-12-06T11:05:50Z

core/src/decoder.rs

@@ -823,6 +814,49 @@ impl Decoder {
 	}
 }

+/// Decodes old address pre-refactor (https://github.com/paritytech/substrate/pull/7380)
+/// and converts it to a MultiAddress, where "old" here means anything before v0.8.26 or 26/2026/46 on polkadot/kusama/westend respectively.
+fn decode_old_address(data: &[u8], cursor: &mut usize) -> Result<substrate_types::Address, Error> {


AFAIU there's nothing checks that cursor and cursor + 1 are valid indexes into data slice but maybe checked somewhere else?

It may panic then.

Yeah,

This is part of the larger issue that the function decode_single doesn't do any bounds checking on the cursor at all, and panics if anything goes wrong. I'd rather address that in it's own PR though, since it might be a bit of a larger/medium sized refactor so I opened #35 to track that

core/src/decoder.rs

niklasad1 · 2020-12-06T11:52:44Z

core/src/regex.rs

@@ -90,6 +94,10 @@ fn rust_array_decl_struct() -> Regex {
 	Regex::new(r"^\[ *?([\w><]+) *?; *?(\d+) *?\]").expect("Primitive Regex expression invalid")
 }

+pub fn rust_bit_size() -> Regex {
+	Regex::new(r"^(Int|UInt)<([\w\d]+), ?([\w\d]+)>").expect("Regex expression should be infallible; qed")


I guess this regex is for Int<size, type>, I'm not a regex export but...

I'm surprised that [\d\w]+ is used for the size, it would match beef999bibimbap for example. Thus, if size is in decimal just use \d+ then.

Why , ?([\w\d]+) it would only work for Int<999,bar> and Int<999, bar> but not for arbitrary number of spaces :)

I suggest , *(\w+) instead.

I don't think the capture groups are necessary for the type params, i.e. this should be fine: (Int|UInt)<[\w\d]+, *[\w\d]+> and like niklas I'm not sure I understand what the input to this looks like: can both params contain both letters and number? A test that exercise this case would be good.

Here's a sample of some of the input from polkadot.js:

"Fixed64": "Int<64, Fixed64>", "FixedI64": "Int<64, FixedI64>", "FixedU64": "UInt<64, FixedU64>", "Fixed128": "Int<128, Fixed128>", "FixedI128": "Int<128, FixedI128>", "FixedU128": "UInt<128, FixedU128>", "I32F32": "Int<64, I32F32>", "U32F32": "UInt<64, U32F32>", "PerU16": "UInt<16, PerU16>", "Perbill": "UInt<32, Perbill>", "Percent": "UInt<8, Percent>", "Permill": "UInt<32, Permill>", "Perquintill": "UInt<64, Perquintill>", "Balance": "UInt<128, Balance>",

So the second type parameter is unnecessary, since it's already a key in our de-serialized JSON, but the first type parameter is needed to delegate the size of the type we are dealing with. This also leads me to believe that the first type parameter will always be a digit and the second just a repeat of the JSON key (I haven't found anything in the definitions that contradicts this). I'm not 100% on what the rationale of having the type repeat is in Polkadot.JS, but in the PR's I found like here they use the Type<BitLength> in that style.

I agree with @niklasad1 , though, arbitrary number of spaces is definitely preferable, fixes for these/and above incoming

All right, I see

We could do (Int|UInt)<[\d]+(, *[\w\d]+)?> instead to make the second type parameter optional then I guess

core/src/lib.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

…h/desub into insipx/update-type-definitions

Andrew Plaza added 13 commits December 3, 2020 18:05

update definitions to latest polkadot.js

d3f2fc6

- Remove TYPES_META (it's not used, and not in the latest polkadot.js definitions) - Revise Generic Parsing regex

handle custom bit-size declarations for polkadotjs

26fb7c6

update type definitions

6cbc5af

update substrate

02f19e1

fix type in warning

7e6e3f1

include older address decoding from paritytech/substrate#7380

841a8a3

remove duplicate match in enum on MultiAddress

1f5d5b6

merge with master

5d58622

rustfmt

c3cc791

remove comment with old implementation

022a859

typo

ae81fb7

remove backtrace

d4d67c9

fix clippy lints

6d45bf1

dvdplm approved these changes Dec 4, 2020

View reviewed changes

core/src/decoder.rs Outdated Show resolved Hide resolved

core/src/decoder.rs Outdated Show resolved Hide resolved

dvdplm mentioned this pull request Dec 4, 2020

Update to latest substrate #33

Closed

dvdplm requested review from kianenigma and niklasad1 December 4, 2020 09:55

insipx and others added 3 commits December 4, 2020 15:40

Update core/src/decoder.rs

ff867b1

Co-authored-by: David <dvdplm@gmail.com>

move need_more_than into decode_old_address

9441229

Merge branch 'insipx/update-type-definitions' of github.com:paritytec…

3de3101

…h/desub into insipx/update-type-definitions

dvdplm reviewed Dec 4, 2020

View reviewed changes

core/src/decoder.rs Outdated Show resolved Hide resolved

Update core/src/decoder.rs

c6bdd11

Co-authored-by: David <dvdplm@gmail.com>

niklasad1 reviewed Dec 6, 2020

View reviewed changes

core/src/decoder.rs Outdated Show resolved Hide resolved

niklasad1 reviewed Dec 6, 2020

View reviewed changes

core/src/lib.rs Outdated Show resolved Hide resolved

insipx and others added 4 commits December 6, 2020 23:27

Update core/src/lib.rs

6dc9052

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

Update core/src/decoder.rs

c288fd0

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

fix whitespace & capture groups

0b42103

Merge branch 'insipx/update-type-definitions' of github.com:paritytec…

d67d602

…h/desub into insipx/update-type-definitions

niklasad1 approved these changes Dec 8, 2020

View reviewed changes

dvdplm merged commit bee6388 into master Dec 8, 2020

dvdplm deleted the insipx/update-type-definitions branch December 8, 2020 10:17

insipx mentioned this pull request Dec 14, 2020

Fix CI #24

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update Type Definitions to latest PolkadotJS #34

Update Type Definitions to latest PolkadotJS #34

Uh oh!

insipx commented Dec 3, 2020 •

edited

Loading

Uh oh!

dvdplm left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

niklasad1 Dec 6, 2020 •

edited

Loading

Uh oh!

insipx Dec 7, 2020 •

edited

Loading

Uh oh!

niklasad1 Dec 8, 2020

Uh oh!

Uh oh!

niklasad1 Dec 6, 2020 •

edited

Loading

Uh oh!

dvdplm Dec 7, 2020

Uh oh!

insipx Dec 7, 2020 •

edited

Loading

Uh oh!

niklasad1 Dec 8, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Update Type Definitions to latest PolkadotJS #34

Update Type Definitions to latest PolkadotJS #34

Uh oh!

Conversation

insipx commented Dec 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dvdplm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

niklasad1 Dec 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

insipx Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niklasad1 Dec 8, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

niklasad1 Dec 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dvdplm Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

insipx Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niklasad1 Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

insipx commented Dec 3, 2020 •

edited

Loading

niklasad1 Dec 6, 2020 •

edited

Loading

insipx Dec 7, 2020 •

edited

Loading

niklasad1 Dec 6, 2020 •

edited

Loading

insipx Dec 7, 2020 •

edited

Loading

niklasad1 Dec 8, 2020 •

edited

Loading