Code cleanup task

[n13]

Found on PR 70

PR #70: Referenda & Runtime Upgrade Commands Restored

Author: czareko | +2209 / -988 across 14 files | 8 commits

Overview

This PR removes the Sudo pallet from the runtime and replaces all previously sudo-gated operations with a governance-based approach using Referenda and Tech Referenda. It also bumps qp-* dependencies to 1.4.0, migrates the preimage hasher from PoseidonHasher to BlakeTwo256, and updates the chain metadata to runtime spec v123.

---

What's Good

• Sudo removal is the right direction. Moving from a single privileged key to on-chain governance (Tech Referenda for privileged ops, standard Referenda for public governance) is a meaningful security and decentralization improvement.
• Comprehensive CLI surface. Both referenda and tech-referenda subcommands offer a full lifecycle: submit, list, get, status, vote, place-decision-deposit, refund deposits, and config. The SubmitWithPreimage and SubmitTreasuryPortion workflows in Tech Referenda are convenient end-to-end flows.
• Preimage decode utility (referenda_decode.rs) is a nice UX touch -- being able to --decode a referendum proposal is valuable for governance participants.
• Tech Collective member list now uses MemberCount + IndexToId instead of raw key-byte extraction from the iterator, which is a much more robust approach.
• Docs/help strings are consistently updated to reflect that operations now go through governance rather than sudo.

---

Issues & Suggestions

1. Significant DRY Violation Between referenda.rs and tech_referenda.rs

This is the biggest concern. These two files total ~1600 lines and share a massive amount of nearly identical logic:

• list_proposals -- identical except referenda() vs tech_referenda() storage call
• get_proposal_status -- identical logic, different pallet name string
• place_decision_deposit -- identical except pallet
• refund_submission_deposit / refund_decision_deposit -- identical except pallet
• get_config -- identical except pallet constant

Additionally, the preimage submission pattern (encode call -> BlakeTwo256 hash -> note_preimage -> build Bounded::Lookup -> submit) is repeated 6 times across referenda.rs, tech_referenda.rs, and runtime.rs.

Consider extracting shared helpers, e.g.:

async fn submit_preimage_and_referendum(
    client: &QuantusClient,
    keypair: &QuantumKeyPair,
    encoded_call: Vec<u8>,
    origin_caller: OriginCaller,
    enactment_delay: u32,
    is_tech: bool, // or an enum
    execution_mode: ExecutionMode,
) -> Result<H256> { ... }

And for the read-only queries, a generic function parameterized on the referenda pallet instance.

2. Origin Caller Construction Duplicated

The match origin_type block that converts "signed"/"root"/"none" to OriginCaller appears verbatim twice in referenda.rs (lines in submit_remark_proposal and submit_proposal). This should be a shared helper.

3. referenda_decode.rs Hardcodes Pallet Indices

The decoder dispatches on raw byte tuples like (0, 0, _) for System and (18, 5, _) for Treasury. These indices can change between runtime versions. Consider:

• Adding a note that these are tied to a specific runtime version
• Or better, using subxt's metadata-driven decoding if possible

4. --decode Flag Inconsistency

referenda get --decode supports human-readable proposal decoding, but tech-referenda get does not have the --decode flag at all. Since Tech Referenda proposals (runtime upgrades, treasury portion changes) are arguably more important to decode, this seems like an oversight.

5. "Waiting for preimage confirmation" Does Nothing

In submit_remark_proposal, submit_runtime_upgrade_with_preimage, and submit_treasury_portion_with_preimage, there are log messages like:

log_print!("Waiting for preimage transaction confirmation...");

But nothing actually waits. If the preimage hasn't been included in a block before the referendum submission is sent, the referendum could fail. This is probably fine in practice (sequential transactions from the same sender are ordered by nonce), but the misleading log message should either be removed or replaced with actual confirmation logic.

6. wallet.rs Efficiency Regression

The pending transfers lookup changed from per-sender queries (pending_transfers_by_sender) to iterating all pending transfers and filtering. For a chain with many pending transfers but few entrusted accounts, this could be significantly slower. Was pending_transfers_by_sender removed from the runtime? If so, this is unavoidable, but worth a note.

7. Treasury Spend Decoding is Fragile

decode_treasury_spend_call relies on calculating field sizes by subtracting fixed offsets (args.len() - 32 - 1). If the treasury spend extrinsic format changes (e.g., the asset_kind becomes non-unit, or valid_from becomes Option<u32> with a 5-byte encoding), this will silently produce wrong results rather than erroring.

8. Minor: Compatible Runtime Versions

pub const COMPATIBLE_RUNTIME_VERSIONS: &[u32] = &[123];

Dropping compatibility with v115/v116 entirely. This is expected with the sudo removal, but worth calling out -- users on older runtimes will need to update both chain and CLI simultaneously.

---

Summary

The PR accomplishes its goal well -- the sudo-to-governance migration is clean and the new CLI commands provide a complete workflow. The main concern is the substantial code duplication between referenda.rs and tech_referenda.rs (easily 400+ lines of near-identical code), which goes against the DRY principle. Extracting shared helpers would reduce the maintenance surface and risk of the two modules diverging over time.

Recommendation: Address the DRY issues (especially the repeated preimage+referendum submission flow and the identical read-only query functions) before merging. The other items are lower priority and could be follow-ups.