Skip to content

Update Type Definitions to latest PolkadotJS #34

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.

Sign up for GitHub

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

Merged
dvdplm merged 21 commits into from
Dec 8, 2020
Merged

Update Type Definitions to latest PolkadotJS #34

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d3f2fc6
update definitions to latest polkadot.js
Dec 3, 2020
26fb7c6
handle custom bit-size declarations for polkadotjs
Dec 3, 2020
6cbc5af
update type definitions
Dec 3, 2020
02f19e1
update substrate
Dec 3, 2020
7e6e3f1
fix type in warning
Dec 3, 2020
841a8a3
include older address decoding from https://github.com/paritytech/sub…
Dec 3, 2020
1f5d5b6
remove duplicate match in enum on MultiAddress
Dec 3, 2020
5d58622
merge with master
Dec 3, 2020
c3cc791
rustfmt
Dec 3, 2020
022a859
remove comment with old implementation
Dec 3, 2020
ae81fb7
typo
Dec 3, 2020
d4d67c9
remove backtrace
Dec 3, 2020
6d45bf1
fix clippy lints
Dec 3, 2020
ff867b1
Update core/src/decoder.rs
insipx Dec 4, 2020
9441229
move need_more_than into decode_old_address
Dec 4, 2020
3de3101
Merge branch 'insipx/update-type-definitions' of github.com:paritytec…
Dec 4, 2020
c6bdd11
Update core/src/decoder.rs
insipx Dec 5, 2020
6dc9052
Update core/src/lib.rs
insipx Dec 6, 2020
c288fd0
Update core/src/decoder.rs
insipx Dec 7, 2020
0b42103
fix whitespace & capture groups
Dec 7, 2020
d67d602
Merge branch 'insipx/update-type-definitions' of github.com:paritytec…
Dec 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
889 changes: 498 additions & 391 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ edition = "2018"

[dependencies]
log = "0.4"
thiserror = "1.0"
thiserror = "1.0.22"
codec = { version = "1", package = "parity-scale-codec" }
serde = { version = "1.0", features = ["derive"] }
serde_json = { version = "1.0", features = ["preserve_order"] }
Expand All @@ -15,15 +15,15 @@ derive_more = "0.99.3"
dyn-clone = "1.0"
hex = "0.4"

runtime-version = { package = "sp-version", git = "https://github.com/paritytech/substrate", rev = "7314a78d49d0dc3862c6ef7c8115cb1b3d0c0fd2" }
pallet-indices = { package = "pallet-indices", git = "https://github.com/paritytech/substrate", rev = "7314a78d49d0dc3862c6ef7c8115cb1b3d0c0fd2" }
pallet-democracy = { package = "pallet-democracy", git = "https://github.com/paritytech/substrate", rev = "7314a78d49d0dc3862c6ef7c8115cb1b3d0c0fd2" }
pallet-identity = { package = "pallet-identity", git = "https://github.com/paritytech/substrate", rev = "7314a78d49d0dc3862c6ef7c8115cb1b3d0c0fd2" }
runtime-version = { package = "sp-version", git = "https://github.com/paritytech/substrate", rev = "a364e27d6e3971d756d28435efc468d95add52d3" }
pallet-indices = { package = "pallet-indices", git = "https://github.com/paritytech/substrate", rev = "a364e27d6e3971d756d28435efc468d95add52d3" }
pallet-democracy = { package = "pallet-democracy", git = "https://github.com/paritytech/substrate", rev = "a364e27d6e3971d756d28435efc468d95add52d3" }
pallet-identity = { package = "pallet-identity", git = "https://github.com/paritytech/substrate", rev = "a364e27d6e3971d756d28435efc468d95add52d3" }

primitives = { package = "sp-core", git = "https://github.com/paritytech/substrate", rev = "7314a78d49d0dc3862c6ef7c8115cb1b3d0c0fd2" }
runtime-primitives = { package = "sp-runtime", git = "https://github.com/paritytech/substrate", rev = "7314a78d49d0dc3862c6ef7c8115cb1b3d0c0fd2" }
primitives = { package = "sp-core", git = "https://github.com/paritytech/substrate", rev = "a364e27d6e3971d756d28435efc468d95add52d3" }
runtime-primitives = { package = "sp-runtime", git = "https://github.com/paritytech/substrate", rev = "a364e27d6e3971d756d28435efc468d95add52d3" }

runtime-metadata-latest = { package = "frame-metadata", git = "https://github.com/paritytech/substrate", rev = "7314a78d49d0dc3862c6ef7c8115cb1b3d0c0fd2" }
runtime-metadata-latest = { package = "frame-metadata", git = "https://github.com/paritytech/substrate", rev = "a364e27d6e3971d756d28435efc468d95add52d3" }
runtime-metadata11 = { git = "https://github.com/insipx/substrate-metadata-versions", package = "frame-metadatav11", branch = "master" }
runtime-metadata10 = { git = "https://github.com/insipx/substrate-metadata-versions", package = "frame-metadatav10", branch = "master" }
runtime-metadata09 = { git = "https://github.com/insipx/substrate-metadata-versions", package = "frame-metadatav9", branch = "master" }
Expand Down
84 changes: 59 additions & 25 deletions core/src/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ impl Decoder {
RustTypeMarker::Array { size, ty } => {
log::trace!("Array::cursor={}", *cursor);
let mut decoded_arr = Vec::with_capacity(*size);
if *size == 0 as usize {
if *size == 0_usize {
log::trace!("Returning Empty Vector");
return Ok(SubstrateType::Composite(Vec::new()));
} else {
Expand Down Expand Up @@ -467,7 +467,7 @@ impl Decoder {
self.decode_single(module, spec, v, data, cursor, true)?
}
},
RustTypeMarker::Generic((outer, _)) => {
RustTypeMarker::Generic(outer, _) => {
log::trace!("Generic Type");
// disregard 'inner' type of a generic
self.decode_single(module, spec, outer, data, cursor, is_compact)?
Expand Down Expand Up @@ -732,34 +732,25 @@ impl Decoder {
*cursor += 1;
Ok(Some(SubstrateType::GenericVote(vote)))
}
// Old Address Format for backwards-compatibility https://github.com/paritytech/substrate/pull/7380
"Lookup" | "GenericAddress" | "GenericLookupSource" | "GenericAccountId" => {
// a specific type that is <T as StaticSource>::Lookup concatenated to just 'Lookup'
log::trace!("cursor={}, data length={}", cursor, data.len());
let inc: usize;
// TODO: requires more investigation
// cursor increments for 0x00 .. 0xfe may be incorrect
match data[*cursor] {
0x00..=0xef => {
inc = 0;
}
0xfc => {
inc = 2;
}
0xfd => {
inc = 4;
}
0xfe => {
inc = 8;
}
0xff => {
inc = 32;
}
_ => return Err(Error::Fail("Invalid Address".to_string())),
}

let val: substrate_types::Address = Decode::decode(&mut &data[*cursor..])?;
let val: substrate_types::Address = decode_old_address(data, cursor)?;

*cursor += inc + 1; // +1 for byte 0x00-0xff
Ok(Some(SubstrateType::Address(val)))
}
"GenericMultiAddress" => {
let val: substrate_types::Address = Decode::decode(&mut &data[*cursor..])?;
let cursor_offset = match &val {
substrate_types::Address::Id(_) => 32,
substrate_types::Address::Index(_) => 1,
substrate_types::Address::Raw(v) => v.len(),
substrate_types::Address::Address32(_) => 32,
substrate_types::Address::Address20(_) => 20,
};
*cursor += cursor_offset;
Ok(Some(SubstrateType::Address(val)))
}
"Era" => {
Expand Down Expand Up @@ -823,6 +814,49 @@ impl Decoder {
}
}

/// Decodes old address pre-refactor (https://github.com/paritytech/substrate/pull/7380)
/// and converts it to a MultiAddress
fn decode_old_address(data: &[u8], cursor: &mut usize) -> Result<substrate_types::Address, Error> {
Copy link
Contributor

@niklasad1 niklasad1 Dec 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@insipx insipx Dec 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@niklasad1 niklasad1 Dec 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, cool

/// Kept around for backwards-compatibility with old address struct
fn need_more_than<T: PartialOrd>(a: T, b: T) -> Result<T, Error> {
if a < b {
Ok(b)
} else {
Err("Invalid range".into())
}
}

let inc;
let addr = match data[*cursor] {
x @ 0x00..=0xef => {
inc = 0;
substrate_types::Address::Index(x as u32)
}
0xfc => {
inc = 2;
substrate_types::Address::Index(need_more_than(0xef, u16::decode(&mut &data[(*cursor + 1)..])?)? as u32)
}
0xfd => {
inc = 4;
substrate_types::Address::Index(need_more_than(0xffff, u32::decode(&mut &data[(*cursor + 1)..])?)?)
}
0xfe => {
inc = 8;
substrate_types::Address::Index(need_more_than(
0xffffffffu32,
Decode::decode(&mut &data[(*cursor + 1)..])?,
)?)
}
0xff => {
inc = 32;
substrate_types::Address::Id(Decode::decode(&mut &data[(*cursor + 1)..])?)
}
_ => return Err(Error::Fail("Invalid Address".to_string())),
};
*cursor += inc + 1; // +1 for byte 0x00-0xff
Ok(addr)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
4 changes: 2 additions & 2 deletions core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ pub enum RustTypeMarker {

/// A Generic Type, EX: HeartBeat<BlockNumber>
/// Tuple of (OuterType, InnerType)
Generic((Box<RustTypeMarker>, Box<RustTypeMarker>)),
Generic(Box<RustTypeMarker>, Box<RustTypeMarker>),
/// primitive unsigned 8 bit integer
U8,
/// primtiive unsigned 16 bit integer
Expand Down Expand Up @@ -288,7 +288,7 @@ impl Display for RustTypeMarker {
}
RustTypeMarker::Array { size, ty } => type_marker.push_str(&format!("[{};{}], ", ty, size)),
RustTypeMarker::Std(t) => type_marker.push_str(&t.to_string()),
RustTypeMarker::Generic((outer, inner)) => type_marker.push_str(&format!("{}<{}>", outer, inner)),
RustTypeMarker::Generic(outer, inner) => type_marker.push_str(&format!("{}<{}>", outer, inner)),
RustTypeMarker::U8 => type_marker.push_str("u8"),
RustTypeMarker::U16 => type_marker.push_str("u16"),
RustTypeMarker::U32 => type_marker.push_str("u32"),
Expand Down
54 changes: 52 additions & 2 deletions core/src/regex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use onig::{Regex, Region, SearchOptions};
#[derive(Debug, Clone, PartialEq, Eq)]
enum RegexSet {
ArrayPrimitive,
BitSize,
ArrayStruct,
Vec,
Option,
Expand All @@ -37,6 +38,8 @@ impl RegexSet {
fn get_type(s: &str) -> Option<RegexSet> {
if rust_array_decl_prim().is_match(s) {
Some(RegexSet::ArrayPrimitive)
} else if rust_bit_size().is_match(s) {
Some(RegexSet::BitSize)
} else if rust_array_decl_struct().is_match(s) {
Some(RegexSet::ArrayStruct)
} else if rust_vec_decl().is_match(s) {
Expand All @@ -61,6 +64,7 @@ impl RegexSet {
fn parse_type(&self, s: &str) -> Option<RustTypeMarker> {
match self {
RegexSet::ArrayPrimitive => parse_primitive_array(s),
RegexSet::BitSize => parse_bit_size(s),
RegexSet::ArrayStruct => parse_struct_array(s),
RegexSet::Vec => parse_vec(s),
RegexSet::Option => parse_option(s),
Expand Down Expand Up @@ -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")
Copy link
Contributor

@niklasad1 niklasad1 Dec 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@dvdplm dvdplm Dec 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@insipx insipx Dec 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@niklasad1 niklasad1 Dec 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, I see

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

}

/// Match a rust vector
/// allowed to be nested within, or have other (ie Option<>) nested within
pub fn rust_vec_decl() -> Regex {
Expand Down Expand Up @@ -354,7 +362,40 @@ fn parse_generic(s: &str) -> Option<RustTypeMarker> {
// account that a HeartBeat type in Polkadot is HeartBeat<T::BlockNumber>
let ty_inner = parse(ty_inner).expect("Must be a type; qed");

Some(RustTypeMarker::Generic((Box::new(ty_outer), Box::new(ty_inner))))
Some(RustTypeMarker::Generic(Box::new(ty_outer), Box::new(ty_inner)))
}

fn parse_bit_size(s: &str) -> Option<RustTypeMarker> {
let re = rust_bit_size();
if !re.is_match(s) {
return None;
}

let ty = re.captures(s)?.at(1)?;
let size = re.captures(s)?.at(2)?;

match ty {
"UInt" => match size.parse::<usize>().expect("Should always be a number") {
8 => Some(RustTypeMarker::U8),
16 => Some(RustTypeMarker::U16),
32 => Some(RustTypeMarker::U32),
64 => Some(RustTypeMarker::U64),
128 => Some(RustTypeMarker::U128),
s => Some(RustTypeMarker::Array { size: s, ty: Box::new(RustTypeMarker::U8) }),
},
"Int" => match size.parse::<usize>().expect("Should always be number") {
8 => Some(RustTypeMarker::I8),
16 => Some(RustTypeMarker::I16),
32 => Some(RustTypeMarker::I32),
64 => Some(RustTypeMarker::I64),
128 => Some(RustTypeMarker::I128),
s => Some(RustTypeMarker::Array { size: s, ty: Box::new(RustTypeMarker::U8) }),
},
_ => {
log::warn!("Could not ascertain type of bit-size declaration");
None
}
}
}

/// recursively parses a regex set
Expand Down Expand Up @@ -674,7 +715,7 @@ mod tests {
let re = rust_generic_decl();
let caps = re.captures("GenericOuterType<GenericInnerType>").unwrap();
assert_eq!(
vec![Some("GenericOuterType<GenericInnerType>"), Some("GenericOuterType"), Some("GenericInnerType")],
vec![Some("GenericOuterType<GenericInnerType>"), Some("GenericOuterType"), Some("GenericInnerType"),],
caps.iter().collect::<Vec<Option<&str>>>()
);
}
Expand Down Expand Up @@ -829,4 +870,13 @@ mod tests {
let res = parse_vec(ty).unwrap();
log::debug!("{:?}", res);
}

#[test]
fn should_parse_bit_size() {
pretty_env_logger::try_init();
let ty = "UInt<128, Balance>";
assert_eq!(parse_bit_size(ty).unwrap(), RustTypeMarker::U128);
let ty = "Int<64, Balance>";
assert_eq!(parse_bit_size(ty).unwrap(), RustTypeMarker::I64);
}
}
9 changes: 6 additions & 3 deletions core/src/substrate_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use primitives::crypto::{Ss58AddressFormat, Ss58Codec};
use serde::Serialize;
use std::{convert::TryFrom, fmt};

pub type Address = pallet_indices::address::Address<AccountId32, u32>;
pub type Address = runtime_primitives::MultiAddress<AccountId32, u32>;
pub type Vote = pallet_democracy::Vote;
pub type Conviction = pallet_democracy::Conviction;
pub type Data = pallet_identity::Data;
Expand Down Expand Up @@ -135,10 +135,13 @@ impl fmt::Display for SubstrateType {
},
SubstrateType::GenericVote(v) => write!(f, "Aye={}, Conviction={}", v.aye, v.conviction.lock_periods()),
SubstrateType::Address(v) => match v {
pallet_indices::address::Address::Id(ref i) => {
runtime_primitives::MultiAddress::Id(ref i) => {
write!(f, "Account::Id({})", i.to_ss58check_with_version(Ss58AddressFormat::SubstrateAccount))
}
pallet_indices::address::Address::Index(i) => write!(f, "Index: {:?}", i),
runtime_primitives::MultiAddress::Index(i) => write!(f, "Index: {:?}", i),
runtime_primitives::MultiAddress::Raw(bytes) => write!(f, "Raw: {:?}", bytes),
runtime_primitives::MultiAddress::Address32(ary) => write!(f, "Address32: {:?}", ary),
runtime_primitives::MultiAddress::Address20(ary) => write!(f, "Address20: {:?}", ary),
},
SubstrateType::Data(d) => write!(f, "{:?}", d),
SubstrateType::SignedExtra(v) => write!(f, "{}", v),
Expand Down
8 changes: 8 additions & 0 deletions core/src/substrate_types/remote.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,16 @@ use super::{Address, Conviction, Data, Vote};
#[derive(Serialize, Deserialize)]
#[serde(remote = "Address")]
pub enum RemoteAddress {
/// It's an account ID (pubkey).
Id(AccountId32),
/// It's an account index.
Index(u32),
/// It's some arbitrary raw bytes.
Raw(Vec<u8>),
/// It's a 32 byte representation.
Address32([u8; 32]),
/// It's a 20 byte representation.
Address20([u8; 20]),
}

#[derive(Serialize, Deserialize)]
Expand Down
7 changes: 5 additions & 2 deletions core/src/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,14 @@ pub fn as_substrate_address<S: Serializer>(ty: &SubstrateType, serializer: S) ->
serializer.serialize_str(&addr)
}
SubstrateType::Address(v) => match v {
pallet_indices::address::Address::Id(ref i) => {
runtime_primitives::MultiAddress::Id(ref i) => {
let addr = i.to_ss58check_with_version(Ss58AddressFormat::SubstrateAccount);
serializer.serialize_str(&addr)
}
pallet_indices::address::Address::Index(i) => serializer.serialize_str(&format!("{}", i)),
runtime_primitives::MultiAddress::Index(i) => serializer.serialize_str(&format!("{}", i)),
runtime_primitives::MultiAddress::Raw(bytes) => serializer.serialize_str(&format!("{:?}", bytes)),
runtime_primitives::MultiAddress::Address32(ary) => serializer.serialize_str(&format!("{:?}", ary)),
runtime_primitives::MultiAddress::Address20(ary) => serializer.serialize_str(&format!("{:?}", ary)),
},
_ => Err(ser::Error::custom(format!("Could not format {:?} as Ss58 Address", ty))),
}
Expand Down
Loading