From 65db70c51669ea68dca5874bde4c141cf82932fa Mon Sep 17 00:00:00 2001 From: Nikolaus Heger Date: Wed, 2 Sep 2026 01:09:53 +0800 Subject: [PATCH 1/5] fix: decode remaining version-fragile reads against live metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A follow-up to #149, which fixed only the tech-referenda read path. Three more sites decoded chain state through the generated codegen, so they failed with Metadata(IncompatibleCodegen) on any runtime whose type graph differs from the one the CLI was built against. - Scheduler::Agenda (quantus scheduler agenda): a scheduled task carries origin: OriginCaller, the same field that broke tech-referenda. This is the queue referendum enactments land in, so it was unreadable exactly when an operator most needs it. The command also reported success after failing every block in the range; it now exits non-zero if any block could not be read. - The exercise suite read ReferendumInfoFor statically in three places, which is literally the bug #149 fixed. They now reuse fetch_referendum/ReferendumSnapshot. - Preimage::RequestStatusFor embeds the runtime-local PreimageDeposit ticket. It gated quantus preimage status/list and, more importantly, the tech-referenda submit path. The scale_value navigation helpers #149 added privately are now a shared cli::dynamic_decode module rather than being copied per call site. Display output is unchanged except Scheduler::Agenda, which moves from a Rust {:?} dump to the scale_value rendering — the same trade #149 already made for tech-referenda get. --- src/cli/dynamic_decode.rs | 92 +++++++++++++++ src/cli/exercise/scenarios/governance.rs | 14 +-- src/cli/exercise/scenarios/upgrade.rs | 10 +- src/cli/mod.rs | 1 + src/cli/preimage.rs | 82 +++++++++++-- src/cli/scheduler.rs | 33 ++++-- src/cli/tech_referenda.rs | 141 ++++++----------------- 7 files changed, 233 insertions(+), 140 deletions(-) create mode 100644 src/cli/dynamic_decode.rs diff --git a/src/cli/dynamic_decode.rs b/src/cli/dynamic_decode.rs new file mode 100644 index 0000000..2d232d8 --- /dev/null +++ b/src/cli/dynamic_decode.rs @@ -0,0 +1,92 @@ +//! Helpers for reading chain state that the generated codegen cannot decode. +//! +//! The types in [`crate::chain::quantus_subxt`] are generated from one runtime's +//! metadata. Any storage entry whose type graph reaches a runtime-composed type — +//! `OriginCaller`, `RuntimeCall`, a runtime-local struct — gets a different +//! validation hash on a runtime that composes those differently, and subxt then +//! refuses the read with `Metadata(IncompatibleCodegen)`. That is not a corruption +//! risk (subxt validates before decoding), but it makes the command unusable +//! against any runtime but the one the CLI shipped against. +//! +//! Reading such entries through [`subxt::dynamic`] decodes against the *live* +//! metadata instead. These helpers navigate the resulting [`Value`] by field and +//! variant *name*, so a command keeps working on any runtime that still calls the +//! fields what the pallet upstream calls them. + +use crate::error::QuantusError; +use subxt::ext::scale_value::{Composite, Primitive, Value, ValueDef}; + +/// A dynamically decoded value, as produced by `DecodedValueThunk::to_value`. +pub(crate) type Val = Value; + +/// The variant's name and its payload, or `None` if this is not an enum. +pub(crate) fn variant(v: &Val) -> Option<(&str, &Composite)> { + match &v.value { + ValueDef::Variant(var) => Some((var.name.as_str(), &var.values)), + _ => None, + } +} + +/// The struct/tuple body, or `None` if this is not a composite. +pub(crate) fn composite(v: &Val) -> Option<&Composite> { + match &v.value { + ValueDef::Composite(c) => Some(c), + _ => None, + } +} + +/// Look a field up by name. Unnamed composites have no named fields. +pub(crate) fn field<'a>(c: &'a Composite, name: &str) -> Option<&'a Val> { + match c { + Composite::Named(fields) => fields.iter().find(|(n, _)| n == name).map(|(_, v)| v), + Composite::Unnamed(_) => None, + } +} + +/// Positional access, for tuple variants such as `Ongoing(status)`. +pub(crate) fn nth(c: &Composite, i: usize) -> Option<&Val> { + match c { + Composite::Named(fields) => fields.get(i).map(|(_, v)| v), + Composite::Unnamed(fields) => fields.get(i), + } +} + +/// Any unsigned integer, transparently unwrapping single-field newtypes. +pub(crate) fn uint(v: &Val) -> Option { + match &v.value { + ValueDef::Primitive(Primitive::U128(n)) => Some(*n), + ValueDef::Composite(c) => nth(c, 0).and_then(uint), + _ => None, + } +} + +pub(crate) fn boolean(v: &Val) -> Option { + match &v.value { + ValueDef::Primitive(Primitive::Bool(b)) => Some(*b), + _ => None, + } +} + +/// `Some(Some(inner))` / `Some(None)`, or `None` if the value is not an `Option`. +pub(crate) fn option(v: &Val) -> Option> { + let (name, inner) = variant(v)?; + match name { + "None" => Some(None), + "Some" => Some(Some(nth(inner, 0)?)), + _ => None, + } +} + +/// A uniform error for a field the live runtime does not expose as expected. +pub(crate) fn missing(what: &str) -> QuantusError { + QuantusError::Generic(format!( + "decode: {what} is missing or has an unexpected shape; this runtime's pallet layout is \ + not recognized" + )) +} + +/// Read a named field as a `u32`. +pub(crate) fn u32_field(c: &Composite, name: &str) -> Result { + let raw = field(c, name).and_then(uint).ok_or_else(|| missing(name))?; + u32::try_from(raw).map_err(|_| missing(name)) +} diff --git a/src/cli/exercise/scenarios/governance.rs b/src/cli/exercise/scenarios/governance.rs index 859607c..fc58f31 100644 --- a/src/cli/exercise/scenarios/governance.rs +++ b/src/cli/exercise/scenarios/governance.rs @@ -88,20 +88,14 @@ async fn referendum_flow(ctx: &mut ExerciseCtx) -> Result { .await?; } - use quantus_subxt::api::runtime_types::pallet_referenda::types::ReferendumInfo; - let info_addr = quantus_subxt::api::storage().tech_referenda().referendum_info_for(index); + use crate::cli::tech_referenda::{fetch_referendum, ReferendumSnapshot}; let latest = ctx.client.get_latest_block().await?; - let info = ctx - .client - .client() - .storage() - .at(latest) - .fetch(&info_addr) + let info = fetch_referendum(&ctx.client, index, latest) .await? .ok_or_else(|| QuantusError::Generic(format!("referendum #{index} not found")))?; match info { - ReferendumInfo::Ongoing(status) => { + ReferendumSnapshot::Ongoing(status) => { if status.tally.ayes < 3 { return Err(QuantusError::Generic(format!( "expected 3 aye votes on referendum #{index}, tally shows {}", @@ -112,7 +106,7 @@ async fn referendum_flow(ctx: &mut ExerciseCtx) -> Result { "referendum #{index} submitted, deposit placed, 3 aye votes tallied (ongoing)" )) }, - ReferendumInfo::Approved(..) => Ok(format!( + ReferendumSnapshot::Approved(..) => Ok(format!( "referendum #{index} submitted, voted, and already approved (fast-governance node)" )), other => Err(QuantusError::Generic(format!( diff --git a/src/cli/exercise/scenarios/upgrade.rs b/src/cli/exercise/scenarios/upgrade.rs index 72c9eda..eb70012 100644 --- a/src/cli/exercise/scenarios/upgrade.rs +++ b/src/cli/exercise/scenarios/upgrade.rs @@ -154,12 +154,11 @@ async fn cast_collective_ayes(ctx: &ExerciseCtx, index: u32) -> Result<()> { } async fn referendum_is_approved(ctx: &ExerciseCtx, index: u32) -> Result { - use quantus_subxt::api::runtime_types::pallet_referenda::types::ReferendumInfo; - let info_addr = quantus_subxt::api::storage().tech_referenda().referendum_info_for(index); + use crate::cli::tech_referenda::{fetch_referendum, ReferendumSnapshot}; let latest = ctx.client.get_latest_block().await?; Ok(matches!( - ctx.client.client().storage().at(latest).fetch(&info_addr).await?, - Some(ReferendumInfo::Approved(..)) + fetch_referendum(&ctx.client, index, latest).await?, + Some(ReferendumSnapshot::Approved(..)) )) } @@ -178,9 +177,8 @@ async fn refund_deposits(ctx: &mut ExerciseCtx, index: u32) -> &'static str { } async fn referendum_state(ctx: &ExerciseCtx, index: u32) -> Result { - let info_addr = quantus_subxt::api::storage().tech_referenda().referendum_info_for(index); let latest = ctx.client.get_latest_block().await?; - let info = ctx.client.client().storage().at(latest).fetch(&info_addr).await?; + let info = crate::cli::tech_referenda::fetch_referendum(&ctx.client, index, latest).await?; Ok(format!("{info:?}")) } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index af2e922..5e1ade2 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -7,6 +7,7 @@ pub mod batch; pub mod block; pub mod cold_signing; pub mod common; +pub mod dynamic_decode; pub mod events; pub mod exercise; pub mod generic_call; diff --git a/src/cli/preimage.rs b/src/cli/preimage.rs index 549dceb..22164f6 100644 --- a/src/cli/preimage.rs +++ b/src/cli/preimage.rs @@ -66,6 +66,69 @@ pub enum PreimageCommands { }, } +/// The subset of `Preimage::RequestStatusFor` the CLI displays. +/// +/// Read dynamically: the stored ticket is a runtime-local deposit type, so the +/// generated codegen rejects this entry on any runtime that defines it differently. +#[derive(Clone, Copy, Debug)] +pub(crate) enum PreimageStatusSnapshot { + Unrequested { len: u32 }, + Requested { count: u32, maybe_len: Option }, +} + +impl PreimageStatusSnapshot { + /// Byte length of the stored preimage, when the chain knows it. + pub(crate) fn len(self) -> Option { + match self { + Self::Unrequested { len } => Some(len), + Self::Requested { maybe_len, .. } => maybe_len, + } + } +} + +/// Fetch `Preimage::RequestStatusFor(hash)` decoded against live metadata. +pub(crate) async fn fetch_request_status( + quantus_client: &crate::chain::client::QuantusClient, + hash: sp_core::H256, + block_hash: subxt::utils::H256, +) -> crate::error::Result> { + use crate::cli::dynamic_decode::{field, missing, option, u32_field, uint, variant}; + + let addr = subxt::dynamic::storage( + "Preimage", + "RequestStatusFor", + vec![subxt::dynamic::Value::from_bytes(hash.as_bytes())], + ); + let storage_at = quantus_client.client().storage().at(block_hash); + let Some(thunk) = storage_at.fetch(&addr).await? else { + return Ok(None); + }; + let value = thunk + .to_value() + .map_err(|e| QuantusError::Generic(format!("Failed to decode preimage status: {e:?}")))?; + + let (name, fields) = variant(&value).ok_or_else(|| missing("RequestStatus"))?; + let snapshot = match name { + "Unrequested" => PreimageStatusSnapshot::Unrequested { len: u32_field(fields, "len")? }, + "Requested" => { + let maybe_len = match field(fields, "maybe_len").and_then(option) { + Some(Some(v)) => Some( + u32::try_from(uint(v).ok_or_else(|| missing("maybe_len"))?) + .map_err(|_| missing("maybe_len"))?, + ), + Some(None) => None, + None => return Err(missing("maybe_len")), + }; + PreimageStatusSnapshot::Requested { count: u32_field(fields, "count")?, maybe_len } + }, + other => + return Err(QuantusError::Generic(format!( + "preimage decode: unknown RequestStatus variant `{other}`" + ))), + }; + Ok(Some(snapshot)) +} + /// Handle preimage commands pub async fn handle_preimage_command( command: PreimageCommands, @@ -122,10 +185,9 @@ async fn check_preimage_status( let status_addr = quantus_subxt::api::storage().preimage().status_for(preimage_hash); let status_result = storage_at.fetch(&status_addr).await; - // Check RequestStatusFor (new format) - let request_status_addr = - quantus_subxt::api::storage().preimage().request_status_for(preimage_hash); - let request_status_result = storage_at.fetch(&request_status_addr).await; + // Check RequestStatusFor (new format), decoded against live metadata + let request_status_result = + fetch_request_status(quantus_client, preimage_hash, latest_block_hash).await; log_print!("📊 Preimage Status Results:"); log_print!(" 🔗 Hash: {}", hash_str.bright_yellow()); @@ -257,21 +319,23 @@ async fn list_preimages( let len = u32::from_le_bytes([len_le[0], len_le[1], len_le[2], len_le[3]]); let hash = sp_core::H256::from_slice(&key[key.len() - 36..key.len() - 4]); - let status = storage_at - .fetch(&quantus_subxt::api::storage().preimage().request_status_for(hash)) + let status = fetch_request_status(quantus_client, hash, latest_block_hash) .await .ok() .flatten(); preimage_count += 1; match status { - Some(quantus_subxt::api::runtime_types::pallet_preimage::RequestStatus::Unrequested { ticket: _, len: status_len }) => { + Some(PreimageStatusSnapshot::Unrequested { len: status_len }) => { unrequested_count += 1; log_print!(" 🔗 {} (Unrequested, {} bytes)", hash, status_len); }, - Some(quantus_subxt::api::runtime_types::pallet_preimage::RequestStatus::Requested { maybe_ticket: _, count, maybe_len }) => { + Some(PreimageStatusSnapshot::Requested { count, maybe_len }) => { requested_count += 1; - let len_str = match maybe_len { Some(l) => format!("{} bytes", l), None => format!("{} bytes (from key)", len) }; + let len_str = match maybe_len { + Some(l) => format!("{} bytes", l), + None => format!("{} bytes (from key)", len), + }; log_print!(" 🔗 {} (Requested, count: {}, {})", hash, count, len_str); }, None => { diff --git a/src/cli/scheduler.rs b/src/cli/scheduler.rs index c128b4b..eaf1862 100644 --- a/src/cli/scheduler.rs +++ b/src/cli/scheduler.rs @@ -44,8 +44,6 @@ async fn list_agenda_range( quantus_client: &crate::chain::client::QuantusClient, range: &str, ) -> Result<()> { - use quantus_subxt::api; - // Parse range: from..to (inclusive) let parts: Vec<&str> = range.split("..").collect(); if parts.len() != 2 { @@ -69,25 +67,44 @@ async fn list_agenda_range( log_print!("🗓️ Scheduler::Agenda entries for blocks {}..={} (inclusive)", start, end); + // Decoded against live metadata rather than the generated types: a scheduled + // task carries `origin: OriginCaller`, whose variants change whenever a runtime + // gains or loses a custom-origin pallet, so the static address rejects any + // runtime but the one this CLI was built against. + let mut failures = 0u32; for bn in start..=end { - let addr = api::storage().scheduler().agenda( - quantus_subxt::api::runtime_types::qp_scheduler::BlockNumberOrTimestamp::BlockNumber( - bn, - ), + let addr = subxt::dynamic::storage( + "Scheduler", + "Agenda", + vec![subxt::dynamic::Value::unnamed_variant( + "BlockNumber", + [subxt::dynamic::Value::u128(u128::from(bn))], + )], ); match storage_at.fetch(&addr).await { - Ok(Some(agenda)) => { - log_print!("#{}: {:?}", bn, agenda); + Ok(Some(thunk)) => match thunk.to_value() { + Ok(agenda) => log_print!("#{}: {}", bn, agenda), + Err(e) => { + failures += 1; + log_print!("#{}: error decoding agenda: {:?}", bn, e); + }, }, Ok(None) => { log_print!("#{}: ", bn); }, Err(e) => { + failures += 1; log_print!("#{}: error fetching agenda: {:?}", bn, e); }, } } + if failures > 0 { + return Err(crate::error::QuantusError::Generic(format!( + "{failures} of {} blocks could not be read from Scheduler::Agenda", + end - start + 1 + ))); + } log_success!("Finished scanning Scheduler::Agenda"); Ok(()) } diff --git a/src/cli/tech_referenda.rs b/src/cli/tech_referenda.rs index 3dbae5c..275ae33 100644 --- a/src/cli/tech_referenda.rs +++ b/src/cli/tech_referenda.rs @@ -250,30 +250,18 @@ async fn submit_runtime_upgrade( let latest_block_hash = quantus_client.get_latest_block().await?; let storage_at = quantus_client.client().storage().at(latest_block_hash); - let preimage_status = storage_at - .fetch( - &quantus_subxt::api::storage() - .preimage() - .request_status_for(preimage_hash_parsed), - ) - .await - .map_err(|e| QuantusError::Generic(format!("Failed to fetch preimage status: {:?}", e)))? - .ok_or_else(|| QuantusError::Generic("Preimage not found on chain".to_string()))?; - - let preimage_len = match preimage_status { - quantus_subxt::api::runtime_types::pallet_preimage::RequestStatus::Unrequested { - ticket: _, - len, - } => len, - quantus_subxt::api::runtime_types::pallet_preimage::RequestStatus::Requested { - maybe_ticket: _, - count: _, - maybe_len, - } => match maybe_len { - Some(len) => len, - None => return Err(QuantusError::Generic("Preimage length not available".to_string())), - }, - }; + let preimage_status = crate::cli::preimage::fetch_request_status( + quantus_client, + preimage_hash_parsed, + latest_block_hash, + ) + .await + .map_err(|e| QuantusError::Generic(format!("Failed to fetch preimage status: {e:?}")))? + .ok_or_else(|| QuantusError::Generic("Preimage not found on chain".to_string()))?; + + let preimage_len = preimage_status + .len() + .ok_or_else(|| QuantusError::Generic("Preimage length not available".to_string()))?; let preimage = storage_at .fetch( @@ -506,29 +494,29 @@ async fn scheduled_enactment_block( /// Compute the enactment block for a given approval block, honoring the track's minimum delay /// When the referendum's approved call is scheduled to run. #[derive(Clone, Copy, Debug)] -enum EnactmentMoment { +pub(crate) enum EnactmentMoment { At(u32), After(u32), } #[derive(Clone, Copy, Debug)] -struct DecidingSnapshot { +pub(crate) struct DecidingSnapshot { since: u32, confirming: Option, } #[derive(Clone, Copy, Debug)] -struct TallySnapshot { - ayes: u32, - nays: u32, +pub(crate) struct TallySnapshot { + pub(crate) ayes: u32, + pub(crate) nays: u32, } /// The subset of `ReferendumStatus` this command displays. #[derive(Clone, Copy, Debug)] -struct OngoingSnapshot { - track: u16, - submitted: u32, - tally: TallySnapshot, +pub(crate) struct OngoingSnapshot { + pub(crate) track: u16, + pub(crate) submitted: u32, + pub(crate) tally: TallySnapshot, decision_deposit_placed: bool, in_queue: bool, deciding: Option, @@ -536,7 +524,7 @@ struct OngoingSnapshot { } #[derive(Clone, Copy, Debug)] -enum ReferendumSnapshot { +pub(crate) enum ReferendumSnapshot { Ongoing(OngoingSnapshot), Approved(u32), Rejected(u32), @@ -555,77 +543,14 @@ enum ReferendumSnapshot { /// that still calls the fields what upstream `pallet-referenda` calls them. mod referendum_value { use super::{ - DecidingSnapshot, EnactmentMoment, OngoingSnapshot, QuantusError, ReferendumSnapshot, + DecidingSnapshot, EnactmentMoment, OngoingSnapshot, ReferendumSnapshot, TallySnapshot, + }; + use crate::{ + cli::dynamic_decode::{ + composite, field, missing, nth, option, u32_field, uint, variant, Val, + }, + error::QuantusError, }; - use subxt::ext::scale_value::{Composite, Primitive, Value, ValueDef}; - - type Val = Value; - - fn variant(v: &Val) -> Option<(&str, &Composite)> { - match &v.value { - ValueDef::Variant(var) => Some((var.name.as_str(), &var.values)), - _ => None, - } - } - - fn composite(v: &Val) -> Option<&Composite> { - match &v.value { - ValueDef::Composite(c) => Some(c), - _ => None, - } - } - - fn field<'a>(c: &'a Composite, name: &str) -> Option<&'a Val> { - match c { - Composite::Named(fields) => fields.iter().find(|(n, _)| n == name).map(|(_, v)| v), - Composite::Unnamed(_) => None, - } - } - - fn nth(c: &Composite, i: usize) -> Option<&Val> { - match c { - Composite::Named(fields) => fields.get(i).map(|(_, v)| v), - Composite::Unnamed(fields) => fields.get(i), - } - } - - fn uint(v: &Val) -> Option { - match &v.value { - ValueDef::Primitive(Primitive::U128(n)) => Some(*n), - // A single-field newtype (e.g. a compact wrapper) decodes as a composite. - ValueDef::Composite(c) => nth(c, 0).and_then(uint), - _ => None, - } - } - - fn boolean(v: &Val) -> Option { - match &v.value { - ValueDef::Primitive(Primitive::Bool(b)) => Some(*b), - _ => None, - } - } - - /// `Some(inner)` / `None`, or `Err` if the value is not an Option at all. - fn option(v: &Val) -> Option> { - let (name, inner) = variant(v)?; - match name { - "None" => Some(None), - "Some" => Some(Some(nth(inner, 0)?)), - _ => None, - } - } - - fn missing(what: &str) -> QuantusError { - QuantusError::Generic(format!( - "referendum decode: {what} is missing or has an unexpected shape; the runtime's \ - pallet-referenda layout is not recognized" - )) - } - - fn u32_field(c: &Composite, name: &str) -> Result { - let raw = field(c, name).and_then(uint).ok_or_else(|| missing(name))?; - u32::try_from(raw).map_err(|_| missing(name)) - } pub fn decode(value: &Val) -> Result { let (name, body) = variant(value).ok_or_else(|| missing("ReferendumInfo"))?; @@ -677,7 +602,7 @@ mod referendum_value { Ok(ReferendumSnapshot::Ongoing(OngoingSnapshot { track, submitted: u32_field(status, "submitted")?, - tally: super::TallySnapshot { + tally: TallySnapshot { ayes: u32_field(tally, "ayes")?, nays: u32_field(tally, "nays")?, }, @@ -685,7 +610,9 @@ mod referendum_value { field(status, "decision_deposit").and_then(option), Some(Some(_)) ), - in_queue: field(status, "in_queue").and_then(boolean).unwrap_or(false), + in_queue: field(status, "in_queue") + .and_then(crate::cli::dynamic_decode::boolean) + .unwrap_or(false), deciding, enactment, })) @@ -714,7 +641,7 @@ fn referendum_info_address( } /// Fetch and decode one referendum, tolerating runtime versions the CLI was not built against. -async fn fetch_referendum( +pub(crate) async fn fetch_referendum( quantus_client: &crate::chain::client::QuantusClient, index: u32, block_hash: subxt::utils::H256, From 7eff85098f3a8c4a46c0e567017d67a098d04aba Mon Sep 17 00:00:00 2001 From: Nikolaus Heger Date: Wed, 2 Sep 2026 01:22:54 +0800 Subject: [PATCH 2/5] fix: render multisig proposal calls from live metadata decode_call_data resolved the pallet name from metadata but then matched hardcoded call indices (idx == 0 => transfer_allow_death, idx == 3 => transfer_keep_alive, ReversibleTransfers idx == 0) and hand-parsed arguments by byte offset. On a runtime that ordered calls differently it would render a confident, fully formatted description of a different call, with no error -- on the screen a multisig signer reads before approving. Every other decode bug in this PR fails loudly; this one did not. Look the pallet up by index, the call variant by index within that pallet, and decode each argument against the type id the metadata declares for it. Nothing about the call layout is hardcoded, so an unknown pallet or call index now says so instead of guessing, and trailing bytes are reported rather than ignored. Account ids still render as SS58 and balances with symbol and decimals, which is what a signer is actually checking. The pure rendering half is split out as describe_call so it can be tested against the checked-in metadata blob without a node. Four tests cover it, including one that derives an undefined call index from the metadata and asserts the output names no real call. --- src/cli/multisig.rs | 397 ++++++++++++++++++++++---------------------- 1 file changed, 199 insertions(+), 198 deletions(-) diff --git a/src/cli/multisig.rs b/src/cli/multisig.rs index cd99ed6..af4c88f 100644 --- a/src/cli/multisig.rs +++ b/src/cli/multisig.rs @@ -2315,227 +2315,162 @@ async fn handle_info( } /// Decode call data into human-readable format +/// Render a multisig proposal's stored call for a human about to approve it. +/// +/// Decoded entirely from the **live** metadata: the pallet is looked up by index, +/// the call variant by index within that pallet, and each argument against the +/// type id the metadata declares for it. Nothing about the call layout is +/// hardcoded here. +/// +/// The previous implementation resolved the pallet name from metadata but then +/// matched hardcoded call indices (`idx == 0 => "transfer_allow_death"`) and +/// hand-parsed arguments by byte offset. A runtime that reordered calls within a +/// pallet would make it render a confident, fully-formatted description of a +/// *different* call, with no error — on the screen a signer reads before +/// approving. Unknown calls now say so instead of guessing. async fn decode_call_data( quantus_client: &crate::chain::client::QuantusClient, call_data: &[u8], ) -> crate::error::Result { - use codec::Decode; + // Symbol/decimals only affect how balances are rendered; a lookup failure must + // not hide the call itself, so fall back to the raw integer. + let symbol_decimals = crate::cli::send::get_chain_properties(quantus_client).await.ok(); + Ok(describe_call(&quantus_client.client().metadata(), call_data, symbol_decimals.as_ref())) +} +/// Pure rendering half of [`decode_call_data`], so it can be tested against a +/// metadata blob without a live node. +fn describe_call( + metadata: &subxt::Metadata, + call_data: &[u8], + symbol_decimals: Option<&(String, u8)>, +) -> String { if call_data.len() < 2 { - return Ok(format!(" {} {} bytes (too short)", "Call Size:".dimmed(), call_data.len())); + return format!(" {} {} bytes (too short)", "Call Size:".dimmed(), call_data.len()); } let pallet_index = call_data[0]; let call_index = call_data[1]; - let args = &call_data[2..]; - - // Get metadata to find pallet and call names - let metadata = quantus_client.client().metadata(); - // Try to find pallet by index - let pallet_name = metadata - .pallets() - .find(|p| p.index() == pallet_index) - .map(|p| p.name()) - .unwrap_or("Unknown"); - - // Try to decode based on known patterns - match (pallet_index, call_index) { - // Balances pallet transfers - // transfer_allow_death (0) or transfer_keep_alive (3) - (_, idx) if pallet_name == "Balances" && (idx == 0 || idx == 3) => { - let call_name = match idx { - 0 => "transfer_allow_death", - 3 => "transfer_keep_alive", - _ => unreachable!(), - }; + let Some(pallet) = metadata.pallet_by_index(pallet_index) else { + return format!( + " {} unknown pallet index {}\n {} {} bytes", + "Call:".dimmed(), + pallet_index, + "Args:".dimmed(), + call_data.len() - 2 + ); + }; + let Some(variant) = pallet.call_variant_by_index(call_index) else { + return format!( + " {} {}::\n {} {} bytes", + "Call:".dimmed(), + pallet.name().bright_cyan(), + call_index, + "Args:".dimmed(), + call_data.len() - 2 + ); + }; - if args.len() < 33 { - return Ok(format!( - " {} {}::{} (index {})\n {} {} bytes (too short)", - "Call:".dimmed(), - pallet_name.bright_cyan(), - call_name.bright_yellow(), - idx, - "Args:".dimmed(), - args.len() - )); - } + let mut out = format!( + " {} {}::{}", + "Call:".dimmed(), + pallet.name().bright_cyan(), + variant.name.bright_yellow() + ); - // Decode MultiAddress::Id (first byte is variant, 0x00 = Id) - // Then 32 bytes for AccountId32 - let address_variant = args[0]; - if address_variant != 0 { - return Ok(format!( - " {} {}::{} (index {})\n {} {} bytes\n {} Unknown address variant: {}", - "Call:".dimmed(), - pallet_name.bright_cyan(), - call_name.bright_yellow(), - idx, - "Args:".dimmed(), - args.len(), + let mut cursor = &call_data[2..]; + for field in &variant.fields { + let name = field.name.clone().unwrap_or_else(|| "arg".to_string()); + match subxt::ext::scale_value::scale::decode_as_type( + &mut cursor, + field.ty.id, + metadata.types(), + ) { + Ok(value) => { + let rendered = render_call_arg(&value, &name, symbol_decimals); + out.push_str(&format!("\n {} {}", format!("{name}:").dimmed(), rendered)); + }, + Err(e) => { + out.push_str(&format!( + "\n {} failed to decode `{}`: {}", "Error:".dimmed(), - address_variant + name, + e )); - } - - let account_bytes: [u8; 32] = args[1..33].try_into().map_err(|_| { - crate::error::QuantusError::Generic("Failed to extract account bytes".to_string()) - })?; - let account_id = SpAccountId32::from(account_bytes); - let to_address = account_id.to_ss58check(); - - // Decode amount (Compact) - let mut cursor = &args[33..]; - let amount: u128 = match codec::Compact::::decode(&mut cursor) { - Ok(compact) => compact.0, - Err(_) => { - return Ok(format!( - " {} {}::{} (index {})\n {} {}\n {} Failed to decode amount", - "Call:".dimmed(), - pallet_name.bright_cyan(), - call_name.bright_yellow(), - idx, - "To:".dimmed(), - to_address.bright_cyan(), - "Error:".dimmed() - )); - }, - }; + return out; + }, + } + } - Ok(format!( - " {} {}::{}\n {} {}\n {} {}", - "Call:".dimmed(), - pallet_name.bright_cyan(), - call_name.bright_yellow(), - "To:".dimmed(), - to_address.bright_cyan(), - "Amount:".dimmed(), - format_balance(amount).bright_green() - )) - }, - // ReversibleTransfers::set_high_security - (_, idx) if pallet_name == "ReversibleTransfers" && idx == 0 => { - // set_high_security has: delay (enum), interceptor (AccountId32) - if args.is_empty() { - return Ok(format!( - " {} {}::set_high_security\n {} {} bytes (too short)", - "Call:".dimmed(), - pallet_name.bright_cyan(), - "Args:".dimmed(), - args.len() - )); - } + if !cursor.is_empty() { + out.push_str(&format!( + "\n {} {} trailing byte(s) after the decoded arguments", + "Warning:".dimmed(), + cursor.len() + )); + } - // Decode delay (BlockNumberOrTimestamp enum) - let delay_variant = args[0]; - let delay_str: String; - let offset: usize; - - match delay_variant { - 0 => { - // BlockNumber(u32) - if args.len() < 5 { - return Ok(format!( - " {} {}::set_high_security\n {} Failed to decode delay (BlockNumber)", - "Call:".dimmed(), - pallet_name.bright_cyan(), - "Error:".dimmed() - )); - } - let blocks = u32::from_le_bytes([args[1], args[2], args[3], args[4]]); - delay_str = format!("{} blocks", blocks); - offset = 5; - }, - 1 => { - // Timestamp(u64) - if args.len() < 9 { - return Ok(format!( - " {} {}::set_high_security\n {} Failed to decode delay (Timestamp)", - "Call:".dimmed(), - pallet_name.bright_cyan(), - "Error:".dimmed() - )); - } - let millis = u64::from_le_bytes([ - args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8], - ]); - let seconds = millis / 1000; - delay_str = format!("{} seconds ({} ms)", seconds, millis); - offset = 9; - }, - _ => { - return Ok(format!( - " {} {}::set_high_security\n {} Unknown delay variant: {}", - "Call:".dimmed(), - pallet_name.bright_cyan(), - "Error:".dimmed(), - delay_variant - )); - }, - } + out +} - // Decode interceptor (AccountId32) - if args.len() < offset + 32 { - return Ok(format!( - " {} {}::set_high_security\n {} {}\n {} Failed to decode interceptor", - "Call:".dimmed(), - pallet_name.bright_cyan(), - "Delay:".dimmed(), - delay_str.bright_yellow(), - "Error:".dimmed() - )); - } +/// Best-effort human rendering of one decoded call argument. +/// +/// Falls back to `scale_value`'s own formatting; the special cases only make +/// account ids and balances readable, which is what a signer is checking. +fn render_call_arg( + value: &subxt::ext::scale_value::Value, + field_name: &str, + symbol_decimals: Option<&(String, u8)>, +) -> String { + if let Some(bytes) = account_bytes(value) { + return SpAccountId32::from(bytes).to_ss58check().bright_cyan().to_string(); + } + let looks_like_balance = + matches!(field_name, "value" | "amount" | "new_free" | "fee" | "deposit"); + if looks_like_balance { + if let (Some((symbol, decimals)), Some(n)) = + (symbol_decimals, crate::cli::dynamic_decode::uint(value)) + { + let formatted = crate::cli::send::format_balance(n, *decimals); + return format!("{formatted} {symbol}").bright_green().to_string(); + } + } + value.to_string() +} - let interceptor_bytes: [u8; 32] = - args[offset..offset + 32].try_into().map_err(|_| { - crate::error::QuantusError::Generic( - "Failed to extract interceptor bytes".to_string(), - ) - })?; - let interceptor = SpAccountId32::from(interceptor_bytes); - let interceptor_ss58 = interceptor - .to_ss58check_with_version(sp_core::crypto::Ss58AddressFormat::custom(189)); +/// Extract a 32-byte account id from `AccountId32`, `MultiAddress::Id`, or a bare +/// 32-byte sequence, so signers see an address rather than a byte array. +fn account_bytes(value: &subxt::ext::scale_value::Value) -> Option<[u8; 32]> { + use crate::cli::dynamic_decode::{composite, nth, uint, variant}; + use subxt::ext::scale_value::{Composite, ValueDef}; - Ok(format!( - " {} {}::set_high_security\n {} {}\n {} {}", - "Call:".dimmed(), - pallet_name.bright_cyan(), - "Delay:".dimmed(), - delay_str.bright_yellow(), - "Guardian:".dimmed(), - interceptor_ss58.bright_green() - )) - }, - _ => { - // Try to get call name from metadata - let call_name = metadata - .pallets() - .find(|p| p.index() == pallet_index) - .and_then(|p| { - p.call_variants().and_then(|calls| { - calls.iter().find(|v| v.index == call_index).map(|v| v.name.as_str()) - }) - }) - .unwrap_or("unknown"); - - Ok(format!( - " {} {}::{} (index {}:{})\n {} {} bytes\n {} {}", - "Call:".dimmed(), - pallet_name.bright_cyan(), - call_name.bright_yellow(), - pallet_index, - call_index, - "Args:".dimmed(), - args.len(), - "Raw:".dimmed(), - hex::encode(args).bright_green() - )) - }, + if let Some(("Id", inner)) = variant(value) { + return nth(inner, 0).and_then(account_bytes); } + let fields: &Composite = composite(value)?; + // AccountId32 is a newtype around [u8; 32]. + if let Some(inner) = nth(fields, 0) { + if matches!(inner.value, ValueDef::Composite(_)) { + if let Some(bytes) = account_bytes(inner) { + return Some(bytes); + } + } + } + let len = match fields { + Composite::Named(f) => f.len(), + Composite::Unnamed(f) => f.len(), + }; + if len != 32 { + return None; + } + let mut out = [0u8; 32]; + for (i, slot) in out.iter_mut().enumerate() { + *slot = u8::try_from(uint(nth(fields, i)?)?).ok()?; + } + Some(out) } -/// Query proposal information async fn handle_proposal_info( multisig_address: String, proposal_id: u32, @@ -3340,6 +3275,72 @@ mod execute_call_tests { assert!(build_propose_tx(address, transfer_call().encode(), 5000).is_ok()); } + fn test_metadata() -> subxt::Metadata { + use codec::Decode; + let bytes: &[u8] = include_bytes!("../quantus_metadata.scale"); + subxt::Metadata::decode(&mut &bytes[..]).expect("valid checked-in metadata") + } + + /// Indices come from the metadata, never from a hardcoded table. + fn call_indices(md: &subxt::Metadata, pallet: &str, call: &str) -> (u8, u8) { + let p = md.pallets().find(|p| p.name() == pallet).expect("pallet present"); + let v = p.call_variants().expect("pallet has calls"); + let c = v.iter().find(|v| v.name == call).expect("call present"); + (p.index(), c.index) + } + + #[test] + fn describe_call_names_the_call_the_metadata_names() { + let md = test_metadata(); + let (pallet_idx, call_idx) = call_indices(&md, "Balances", "transfer_allow_death"); + + let mut bytes = vec![pallet_idx, call_idx]; + bytes.push(0x00); // MultiAddress::Id + bytes.extend_from_slice(&[7u8; 32]); + bytes.extend_from_slice(&codec::Compact(1_000_000_000_000u128).encode()); + + let out = describe_call(&md, &bytes, Some(&("QUAN".to_string(), 12))); + assert!(out.contains("transfer_allow_death"), "{out}"); + assert!(!out.contains("trailing byte"), "{out}"); + } + + /// The regression this decoder exists for: a call index the runtime does not + /// define must never be rendered as some other call. The old implementation + /// matched hardcoded indices and would confidently mislabel it. + #[test] + fn describe_call_refuses_to_guess_an_unknown_call_index() { + let md = test_metadata(); + let balances = md.pallets().find(|p| p.name() == "Balances").expect("Balances present"); + let defined: Vec = + balances.call_variants().expect("has calls").iter().map(|v| v.index).collect(); + let unknown = (0u8..=255).find(|i| !defined.contains(i)).expect("some index is free"); + + let out = describe_call(&md, &[balances.index(), unknown, 0, 0], None); + assert!(out.contains("unknown call index"), "{out}"); + assert!(!out.contains("transfer"), "must not name any real call: {out}"); + } + + #[test] + fn describe_call_reports_an_unknown_pallet_index() { + let md = test_metadata(); + let used: Vec = md.pallets().map(|p| p.index()).collect(); + let unknown = (0u8..=255).find(|i| !used.contains(i)).expect("some index is free"); + let out = describe_call(&md, &[unknown, 0], None); + assert!(out.contains("unknown pallet index"), "{out}"); + } + + #[test] + fn describe_call_flags_trailing_bytes() { + let md = test_metadata(); + let (pallet_idx, call_idx) = call_indices(&md, "Balances", "transfer_allow_death"); + let mut bytes = vec![pallet_idx, call_idx, 0x00]; + bytes.extend_from_slice(&[7u8; 32]); + bytes.extend_from_slice(&codec::Compact(1u128).encode()); + bytes.push(0xAA); + let out = describe_call(&md, &bytes, None); + assert!(out.contains("trailing byte"), "{out}"); + } + #[test] fn decode_proposal_call_rejects_undecodable_bytes() { assert!(decode_proposal_call(&[0xff, 0xff]).is_err()); From 81c5c285c9e385ca1af1a21953e6dfe90b7112c6 Mon Sep 17 00:00:00 2001 From: Nikolaus Heger Date: Wed, 2 Sep 2026 09:43:07 +0800 Subject: [PATCH 3/5] fix: gate SS58 rendering on the declared type, not byte length Review findings on #150. P1: render_call_arg tried account_bytes first, and account_bytes accepted any bare 32-byte sequence. So a non-account argument -- System::authorize_upgrade's code_hash: H256, or a 32-byte remark payload -- rendered as an SS58 address. That is the same class of signer-visible type confusion this decoder exists to prevent: a signer could not compare a proposed runtime hash against the authorized artifact. SS58 is now used only where the metadata resolves the argument's type to AccountId32, directly or inside MultiAddress::Id. Every other byte blob renders as lossless 0x hex. Two regressions cover both directions. P2: exercise/scenarios/preimage.rs still fetched Preimage::RequestStatusFor through the generated address in note_and_verify and preimage_status_exists, so the preimage scenario still broke on exactly the runtimes this PR targets. Both now use fetch_request_status; no static reads of that entry remain. --- src/cli/exercise/scenarios/preimage.rs | 9 +- src/cli/multisig.rs | 115 +++++++++++++++++++------ 2 files changed, 95 insertions(+), 29 deletions(-) diff --git a/src/cli/exercise/scenarios/preimage.rs b/src/cli/exercise/scenarios/preimage.rs index 7ff544a..825f819 100644 --- a/src/cli/exercise/scenarios/preimage.rs +++ b/src/cli/exercise/scenarios/preimage.rs @@ -42,9 +42,9 @@ async fn note_and_verify(ctx: &mut ExerciseCtx) -> Result { ) .await?; - let status_addr = quantus_subxt::api::storage().preimage().request_status_for(expected_hash); let latest = ctx.client.get_latest_block().await?; - let status = ctx.client.client().storage().at(latest).fetch(&status_addr).await?; + let status = + crate::cli::preimage::fetch_request_status(&ctx.client, expected_hash, latest).await?; match status { Some(_) => Ok(format!("preimage {expected_hash:?} noted and visible in RequestStatusFor")), None => Err(QuantusError::Generic(format!( @@ -54,9 +54,10 @@ async fn note_and_verify(ctx: &mut ExerciseCtx) -> Result { } async fn preimage_status_exists(ctx: &ExerciseCtx, hash: sp_core::H256) -> Result { - let status_addr = quantus_subxt::api::storage().preimage().request_status_for(hash); let latest = ctx.client.get_latest_block().await?; - Ok(ctx.client.client().storage().at(latest).fetch(&status_addr).await?.is_some()) + Ok(crate::cli::preimage::fetch_request_status(&ctx.client, hash, latest) + .await? + .is_some()) } /// Note a preimage, then clear it with `unnote_preimage` and verify the diff --git a/src/cli/multisig.rs b/src/cli/multisig.rs index af4c88f..81711f6 100644 --- a/src/cli/multisig.rs +++ b/src/cli/multisig.rs @@ -2388,7 +2388,7 @@ fn describe_call( metadata.types(), ) { Ok(value) => { - let rendered = render_call_arg(&value, &name, symbol_decimals); + let rendered = render_call_arg(&value, &name, symbol_decimals, metadata); out.push_str(&format!("\n {} {}", format!("{name}:").dimmed(), rendered)); }, Err(e) => { @@ -2414,18 +2414,25 @@ fn describe_call( out } -/// Best-effort human rendering of one decoded call argument. +/// Human rendering of one decoded call argument. /// -/// Falls back to `scale_value`'s own formatting; the special cases only make -/// account ids and balances readable, which is what a signer is checking. +/// SS58 is used **only** when the metadata declares the argument as `AccountId32`. +/// Keying it on "32 bytes" instead would print an `H256` — an `authorize_upgrade` +/// code hash, say — as an address, which is exactly the signer-visible type +/// confusion this decoder exists to prevent. Every other byte blob renders as +/// lossless hex so it can be compared against an artifact. fn render_call_arg( value: &subxt::ext::scale_value::Value, field_name: &str, symbol_decimals: Option<&(String, u8)>, + metadata: &subxt::Metadata, ) -> String { - if let Some(bytes) = account_bytes(value) { + if let Some(bytes) = account_bytes(value, metadata) { return SpAccountId32::from(bytes).to_ss58check().bright_cyan().to_string(); } + if let Some(bytes) = byte_blob(value) { + return format!("0x{}", hex::encode(bytes)); + } let looks_like_balance = matches!(field_name, "value" | "amount" | "new_free" | "fee" | "deposit"); if looks_like_balance { @@ -2439,38 +2446,63 @@ fn render_call_arg( value.to_string() } -/// Extract a 32-byte account id from `AccountId32`, `MultiAddress::Id`, or a bare -/// 32-byte sequence, so signers see an address rather than a byte array. -fn account_bytes(value: &subxt::ext::scale_value::Value) -> Option<[u8; 32]> { - use crate::cli::dynamic_decode::{composite, nth, uint, variant}; - use subxt::ext::scale_value::{Composite, ValueDef}; +/// True when the metadata names `id`'s type `name` (last path segment). +fn type_is_named(metadata: &subxt::Metadata, id: u32, name: &str) -> bool { + metadata + .types() + .resolve(id) + .and_then(|t| t.path.segments.last()) + .map(|segment| segment == name) + .unwrap_or(false) +} + +/// A flat `Vec` / `[u8; N]` payload, for lossless hex rendering. +fn byte_blob(value: &subxt::ext::scale_value::Value) -> Option> { + use crate::cli::dynamic_decode::{composite, nth, uint}; + use subxt::ext::scale_value::Composite; - if let Some(("Id", inner)) = variant(value) { - return nth(inner, 0).and_then(account_bytes); - } let fields: &Composite = composite(value)?; - // AccountId32 is a newtype around [u8; 32]. - if let Some(inner) = nth(fields, 0) { - if matches!(inner.value, ValueDef::Composite(_)) { - if let Some(bytes) = account_bytes(inner) { - return Some(bytes); - } - } - } let len = match fields { Composite::Named(f) => f.len(), Composite::Unnamed(f) => f.len(), }; - if len != 32 { + // A newtype wrapper (H256(pub [u8; 32])) nests one level. + if len == 1 { + if let Some(inner) = nth(fields, 0) { + if let Some(bytes) = byte_blob(inner) { + return Some(bytes); + } + } + } + if len == 0 { return None; } - let mut out = [0u8; 32]; - for (i, slot) in out.iter_mut().enumerate() { - *slot = u8::try_from(uint(nth(fields, i)?)?).ok()?; + let mut out = Vec::with_capacity(len); + for i in 0..len { + out.push(u8::try_from(uint(nth(fields, i)?)?).ok()?); } Some(out) } +/// A 32-byte account id, but **only** where the metadata says `AccountId32` — +/// directly, or inside a `MultiAddress::Id`. Any other 32-byte value is not an +/// address and must not be shown as one. +fn account_bytes( + value: &subxt::ext::scale_value::Value, + metadata: &subxt::Metadata, +) -> Option<[u8; 32]> { + use crate::cli::dynamic_decode::{nth, variant}; + + if let Some(("Id", inner)) = variant(value) { + return nth(inner, 0).and_then(|v| account_bytes(v, metadata)); + } + if !type_is_named(metadata, value.context, "AccountId32") { + return None; + } + let bytes = byte_blob(value)?; + bytes.try_into().ok() +} + async fn handle_proposal_info( multisig_address: String, proposal_id: u32, @@ -3307,6 +3339,39 @@ mod execute_call_tests { /// The regression this decoder exists for: a call index the runtime does not /// define must never be rendered as some other call. The old implementation /// matched hardcoded indices and would confidently mislabel it. + /// An `AccountId32` argument renders as an address... + #[test] + fn describe_call_renders_account_id_as_ss58() { + let md = test_metadata(); + let (pallet_idx, call_idx) = call_indices(&md, "Balances", "transfer_allow_death"); + let mut bytes = vec![pallet_idx, call_idx, 0x00]; + bytes.extend_from_slice(&[7u8; 32]); + bytes.extend_from_slice(&codec::Compact(1u128).encode()); + + let out = describe_call(&md, &bytes, None); + let expected = SpAccountId32::from([7u8; 32]).to_ss58check(); + assert!(out.contains(&expected), "expected SS58 {expected} in: {out}"); + } + + /// ...but a 32-byte value the metadata does NOT call an account must not be. + /// `authorize_upgrade.code_hash` is an `H256`; showing it as an address would + /// stop a signer comparing it against the authorized artifact. + #[test] + fn describe_call_does_not_render_a_hash_as_an_address() { + let md = test_metadata(); + let (pallet_idx, call_idx) = call_indices(&md, "System", "authorize_upgrade"); + let mut bytes = vec![pallet_idx, call_idx]; + bytes.extend_from_slice(&[7u8; 32]); + + let out = describe_call(&md, &bytes, None); + let ss58 = SpAccountId32::from([7u8; 32]).to_ss58check(); + assert!(!out.contains(&ss58), "H256 must not render as SS58: {out}"); + assert!( + out.contains(&format!("0x{}", hex::encode([7u8; 32]))), + "H256 must render as lossless hex: {out}" + ); + } + #[test] fn describe_call_refuses_to_guess_an_unknown_call_index() { let md = test_metadata(); From 7d746050f9575d902125bc5b010ac07173449193 Mon Sep 17 00:00:00 2001 From: Nikolaus Heger Date: Wed, 2 Sep 2026 12:43:20 +0800 Subject: [PATCH 4/5] fix: gate byte-blob rendering on the declared type, not the decoded shape byte_blob treated any composite whose elements all read as numbers as a byte string, and the shared uint helper reaches into composites to get there. Multisig::create_multisig.signers is a Vec, so a set of two or more signers rendered as one hex byte per signer and dropped 31 of every 32 bytes. That is signer-visible data loss: the account approving a proposal could not see who was actually in the multisig. The metadata now decides. is_byte_sequence resolves value.context and flattens only a Vec, a [u8; N], or a single-field wrapper around one (H256, BoundedVec). Everything else keeps its structure. The per-element read is strict: dynamic_decode::byte takes a primitive only, so a composite can no longer pass as a byte by yielding its first field. A sequence the metadata does not call bytes now renders element by element, so a signer set reads as addresses rather than a wall of numbers. Tests decode a real two-signer create_multisig and assert both SS58 addresses appear and the truncated hex does not, and assert directly that Vec is not a byte run while H256 is. --- Cargo.lock | 1 + Cargo.toml | 3 + src/cli/dynamic_decode.rs | 9 +++ src/cli/multisig.rs | 149 +++++++++++++++++++++++++++++++++++--- 4 files changed, 152 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a5b46e2..760299a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4918,6 +4918,7 @@ dependencies = [ "rpassword", "rustls-webpki", "rxing", + "scale-info", "self-replace", "self_update", "serde", diff --git a/Cargo.toml b/Cargo.toml index d089368..10ba171 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,6 +108,9 @@ event-listener = "5.4.2" # Blockchain deps: align with chain workspace codec = { package = "parity-scale-codec", version = "3.7", features = ["derive"] } +# Resolving a metadata type id to its `TypeDef`, to tell a real byte run from a `Vec` of +# structs. subxt re-exports scale-value and scale-decode but not scale-info. +scale-info = { version = "2.11", default-features = false } jsonrpsee = { version = "0.24", features = ["client"] } sp-core = { version = "39.0.0" } sp-runtime = { version = "45.0.0" } diff --git a/src/cli/dynamic_decode.rs b/src/cli/dynamic_decode.rs index 2d232d8..119c64f 100644 --- a/src/cli/dynamic_decode.rs +++ b/src/cli/dynamic_decode.rs @@ -60,6 +60,15 @@ pub(crate) fn uint(v: &Val) -> Option { } } +/// One byte, read strictly. Unlike [`uint`] this does not reach into composites: a +/// `Vec` must not pass as a byte string by yielding each account's first byte. +pub(crate) fn byte(v: &Val) -> Option { + match &v.value { + ValueDef::Primitive(Primitive::U128(n)) => u8::try_from(*n).ok(), + _ => None, + } +} + pub(crate) fn boolean(v: &Val) -> Option { match &v.value { ValueDef::Primitive(Primitive::Bool(b)) => Some(*b), diff --git a/src/cli/multisig.rs b/src/cli/multisig.rs index 81711f6..f579913 100644 --- a/src/cli/multisig.rs +++ b/src/cli/multisig.rs @@ -2430,9 +2430,22 @@ fn render_call_arg( if let Some(bytes) = account_bytes(value, metadata) { return SpAccountId32::from(bytes).to_ss58check().bright_cyan().to_string(); } - if let Some(bytes) = byte_blob(value) { + if let Some(bytes) = byte_blob(value, metadata) { return format!("0x{}", hex::encode(bytes)); } + // A sequence the metadata does not call bytes: render it element by element, so a signer + // set reads as addresses a signer can check rather than a wall of numbers. + if is_sequence(metadata, value.context) { + if let Some(subxt::ext::scale_value::Composite::Unnamed(items)) = + crate::cli::dynamic_decode::composite(value) + { + let rendered: Vec = items + .iter() + .map(|item| render_call_arg(item, field_name, symbol_decimals, metadata)) + .collect(); + return format!("[{}]", rendered.join(", ")); + } + } let looks_like_balance = matches!(field_name, "value" | "amount" | "new_free" | "fee" | "deposit"); if looks_like_balance { @@ -2456,9 +2469,61 @@ fn type_is_named(metadata: &subxt::Metadata, id: u32, name: &str) -> bool { .unwrap_or(false) } -/// A flat `Vec` / `[u8; N]` payload, for lossless hex rendering. -fn byte_blob(value: &subxt::ext::scale_value::Value) -> Option> { - use crate::cli::dynamic_decode::{composite, nth, uint}; +/// Whether the metadata declares `id` a flat run of bytes: a `Vec`, a `[u8; N]`, or a +/// single-field wrapper around one (`H256`, `BoundedVec`). +/// +/// The type has to decide this, not the decoded value. Treating "a composite whose elements +/// all read as numbers" as bytes makes `Vec` look like a byte string and renders +/// one byte per signer, silently dropping 31 of every 32 bytes of a signer set — the one thing +/// a signer must be able to check before approving a proposal. +fn is_byte_sequence(metadata: &subxt::Metadata, id: u32) -> bool { + use scale_info::TypeDef; + + let Some(ty) = metadata.types().resolve(id) else { + return false; + }; + match &ty.type_def { + TypeDef::Sequence(seq) => is_u8(metadata, seq.type_param.id), + TypeDef::Array(arr) => is_u8(metadata, arr.type_param.id), + TypeDef::Composite(c) => match c.fields.as_slice() { + [only] => is_byte_sequence(metadata, only.ty.id), + _ => false, + }, + _ => false, + } +} + +/// Whether the metadata declares `id` a sequence or array of anything. +fn is_sequence(metadata: &subxt::Metadata, id: u32) -> bool { + use scale_info::TypeDef; + + matches!( + metadata.types().resolve(id).map(|ty| &ty.type_def), + Some(TypeDef::Sequence(_) | TypeDef::Array(_)) + ) +} + +fn is_u8(metadata: &subxt::Metadata, id: u32) -> bool { + use scale_info::{TypeDef, TypeDefPrimitive}; + + matches!( + metadata.types().resolve(id).map(|ty| &ty.type_def), + Some(TypeDef::Primitive(TypeDefPrimitive::U8)) + ) +} + +/// A flat `Vec` / `[u8; N]` payload, for lossless hex rendering. `None` for anything the +/// metadata does not declare a byte run, which then renders structured instead. +fn byte_blob( + value: &subxt::ext::scale_value::Value, + metadata: &subxt::Metadata, +) -> Option> { + is_byte_sequence(metadata, value.context).then(|| flatten_bytes(value))? +} + +/// Collect the bytes of a value [`is_byte_sequence`] has already vouched for. +fn flatten_bytes(value: &subxt::ext::scale_value::Value) -> Option> { + use crate::cli::dynamic_decode::{byte, composite, nth}; use subxt::ext::scale_value::Composite; let fields: &Composite = composite(value)?; @@ -2468,10 +2533,8 @@ fn byte_blob(value: &subxt::ext::scale_value::Value) -> Option> { }; // A newtype wrapper (H256(pub [u8; 32])) nests one level. if len == 1 { - if let Some(inner) = nth(fields, 0) { - if let Some(bytes) = byte_blob(inner) { - return Some(bytes); - } + if let Some(bytes) = nth(fields, 0).and_then(flatten_bytes) { + return Some(bytes); } } if len == 0 { @@ -2479,7 +2542,7 @@ fn byte_blob(value: &subxt::ext::scale_value::Value) -> Option> { } let mut out = Vec::with_capacity(len); for i in 0..len { - out.push(u8::try_from(uint(nth(fields, i)?)?).ok()?); + out.push(byte(nth(fields, i)?)?); } Some(out) } @@ -2499,7 +2562,7 @@ fn account_bytes( if !type_is_named(metadata, value.context, "AccountId32") { return None; } - let bytes = byte_blob(value)?; + let bytes = byte_blob(value, metadata)?; bytes.try_into().ok() } @@ -3372,6 +3435,72 @@ mod execute_call_tests { ); } + /// The regression: `Multisig::create_multisig.signers` is a `Vec`. Deciding + /// "bytes" from the decoded value made a two-signer set render as two hex bytes — one per + /// account — so the signer approving the proposal could not see who was in it. + #[test] + fn describe_call_renders_every_signer_of_a_signer_set() { + let md = test_metadata(); + let (pallet_idx, call_idx) = call_indices(&md, "Multisig", "create_multisig"); + + let signers = [[0x11u8; 32], [0x22u8; 32]]; + let mut bytes = vec![pallet_idx, call_idx]; + bytes.extend_from_slice(&codec::Compact(signers.len() as u32).encode()); + for signer in &signers { + bytes.extend_from_slice(signer); + } + bytes.extend_from_slice(&2u32.encode()); + bytes.extend_from_slice(&0u64.encode()); + + let out = describe_call(&md, &bytes, None); + assert!(!out.contains("trailing byte"), "{out}"); + for signer in &signers { + let expected = SpAccountId32::from(*signer).to_ss58check(); + assert!(out.contains(&expected), "signer {expected} missing from: {out}"); + } + assert!( + !out.contains("0x1122"), + "a signer set must never collapse to one byte per account: {out}" + ); + } + + /// The flattening that renders `H256` as hex must key on the metadata type, not on the + /// decoded shape, or any `Vec` of small structs silently loses all but its first field. + #[test] + fn byte_blob_only_flattens_what_the_metadata_calls_bytes() { + let md = test_metadata(); + let signers_ty = md + .pallets() + .find(|p| p.name() == "Multisig") + .and_then(|p| p.call_variants().map(|v| v.to_vec())) + .expect("Multisig has calls") + .into_iter() + .find(|v| v.name == "create_multisig") + .expect("create_multisig present") + .fields + .first() + .expect("create_multisig takes signers") + .ty + .id; + assert!(!is_byte_sequence(&md, signers_ty), "Vec is not a byte run"); + assert!(is_sequence(&md, signers_ty), "but it is a sequence"); + + let hash_ty = md + .pallets() + .find(|p| p.name() == "System") + .and_then(|p| p.call_variants().map(|v| v.to_vec())) + .expect("System has calls") + .into_iter() + .find(|v| v.name == "authorize_upgrade") + .expect("authorize_upgrade present") + .fields + .first() + .expect("authorize_upgrade takes a code hash") + .ty + .id; + assert!(is_byte_sequence(&md, hash_ty), "H256 is a deliberate byte wrapper"); + } + #[test] fn describe_call_refuses_to_guess_an_unknown_call_index() { let md = test_metadata(); From 53879c3c60188a0d9b8be4f094e488853f8223c8 Mon Sep 17 00:00:00 2001 From: Nikolaus Heger Date: Wed, 2 Sep 2026 12:49:11 +0800 Subject: [PATCH 5/5] style: order the scale-info dependency for taplo --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 10ba171..19fea9e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,10 +108,10 @@ event-listener = "5.4.2" # Blockchain deps: align with chain workspace codec = { package = "parity-scale-codec", version = "3.7", features = ["derive"] } +jsonrpsee = { version = "0.24", features = ["client"] } # Resolving a metadata type id to its `TypeDef`, to tell a real byte run from a `Vec` of # structs. subxt re-exports scale-value and scale-decode but not scale-info. scale-info = { version = "2.11", default-features = false } -jsonrpsee = { version = "0.24", features = ["client"] } sp-core = { version = "39.0.0" } sp-runtime = { version = "45.0.0" }