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

Fix call enum's metadata regression #3513

Merged
merged 5 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
111 changes: 58 additions & 53 deletions docs/sdk/headers/header.html
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
sidebarElems.style.display = 'none';

// Add click event listener to the button
expandButton.addEventListener('click', function() {
expandButton.addEventListener('click', function () {
// Toggle the display of the '.sidebar-elems'
if (sidebarElems.style.display === 'none') {
sidebarElems.style.display = 'block';
Expand All @@ -72,6 +72,9 @@
if (!crate_name.textContent.startsWith("polkadot_sdk_docs")) {
console.log("skipping -- not `polkadot_sdk_docs`");
return;
} else {
// insert class 'sdk-docs' to the body, so it enables the custom css rules.
document.body.classList.add("sdk-docs");
}

createToC();
Expand All @@ -82,58 +85,60 @@
</script>

<style>
body.sdk-docs {
nav.side-bar {
width: 300px;
}

nav.side-bar {
width: 300px;
}

.sidebar-table-of-contents {
margin-bottom: 1em;
padding: 0.5em;
}

.sidebar-table-of-contents a {
display: block;
margin: 0.2em 0;
}

.sidebar-table-of-contents .h2 {
font-weight: bold;
margin-left: 0;
}

.sidebar-table-of-contents .h3 {
margin-left: 1em;
}

.sidebar-table-of-contents .h4 {
margin-left: 2em;
}

.sidebar h2.location {
display: none;
}

.sidebar-elems {
display: none;
}

/* Center the 'Expand for More' button */
.expand-button {
display: inline-block; /* Use inline-block for sizing */
margin: 10px auto; /* Auto margins for horizontal centering */
padding: 5px 10px;
background-color: #007bff;
color: white;
text-align: center;
cursor: pointer;
border: none;
border-radius: 5px;
width: auto;
/* Centering the button within its parent container */
position: relative;
left: 50%;
transform: translateX(-50%);
}
.sidebar-table-of-contents {
margin-bottom: 1em;
padding: 0.5em;
}

.sidebar-table-of-contents a {
display: block;
margin: 0.2em 0;
}

.sidebar-table-of-contents .h2 {
font-weight: bold;
margin-left: 0;
}

.sidebar-table-of-contents .h3 {
margin-left: 1em;
}

.sidebar-table-of-contents .h4 {
margin-left: 2em;
}

.sidebar h2.location {
display: none;
}

.sidebar-elems {
display: none;
}

/* Center the 'Expand for More' button */
.expand-button {
display: inline-block;
/* Use inline-block for sizing */
margin: 10px auto;
/* Auto margins for horizontal centering */
padding: 5px 10px;
background-color: #007bff;
color: white;
text-align: center;
cursor: pointer;
border: none;
border-radius: 5px;
width: auto;
/* Centering the button within its parent container */
position: relative;
left: 50%;
transform: translateX(-50%);
}
}
</style>
25 changes: 13 additions & 12 deletions docs/sdk/headers/theme.css
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
:root {
--polkadot-pink: #E6007A ;
--polkadot-green: #56F39A ;
--polkadot-lime: #D3FF33 ;
--polkadot-cyan: #00B2FF ;
--polkadot-purple: #552BBF ;
}

body > nav.sidebar > div.sidebar-crate > a > img {
/* logo width, given that the sidebar width is 200px; */
width: 190px;
--polkadot-pink: #E6007A;
--polkadot-green: #56F39A;
--polkadot-lime: #D3FF33;
--polkadot-cyan: #00B2FF;
--polkadot-purple: #552BBF;
}

body nav.sidebar {
flex: 0 0 250px;
body.sdk-docs {
nav.sidebar>div.sidebar-crate>a>img {
width: 190px;
}

nav.sidebar {
flex: 0 0 250px;
}
}
18 changes: 18 additions & 0 deletions prdoc/pr_3513.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Fix call enum's metadata regression

doc:
- audience: Runtime Dev
- audience: Runtime User
description: |
This PR fixes an issue where in the metadata of all FRAME-based chains, the documentation for
all extrinsic/call/transactions has been replaced by a single line saying "see Pallet::name".

Many wallets display the metadata of transactions to the user before signing, and this bug
might have affected this process.

crates:
- name: frame
- name: frame-support
21 changes: 16 additions & 5 deletions substrate/frame/support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,24 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
array-bytes = { version = "6.1", default-features = false }
serde = { features = ["alloc", "derive"], workspace = true }
codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false, features = ["derive", "max-encoded-len"] }
scale-info = { version = "2.10.0", default-features = false, features = ["derive"] }
frame-metadata = { version = "16.0.0", default-features = false, features = ["current"] }
sp-api = { path = "../../primitives/api", default-features = false, features = ["frame-metadata"] }
codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false, features = [
"derive",
"max-encoded-len",
] }
scale-info = { version = "2.10.0", default-features = false, features = [
"derive",
] }
frame-metadata = { version = "16.0.0", default-features = false, features = [
"current",
] }
sp-api = { path = "../../primitives/api", default-features = false, features = [
"frame-metadata",
] }
sp-std = { path = "../../primitives/std", default-features = false }
sp-io = { path = "../../primitives/io", default-features = false }
sp-runtime = { path = "../../primitives/runtime", default-features = false, features = ["serde"] }
sp-runtime = { path = "../../primitives/runtime", default-features = false, features = [
"serde",
] }
sp-tracing = { path = "../../primitives/tracing", default-features = false }
sp-core = { path = "../../primitives/core", default-features = false }
sp-arithmetic = { path = "../../primitives/arithmetic", default-features = false }
Expand Down
21 changes: 3 additions & 18 deletions substrate/frame/support/procedural/src/pallet/expand/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use crate::{
pallet::{
expand::warnings::{weight_constant_warning, weight_witness_warning},
parse::call::{CallVariantDef, CallWeightDef},
parse::call::CallWeightDef,
Def,
},
COUNTER,
Expand Down Expand Up @@ -112,22 +112,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
}
debug_assert_eq!(fn_weight.len(), methods.len());

let map_fn_docs = if !def.dev_mode {
// Emit the [`Pallet::method`] documentation only for non-dev modes.
|method: &CallVariantDef| {
let reference = format!("See [`Pallet::{}`].", method.name);
quote!(#reference)
}
} else {
// For the dev-mode do not provide a documenation link as it will break the
// `cargo doc` if the pallet is private inside a test.
|method: &CallVariantDef| {
let reference = format!("See `Pallet::{}`.", method.name);
quote!(#reference)
}
};

let fn_doc = methods.iter().map(map_fn_docs).collect::<Vec<_>>();
let fn_doc = methods.iter().map(|method| &method.docs).collect::<Vec<_>>();

let args_name = methods
.iter()
Expand Down Expand Up @@ -309,7 +294,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
),
#(
#cfg_attrs
#[doc = #fn_doc]
#( #[doc = #fn_doc] )*
#[codec(index = #call_index)]
#fn_name {
#(
Expand Down
45 changes: 38 additions & 7 deletions substrate/frame/support/test/tests/pallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ pub mod pallet {
where
T::AccountId: From<SomeType1> + From<SomeType3> + SomeAssociation1,
{
/// Doc comment put in metadata
/// call foo doc comment put in metadata
#[pallet::call_index(0)]
#[pallet::weight(Weight::from_parts(*foo as u64, 0))]
pub fn foo(
Expand All @@ -225,7 +225,7 @@ pub mod pallet {
Ok(().into())
}

/// Doc comment put in metadata
/// call foo_storage_layer doc comment put in metadata
#[pallet::call_index(1)]
#[pallet::weight({1})]
pub fn foo_storage_layer(
Expand Down Expand Up @@ -270,7 +270,7 @@ pub mod pallet {
#[pallet::error]
#[derive(PartialEq, Eq)]
pub enum Error<T> {
/// doc comment put into metadata
/// error doc comment put in metadata
InsufficientProposersBalance,
NonExistentStorageValue,
Code(u8),
Expand All @@ -287,9 +287,8 @@ pub mod pallet {
where
T::AccountId: SomeAssociation1 + From<SomeType1>,
{
/// doc comment put in metadata
/// event doc comment put in metadata
Proposed(<T as frame_system::Config>::AccountId),
/// doc
Spending(BalanceOf<T>),
Something(u32),
SomethingElse(<T::AccountId as SomeAssociation1>::_1),
Expand Down Expand Up @@ -750,8 +749,7 @@ pub type UncheckedExtrinsic =
sp_runtime::testing::TestXt<RuntimeCall, frame_system::CheckNonZeroSender<Runtime>>;

frame_support::construct_runtime!(
pub struct Runtime
{
pub struct Runtime {
// Exclude part `Storage` in order not to check its metadata in tests.
System: frame_system exclude_parts { Pallet, Storage },
Example: pallet,
Expand Down Expand Up @@ -1362,6 +1360,39 @@ fn migrate_from_pallet_version_to_storage_version() {
});
}

#[test]
fn pallet_item_docs_in_metadata() {
// call
let call_variants = match meta_type::<pallet::Call<Runtime>>().type_info().type_def {
scale_info::TypeDef::Variant(variants) => variants.variants,
_ => unreachable!(),
};

assert_eq!(call_variants[0].docs, vec!["call foo doc comment put in metadata"]);
assert_eq!(call_variants[1].docs, vec!["call foo_storage_layer doc comment put in metadata"]);
assert!(call_variants[2].docs.is_empty());

// event
let event_variants = match meta_type::<pallet::Event<Runtime>>().type_info().type_def {
scale_info::TypeDef::Variant(variants) => variants.variants,
_ => unreachable!(),
};

assert_eq!(event_variants[0].docs, vec!["event doc comment put in metadata"]);
assert!(event_variants[1].docs.is_empty());

// error
let error_variants = match meta_type::<pallet::Error<Runtime>>().type_info().type_def {
scale_info::TypeDef::Variant(variants) => variants.variants,
_ => unreachable!(),
};

assert_eq!(error_variants[0].docs, vec!["error doc comment put in metadata"]);
assert!(error_variants[1].docs.is_empty());

// storage is already covered in the main `fn metadata` test.
}

#[test]
fn metadata() {
use codec::Decode;
Expand Down
Loading