From 467d1f642c8d5d6b11c45e5212f546af86b6f6e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= <hola@pablodorado.com> Date: Fri, 21 Mar 2025 07:14:59 -0500 Subject: [PATCH] Fix: [Referenda Tracks] Resolve representation issues that are breaking PJS apps (#7671) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This Pull Request fixes the following issues present with PJS derived from #2072 - Represents `Tracks` constant as a `Vec<(TrackId, TrackInfo)>` instead of `Vec<Track>`, so tracks iteration in PJS apps _Governance_ hook remains as how it was before, and  - Encapsulates `[u8; N]` from `name` field in a `StringLike` structure that represents its `TypeInfo` as a `&str` and encodes as such. This fixes errors present in PJS apps that treat `name` as a javascript `string`.  --------- Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> --- prdoc/pr_7671.prdoc | 24 +++++++++ substrate/frame/referenda/src/lib.rs | 35 ++++++++++--- substrate/frame/referenda/src/types.rs | 72 ++++++++++++++++++++++++-- 3 files changed, 121 insertions(+), 10 deletions(-) create mode 100644 prdoc/pr_7671.prdoc diff --git a/prdoc/pr_7671.prdoc b/prdoc/pr_7671.prdoc new file mode 100644 index 00000000000..09a1aef40bb --- /dev/null +++ b/prdoc/pr_7671.prdoc @@ -0,0 +1,24 @@ +title: 'Fix: [Referenda Tracks] Resolve representation issues that are breaking PJS apps' + +doc: +- audience: Runtime Dev + description: |- + The PR #2072 introduces a change in the representation of the `name` field, from a `&str` to a `[u8; N]` array. This is because + tracks can be retrieves from storage, and thus, a static string representation doesn't meet with the storage traits requirements. + + This PR encapsulates this array into a `StringLike` structure that allows representing the value as a `str` for SCALE and metadata + purposes. This is to avoid breaking changes. + + This PR also reverts the representation of the `Tracks` constant as a tuple of `(TrackId, TrackInfo)` to accomplish the same + purpose of avoid breaking changes to runtime users and clients. +crates: +- name: pallet-referenda + bump: minor +- name: collectives-westend-runtime + bump: minor +- name: kitchensink-runtime + bump: minor +- name: rococo-runtime + bump: minor +- name: westend-runtime + bump: minor diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index b58baa341cf..6d67dc5eee0 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -97,10 +97,11 @@ use self::branch::{BeginDecidingBranch, OneFewerDecidingBranch, ServiceBranch}; pub use self::{ pallet::*, types::{ - BalanceOf, BlockNumberFor, BoundedCallOf, CallOf, Curve, DecidingStatus, DecidingStatusOf, - Deposit, InsertSorted, NegativeImbalanceOf, PalletsOriginOf, ReferendumIndex, - ReferendumInfo, ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, ScheduleAddressOf, - TallyOf, Track, TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf, + BalanceOf, BlockNumberFor, BoundedCallOf, CallOf, ConstTrackInfo, Curve, DecidingStatus, + DecidingStatusOf, Deposit, InsertSorted, NegativeImbalanceOf, PalletsOriginOf, + ReferendumIndex, ReferendumInfo, ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, + ScheduleAddressOf, StringLike, TallyOf, Track, TrackIdOf, TrackInfo, TrackInfoOf, + TracksInfo, VotesOf, }, weights::WeightInfo, }; @@ -224,9 +225,31 @@ pub mod pallet { #[pallet::extra_constants] impl<T: Config<I>, I: 'static> Pallet<T, I> { + /// A list of tracks. + /// + /// Note: if the tracks are dynamic, the value in the static metadata might be inaccurate. #[pallet::constant_name(Tracks)] - fn tracks() -> Vec<Track<TrackIdOf<T, I>, BalanceOf<T, I>, BlockNumberFor<T, I>>> { - T::Tracks::tracks().map(|t| t.into_owned()).collect() + fn tracks() -> Vec<(TrackIdOf<T, I>, ConstTrackInfo<BalanceOf<T, I>, BlockNumberFor<T, I>>)> + { + T::Tracks::tracks() + .map(|t| t.into_owned()) + .map(|Track { id, info }| { + ( + id, + ConstTrackInfo { + name: StringLike(info.name), + max_deciding: info.max_deciding, + decision_deposit: info.decision_deposit, + prepare_period: info.prepare_period, + decision_period: info.decision_period, + confirm_period: info.confirm_period, + min_enactment_period: info.min_enactment_period, + min_approval: info.min_approval, + min_support: info.min_support, + }, + ) + }) + .collect() } } diff --git a/substrate/frame/referenda/src/types.rs b/substrate/frame/referenda/src/types.rs index 861b4605271..c810af341b1 100644 --- a/substrate/frame/referenda/src/types.rs +++ b/substrate/frame/referenda/src/types.rs @@ -19,13 +19,13 @@ use super::*; use alloc::borrow::Cow; -use codec::{Decode, DecodeWithMemTracking, Encode, EncodeLike, MaxEncodedLen}; +use codec::{Compact, Decode, DecodeWithMemTracking, Encode, EncodeLike, Input, MaxEncodedLen}; use core::fmt::Debug; use frame_support::{ traits::{schedule::v3::Anon, Bounded}, Parameter, }; -use scale_info::TypeInfo; +use scale_info::{Type, TypeInfo}; use sp_arithmetic::{Rounding::*, SignedRounding::*}; use sp_runtime::{FixedI64, PerThing, RuntimeDebug}; @@ -118,13 +118,61 @@ pub struct Deposit<AccountId, Balance> { pub const DEFAULT_MAX_TRACK_NAME_LEN: usize = 25; +/// Helper structure to treat a `[u8; N]` array as a string. +/// +/// This is a temporary fix (see [#7671](https://github.com/paritytech/polkadot-sdk/pull/7671)) in +/// order to stop `polkadot.js` apps to fail when trying to decode the `name` field in `TrackInfo`. +#[derive(Clone, Eq, DecodeWithMemTracking, PartialEq, Debug)] +pub struct StringLike<const N: usize>(pub [u8; N]); + +impl<const N: usize> TypeInfo for StringLike<N> { + type Identity = <&'static str as TypeInfo>::Identity; + + fn type_info() -> Type { + <&str as TypeInfo>::type_info() + } +} + +impl<const N: usize> MaxEncodedLen for StringLike<N> { + fn max_encoded_len() -> usize { + <Compact<u32> as MaxEncodedLen>::max_encoded_len().saturating_add(N) + } +} + +impl<const N: usize> Encode for StringLike<N> { + fn encode(&self) -> Vec<u8> { + use codec::Compact; + (Compact(N as u32), self.0).encode() + } +} + +impl<const N: usize> Decode for StringLike<N> { + fn decode<I: Input>(input: &mut I) -> Result<Self, codec::Error> { + let Compact(size): Compact<u32> = Decode::decode(input)?; + if size != N as u32 { + return Err("Invalid size".into()); + } + + let bytes: [u8; N] = Decode::decode(input)?; + Ok(Self(bytes)) + } +} + +/// Detailed information about the configuration of a referenda track. Used for internal storage. +pub type TrackInfo<Balance, Moment, const N: usize = DEFAULT_MAX_TRACK_NAME_LEN> = + TrackDetails<Balance, Moment, [u8; N]>; + +/// Detailed information about the configuration of a referenda track. Used for const querying. +pub type ConstTrackInfo<Balance, Moment, const N: usize = DEFAULT_MAX_TRACK_NAME_LEN> = + TrackDetails<Balance, Moment, StringLike<N>>; + /// Detailed information about the configuration of a referenda track #[derive( Clone, Encode, Decode, DecodeWithMemTracking, MaxEncodedLen, TypeInfo, Eq, PartialEq, Debug, )] -pub struct TrackInfo<Balance, Moment, const N: usize = DEFAULT_MAX_TRACK_NAME_LEN> { +pub struct TrackDetails<Balance, Moment, Name> { /// Name of this track. - pub name: [u8; N], + pub name: Name, /// A limit for the number of referenda on this track that can be being decided at once. /// For Root origin this should generally be just one. pub max_deciding: u32, @@ -795,4 +843,20 @@ mod tests { Err("The tracks that were returned by `tracks` were not sorted by `Id`") ); } + + #[test] + fn encoding_and_decoding_of_string_like_structure_works() { + let string_like = StringLike::<13>(*b"hello, world!"); + let encoded: Vec<u8> = string_like.encode(); + + let decoded_as_vec: Vec<u8> = + Decode::decode(&mut &encoded.clone()[..]).expect("decoding as Vec<u8> should work"); + assert_eq!(decoded_as_vec.len(), 13); + let decoded_as_str: alloc::string::String = + Decode::decode(&mut &encoded.clone()[..]).expect("decoding as str should work"); + assert_eq!(decoded_as_str.len(), 13); + let decoded_as_string_like: StringLike<13> = + Decode::decode(&mut &encoded.clone()[..]).expect("decoding as StringLike should work"); + assert_eq!(decoded_as_string_like.0.len(), 13); + } } -- GitLab