Skip to content

Improve runtime compatibility checking logic. #99

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 15 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions libs/substrate-differ/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ document-features = { version = "0.2" }
thiserror = "1.0"
serde = { version = "1.0", features = ["derive", "rc"] }
comparable = { version = ">=0.5.3", features = ["derive", "serde"] }
wasm-testbed = { version = "0.21.3", path = "../wasm-testbed" }
wasm-testbed = { version = "0.21.3", path = "../wasm-testbed", optional = true }
scale-info = { version = "2.11.3", default-features = false, features = [
"derive",
"std",
Expand All @@ -21,15 +21,19 @@ scale-info = { version = "2.11.3", default-features = false, features = [
frame-metadata = { version = "16", package = "frame-metadata", features = [
"std", "legacy"
] }
blake3 = "1.5"

[dev-dependencies]
wasm-loader = { version = "0.21.3", path = "../wasm-loader" }

[features]
## The default feature currently excludes feature that are deprecated
default = ["v14", "reduced"]
default = ["v14", "reduced", "runtimes"]
deprecated = ["v12", "v13"]

## Support loading runtimes (wasm files)
runtimes = ["wasm-testbed"]

## v12 runtimes
v12 = []

Expand Down
1 change: 1 addition & 0 deletions libs/substrate-differ/src/differs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pub mod diff_method;

pub mod reduced;
#[cfg(feature = "runtimes")]
pub mod summary;

pub mod utils;
Expand Down
41 changes: 22 additions & 19 deletions libs/substrate-differ/src/differs/reduced/calls/call.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use super::{
prelude::*,
signature::{Arg, Signature},
};
use super::{fields_to_args, prelude::*, signature::Signature};
use comparable::Comparable;
use serde::Serialize;
use std::{collections::BTreeMap, fmt::Display};
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
use std::sync::Arc;

/// Reduced Call
#[derive(Debug, PartialEq, Serialize, Hash, Comparable, PartialOrd, Ord, Eq, Clone)]
#[derive(Debug, Deserialize, Serialize, Comparable, PartialEq, Clone)]
#[self_describing]
pub struct Call {
pub index: ExtrinsicId,
pub name: String,
Expand All @@ -17,17 +16,21 @@ pub struct Call {
pub docs: Documentation,
}

impl Display for Call {
impl std::fmt::Display for Call {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_fmt(format_args!("{: >2}: {} ( {} )", self.index, self.name, self.signature))
}
}

// impl Display for CallChange {
// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// f.write_fmt(format_args!("CALL {self}"))
// }
// }
impl std::fmt::Display for CallChange {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Signature(sig) => f.write_fmt(format_args!("{}", sig))?,
_ => f.write_fmt(format_args!("{:?}", self))?,
}
Ok(())
}
}

// impl Call {
// pub fn comp(&self) {
Expand All @@ -38,15 +41,14 @@ impl Display for Call {
// }
// }

pub fn variant_to_calls(td: &TypeDefVariant<PortableForm>) -> BTreeMap<PalletId, Call> {
pub fn variant_to_calls(
registry: &Arc<PortableRegistry>,
td: &TypeDefVariant<PortableForm>,
) -> BTreeMap<PalletId, Call> {
td.variants
.iter()
.map(|vv| {
let args = vv
.fields
.iter()
.map(|f| Arg { name: f.name.clone().unwrap_or_default(), ty: f.type_name.clone().unwrap_or_default() })
.collect();
let args = fields_to_args(registry, &vv.fields);

// PalletItem::Call(PalletData {
// index: Indexme(vv.index()Indexs u32),
Expand All @@ -70,6 +72,7 @@ pub fn variant_to_calls(td: &TypeDefVariant<PortableForm>) -> BTreeMap<PalletId,
#[cfg(test)]
mod test_reduced_call {
use super::*;
use crate::differs::reduced::calls::Arg;

#[test]
fn test_call() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,20 @@
use super::{call::*, constant::*, error::*, event::*, signature::*, storage::*};
use super::{call::*, constant::*, error::*, event::*, hashed_type::HashedTypeChange, signature::*, storage::*};
use crate::differs::reduced::{diff_analyzer::Compatible, prelude::ReducedPalletChange};
use comparable::{MapChange, VecChange};
use comparable::{Changed, MapChange, StringChange, VecChange};
use log::trace;

impl Compatible for ReducedPalletChange {
fn compatible(&self) -> bool {
let res = match self {
ReducedPalletChange::Index(_) => false,
ReducedPalletChange::Name(_) => false,
ReducedPalletChange::StoragePrefix(_) => false,

ReducedPalletChange::Calls(x) => x
.iter()
.map(|i| match i {
MapChange::Added(_k, _d) => true,
MapChange::Removed(_k) => false,
MapChange::Changed(_k, c) => c.iter().map(|cc| cc.compatible()).all(|x| x),
})
.all(|x| x),
ReducedPalletChange::Events(_x) => true,
ReducedPalletChange::Errors(_x) => true,

ReducedPalletChange::Calls(x) => x.compatible(),
ReducedPalletChange::Events(x) => x.compatible(),
ReducedPalletChange::Errors(x) => x.compatible(),
ReducedPalletChange::Constants(_x) => true,
ReducedPalletChange::Storages(_x) => true,
ReducedPalletChange::Storages(x) => x.compatible(),
};

trace!("Compat. | Pallet: {res}");
Expand All @@ -47,6 +40,7 @@ impl Compatible for ConstantChange {
let res = match self {
ConstantChange::Name(_) => false,
ConstantChange::Value(_) => true,
ConstantChange::Ty(_) => false,
};
trace!("Compat. | Constant: {res}");
res
Expand All @@ -69,7 +63,7 @@ impl Compatible for ErrorChange {
fn compatible(&self) -> bool {
let res = match self {
ErrorChange::Index(_) => false,
ErrorChange::Name(_) => false,
ErrorChange::Name(_) => true,
};
trace!("Compat. | Error: {res}");
res
Expand All @@ -81,22 +75,54 @@ impl Compatible for StorageChange {
let res = match self {
StorageChange::Name(_) => false,
StorageChange::Modifier(_) => false,
StorageChange::Ty(ty) => ty.compatible(),
StorageChange::DefaultValue(_) => true,
};
trace!("Compat. | Storage: {res}");
res
}
}

impl Compatible for StorageTypeChange {
fn compatible(&self) -> bool {
let res = match self {
Self::BothPlain(ty) => ty.compatible(),
Self::BothMap { hashers, key, value } => hashers.is_unchanged() && key.compatible() && value.compatible(),
Self::Different(_, _) => false,
};
trace!("Compat. | StorageType: {res}");
res
}
}

impl Compatible for SignatureChange {
fn compatible(&self) -> bool {
let res = self.args.iter().map(|arg_changes| arg_changes.compatible()).all(|x| x);
let res = self.args.compatible();
trace!("Compat. | Signature: {res}");
res
}
}

impl Compatible for VecChange<ArgDesc, Vec<ArgChange>> {
impl<T: Compatible> Compatible for Changed<T> {
fn compatible(&self) -> bool {
let res = match self {
Changed::Unchanged => true,
Changed::Changed(x) => x.compatible(),
};
trace!("Compat. | {}: {res}", std::any::type_name::<Self>());
res
}
}

impl<T: Compatible> Compatible for Vec<T> {
fn compatible(&self) -> bool {
let res = self.iter().map(|c| c.compatible()).all(|x| x);
trace!("Compat. | {}: {res}", std::any::type_name::<Self>());
res
}
}

impl Compatible for VecChange<Arg, Vec<ArgChange>> {
fn compatible(&self) -> bool {
let res = match self {
VecChange::Added(_size, _desc) => false,
Expand All @@ -108,21 +134,49 @@ impl Compatible for VecChange<ArgDesc, Vec<ArgChange>> {
}
}

impl Compatible for Vec<ArgChange> {
impl<Key, Desc, Change: Compatible> Compatible for MapChange<Key, Desc, Change> {
fn compatible(&self) -> bool {
let res = self.iter().map(|c| c.compatible()).all(|x| x);
trace!("Compat. | Vec<ArgChange>: {res}");
let res = match self {
MapChange::Added(_key, _desc) => true,
MapChange::Removed(_key) => false,
MapChange::Changed(_key, change) => change.compatible(),
};
trace!("Compat. | {}: {res}", std::any::type_name::<Self>());
res
}
}

impl Compatible for ArgChange {
fn compatible(&self) -> bool {
let res = match self {
ArgChange::Name(_) => false,
ArgChange::Ty(_) => false,
ArgChange::Name(StringChange(old, new)) => {
// Ignore underscore prefix change.
if old.trim_start_matches('_') == new.trim_start_matches('_') {
true
} else {
false
}
}
ArgChange::Ty(ty) => ty.compatible(),
};
trace!("Compat. | ArgChange: {res}");
res
}
}

impl Compatible for HashedTypeChange {
fn compatible(&self) -> bool {
let res = match self {
// The type changed.
HashedTypeChange::TypeChanged(_) => false,
// If only the name changed, then it is compatible. This is to allow types to be renamed or moved to different modules.
HashedTypeChange::NameChanged(_) => true,
// If the type hash changed, then it is not compatible.
HashedTypeChange::HashChanged(_) => false,
// If both name and hash changed then it is not compatible.
HashedTypeChange::NameAndHashChanged(_, _) => false,
};
trace!("Compat. | HashedTypeChange: {res}");
res
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ impl RequireTransactionVersionBump for ReducedPalletChange {
.any(|x| x),

ReducedPalletChange::Name(_) => false,
ReducedPalletChange::StoragePrefix(_) => false,
ReducedPalletChange::Events(_x) => false,
ReducedPalletChange::Errors(_x) => false,
ReducedPalletChange::Storages(_x) => false,
Expand Down Expand Up @@ -80,28 +81,20 @@ impl RequireTransactionVersionBump for SignatureChange {
}
}

impl RequireTransactionVersionBump for VecChange<ArgDesc, Vec<ArgChange>> {
impl RequireTransactionVersionBump for VecChange<Arg, Vec<ArgChange>> {
fn require_tx_version_bump(&self) -> bool {
let res = match self {
// If an arg is added/removed, the call will no longer be **compatible** but that does not require a tx_version bump
VecChange::Added(_size, _desc) => false,
VecChange::Removed(_size, _desc) => false,
VecChange::Added(_idx, _desc) => false,
VecChange::Removed(_idx, _desc) => false,

VecChange::Changed(_size, change) => change.require_tx_version_bump(),
VecChange::Changed(_idx, change) => change.require_tx_version_bump(),
};
trace!("TxBump | VecChange<...>: {res}");
res
}
}

impl RequireTransactionVersionBump for Vec<ArgChange> {
fn require_tx_version_bump(&self) -> bool {
let res = self.iter().map(|c| c.require_tx_version_bump()).any(|x| x);
trace!("TxBump | Vec<ArgChange>: {res}");
res
}
}

impl RequireTransactionVersionBump for ArgChange {
fn require_tx_version_bump(&self) -> bool {
let res = match self {
Expand All @@ -115,3 +108,23 @@ impl RequireTransactionVersionBump for ArgChange {
res
}
}

impl<T: RequireTransactionVersionBump> RequireTransactionVersionBump for Vec<T> {
fn require_tx_version_bump(&self) -> bool {
let res = self.iter().map(|c| c.require_tx_version_bump()).any(|x| x);
trace!("TxBump | Vec<T>: {res}");
res
}
}

impl<Key, Desc, Change: RequireTransactionVersionBump> RequireTransactionVersionBump for MapChange<Key, Desc, Change> {
fn require_tx_version_bump(&self) -> bool {
let res = match self {
MapChange::Added(_key, _desc) => false,
MapChange::Removed(_key) => true,
MapChange::Changed(_key, change) => change.require_tx_version_bump(),
};
trace!("TxBump | {}: {res}", std::any::type_name::<Self>());
res
}
}
Loading
Loading