Skip to content

Commit ff06a4e

Browse files
pandres95gui1117ggwpez
committed
Fix: [Referenda Tracks] Resolve representation issues that are breaking PJS apps (paritytech#7671)
This Pull Request fixes the following issues present with PJS derived from paritytech#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 ![Screenshot 2025-02-22 at 12 31 28 pm](https://github.com/user-attachments/assets/a075b0b2-8984-43cc-9e72-99996de0ae1b) - 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`. ![Screenshot 2025-02-22 at 12 31 18 pm](https://github.com/user-attachments/assets/361a4f10-47b7-496d-9591-bc37afcf5ee1) --------- Co-authored-by: Guillaume Thiolliere <[email protected]> Co-authored-by: Oliver Tale-Yazdi <[email protected]>
1 parent 70e7033 commit ff06a4e

3 files changed

Lines changed: 121 additions & 11 deletions

File tree

prdoc/pr_7671.prdoc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
title: 'Fix: [Referenda Tracks] Resolve representation issues that are breaking PJS apps'
2+
3+
doc:
4+
- audience: Runtime Dev
5+
description: |-
6+
The PR #2072 introduces a change in the representation of the `name` field, from a `&str` to a `[u8; N]` array. This is because
7+
tracks can be retrieves from storage, and thus, a static string representation doesn't meet with the storage traits requirements.
8+
9+
This PR encapsulates this array into a `StringLike` structure that allows representing the value as a `str` for SCALE and metadata
10+
purposes. This is to avoid breaking changes.
11+
12+
This PR also reverts the representation of the `Tracks` constant as a tuple of `(TrackId, TrackInfo)` to accomplish the same
13+
purpose of avoid breaking changes to runtime users and clients.
14+
crates:
15+
- name: pallet-referenda
16+
bump: minor
17+
- name: collectives-westend-runtime
18+
bump: minor
19+
- name: kitchensink-runtime
20+
bump: minor
21+
- name: rococo-runtime
22+
bump: minor
23+
- name: westend-runtime
24+
bump: minor

substrate/frame/referenda/src/lib.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,10 @@ use self::branch::{BeginDecidingBranch, OneFewerDecidingBranch, ServiceBranch};
9898
pub use self::{
9999
pallet::*,
100100
types::{
101-
BalanceOf, BoundedCallOf, CallOf, Curve, DecidingStatus, DecidingStatusOf, Deposit,
102-
InsertSorted, NegativeImbalanceOf, PalletsOriginOf, ReferendumIndex, ReferendumInfo,
103-
ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, ScheduleAddressOf, TallyOf, Track,
104-
TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf,
101+
BalanceOf, BoundedCallOf, CallOf, ConstTrackInfo, Curve, DecidingStatus, DecidingStatusOf,
102+
Deposit, InsertSorted, NegativeImbalanceOf, PalletsOriginOf, ReferendumIndex,
103+
ReferendumInfo, ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, ScheduleAddressOf,
104+
StringLike, TallyOf, Track, TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf,
105105
},
106106
weights::WeightInfo,
107107
};
@@ -216,9 +216,30 @@ pub mod pallet {
216216

217217
#[pallet::extra_constants]
218218
impl<T: Config<I>, I: 'static> Pallet<T, I> {
219+
/// A list of tracks.
220+
///
221+
/// Note: if the tracks are dynamic, the value in the static metadata might be inaccurate.
219222
#[pallet::constant_name(Tracks)]
220-
fn tracks() -> Vec<Track<TrackIdOf<T, I>, BalanceOf<T, I>, BlockNumberFor<T>>> {
221-
T::Tracks::tracks().map(|t| t.into_owned()).collect()
223+
fn tracks() -> Vec<(TrackIdOf<T, I>, ConstTrackInfo<BalanceOf<T, I>, BlockNumberFor<T>>)> {
224+
T::Tracks::tracks()
225+
.map(|t| t.into_owned())
226+
.map(|Track { id, info }| {
227+
(
228+
id,
229+
ConstTrackInfo {
230+
name: StringLike(info.name),
231+
max_deciding: info.max_deciding,
232+
decision_deposit: info.decision_deposit,
233+
prepare_period: info.prepare_period,
234+
decision_period: info.decision_period,
235+
confirm_period: info.confirm_period,
236+
min_enactment_period: info.min_enactment_period,
237+
min_approval: info.min_approval,
238+
min_support: info.min_support,
239+
},
240+
)
241+
})
242+
.collect()
222243
}
223244
}
224245

substrate/frame/referenda/src/types.rs

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@
1919
2020
use super::*;
2121
use alloc::borrow::Cow;
22-
use codec::{Decode, Encode, EncodeLike, MaxEncodedLen};
22+
use codec::{Compact, Decode, Encode, EncodeLike, Input, MaxEncodedLen};
2323
use core::fmt::Debug;
2424
use frame_support::{
2525
traits::{schedule::v3::Anon, Bounded},
2626
Parameter,
2727
};
28-
use scale_info::TypeInfo;
28+
use scale_info::{Type, TypeInfo};
2929
use sp_arithmetic::{Rounding::*, SignedRounding::*};
3030
use sp_runtime::{FixedI64, PerThing, RuntimeDebug};
3131

@@ -114,11 +114,59 @@ pub struct Deposit<AccountId, Balance> {
114114

115115
pub const DEFAULT_MAX_TRACK_NAME_LEN: usize = 25;
116116

117+
/// Helper structure to treat a `[u8; N]` array as a string.
118+
///
119+
/// This is a temporary fix (see [#7671](https://github.com/paritytech/polkadot-sdk/pull/7671)) in
120+
/// order to stop `polkadot.js` apps to fail when trying to decode the `name` field in `TrackInfo`.
121+
#[derive(Clone, Eq, Decode, PartialEq, Debug)]
122+
pub struct StringLike<const N: usize>(pub [u8; N]);
123+
124+
impl<const N: usize> TypeInfo for StringLike<N> {
125+
type Identity = <&'static str as TypeInfo>::Identity;
126+
127+
fn type_info() -> Type {
128+
<&str as TypeInfo>::type_info()
129+
}
130+
}
131+
132+
impl<const N: usize> MaxEncodedLen for StringLike<N> {
133+
fn max_encoded_len() -> usize {
134+
<Compact<u32> as MaxEncodedLen>::max_encoded_len().saturating_add(N)
135+
}
136+
}
137+
138+
impl<const N: usize> Encode for StringLike<N> {
139+
fn encode(&self) -> Vec<u8> {
140+
use codec::Compact;
141+
(Compact(N as u32), self.0).encode()
142+
}
143+
}
144+
145+
impl<const N: usize> Decode for StringLike<N> {
146+
fn decode<I: Input>(input: &mut I) -> Result<Self, codec::Error> {
147+
let Compact(size): Compact<u32> = Decode::decode(input)?;
148+
if size != N as u32 {
149+
return Err("Invalid size".into());
150+
}
151+
152+
let bytes: [u8; N] = Decode::decode(input)?;
153+
Ok(Self(bytes))
154+
}
155+
}
156+
157+
/// Detailed information about the configuration of a referenda track. Used for internal storage.
158+
pub type TrackInfo<Balance, Moment, const N: usize = DEFAULT_MAX_TRACK_NAME_LEN> =
159+
TrackDetails<Balance, Moment, [u8; N]>;
160+
161+
/// Detailed information about the configuration of a referenda track. Used for const querying.
162+
pub type ConstTrackInfo<Balance, Moment, const N: usize = DEFAULT_MAX_TRACK_NAME_LEN> =
163+
TrackDetails<Balance, Moment, StringLike<N>>;
164+
117165
/// Detailed information about the configuration of a referenda track
118166
#[derive(Clone, Encode, Decode, MaxEncodedLen, TypeInfo, Eq, PartialEq, Debug)]
119-
pub struct TrackInfo<Balance, Moment, const N: usize = DEFAULT_MAX_TRACK_NAME_LEN> {
167+
pub struct TrackDetails<Balance, Moment, Name> {
120168
/// Name of this track.
121-
pub name: [u8; N],
169+
pub name: Name,
122170
/// A limit for the number of referenda on this track that can be being decided at once.
123171
/// For Root origin this should generally be just one.
124172
pub max_deciding: u32,
@@ -287,7 +335,8 @@ impl<
287335
Tally: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
288336
AccountId: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
289337
ScheduleAddress: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
290-
> ReferendumInfo<TrackId, RuntimeOrigin, Moment, Call, Balance, Tally, AccountId, ScheduleAddress>
338+
>
339+
ReferendumInfo<TrackId, RuntimeOrigin, Moment, Call, Balance, Tally, AccountId, ScheduleAddress>
291340
{
292341
/// Take the Decision Deposit from `self`, if there is one. Returns an `Err` if `self` is not
293342
/// in a valid state for the Decision Deposit to be refunded.
@@ -786,4 +835,20 @@ mod tests {
786835
Err("The tracks that were returned by `tracks` were not sorted by `Id`")
787836
);
788837
}
838+
839+
#[test]
840+
fn encoding_and_decoding_of_string_like_structure_works() {
841+
let string_like = StringLike::<13>(*b"hello, world!");
842+
let encoded: Vec<u8> = string_like.encode();
843+
844+
let decoded_as_vec: Vec<u8> =
845+
Decode::decode(&mut &encoded.clone()[..]).expect("decoding as Vec<u8> should work");
846+
assert_eq!(decoded_as_vec.len(), 13);
847+
let decoded_as_str: alloc::string::String =
848+
Decode::decode(&mut &encoded.clone()[..]).expect("decoding as str should work");
849+
assert_eq!(decoded_as_str.len(), 13);
850+
let decoded_as_string_like: StringLike<13> =
851+
Decode::decode(&mut &encoded.clone()[..]).expect("decoding as StringLike should work");
852+
assert_eq!(decoded_as_string_like.0.len(), 13);
853+
}
789854
}

0 commit comments

Comments
 (0)