feat: prioritise tx by dust

[gilescope]

Overview

We should prioritise high dust transactions over low dust transactions.

🗹 TODO before merging

• Ready

📌 Submission Checklist

• Changes are backward-compatible (or flagged if breaking)
• Pull request description explains why the change is needed
• Self-reviewed the diff
• I have included a change file, or skipped for this reason:
• If the changes introduce a new feature, I have bumped the node minor version
• Update documentation (if relevant)
• Updated AGENTS.md if build commands, architecture, or workflows changed
• No new todos introduced

🧪 Testing Evidence

Please describe any additional testing aside from CI:

• Additional tests are provided (if possible)

🔱 Fork Strategy

• Node Runtime Update
• Node Client Update
• Other:
• N/A

Links

[github-actions]

KICS version: v2.1.16

Category Results CRITICAL 0 HIGH 0 MEDIUM 96 LOW 12 INFO 83 TRACE 0 TOTAL 191	Metric Values Files scanned 31 Files parsed 31 Files failed to scan 0 Total executed queries 73 Queries failed to execute 0 Execution time 8

[m2ux]

PR Review Summary

PR: #741 - feat: prioritise tx by dust
Reviewer: Work Package Workflow Orchestrator Agent
Date: 2026-02-23
Ticket: PM-21985

Executive Summary

Clean, well-scoped change that populates ValidTransaction::priority from transaction gas cost, enabling Substrate's native fee-based ordering in the mempool. The implementation is minimal (10 lines production code), follows established codebase patterns, and all tests pass. Two medium-severity improvement suggestions and one test gap noted — none are merge-blocking.

Overall Rating: Comment Only (no blocking issues)

Detailed Reports

Report	Description
Design Philosophy	Problem classification, complexity assessment
Work Package Plan	Retrospective implementation plan
Test Plan	Test strategy and gap analysis
Code Review	Full code review findings
Test Suite Review	Test quality assessment
Validation Results	Build, test, lint results
Strategic Review	Scope and minimality assessment

---

Code Review Findings

#	Severity	Category	Finding	Location
M1	Medium	Code Reuse	Pallet already has Self::get_transaction_cost(tx) helper (used by get_tx_weight). The PR calls LedgerApi::get_transaction_cost directly with manually-assembled parameters. Using the helper would reduce duplication.	lib.rs:550-557
M2	Medium	Observability	.unwrap_or(0) silently degrades to lowest priority on cost-computation failure. No logging or metrics — operators would have no indication that fee ordering is not functioning.	lib.rs:557
L1	Low	Clarity	Self::get_block_context() is used instead of the block_context parameter. Benign today (parameter is #[allow(unused_variables)] in the ledger), but non-obvious to future readers.	lib.rs:553-554
L2	Low	Style	Minor: change file has no blank line between description and metadata. Consistent with other change files, so not actionable.	fee-priority-mempool.md

Finding Details

M1. Code reuse opportunity

The inner validate_unsigned calls LedgerApi::get_transaction_cost(&state_key, midnight_tx, Self::get_block_context(), max_weight) directly. The pallet already exposes Self::get_transaction_cost(tx) at line 582 which does the same thing, and is used by get_tx_weight() at line 600. Using the helper would reduce the call to:

let priority = Self::get_transaction_cost(midnight_tx).unwrap_or(0);

The direct call may be intentional to avoid a redundant StateKey::get() (already in scope), but the saving is negligible within the same execution context.

M2. Silent cost-computation fallback

The .unwrap_or(0) pattern is consistent with get_tx_weight (line 600-603), so this follows an established codebase convention. However, for a feature whose purpose is fee-based ordering, silently falling back to priority 0 under failure means the feature can stop working without any observable signal. Consider:

let priority = LedgerApi::get_transaction_cost(...)
    .unwrap_or_else(|e| {
        log::warn!(target: "midnight", "Failed to compute tx cost for priority: {:?}", e);
        0
    });

L1. BlockContext parameter discrepancy

validate_transaction receives an adjusted block_context (with tblock bumped for slot duration and skipped slots). The subsequent get_transaction_cost uses Self::get_block_context() (unadjusted). This is inconsequential today because block_context is unused in the current cost model (#[allow(unused_variables)] at ledger/src/versions/common/mod.rs:590), but the TODO COST MODEL comment suggests this function will be reworked. A brief inline comment would help future maintainers.

---

Test Review Findings

Gap	Severity	Recommendation
No relative ordering test	Medium	The core feature goal — higher-fee txs get higher priority — is not directly tested. test_validation_sets_priority asserts priority > 0 but doesn't compare two transactions with different costs.
No exact priority value match	Low	Could assert result.priority == get_transaction_cost() to verify the cost-to-priority mapping
No fallback path test	Low	Hard to test (requires state where validation passes but cost fails); trivially correct pattern

The existing test_validation_sets_priority is well-structured and follows codebase conventions. The > 0 assertion is resilient across cost model changes, which is pragmatic given the TODO COST MODEL marker on the existing test_get_mn_transaction_fee test (currently #[ignore]).

---

Validation Results

Check	Status	Notes
Build	✅ Pass	cargo check -p pallet-midnight clean
Tests	✅ Pass	19/19 passed, 0 failed, 3 pre-existing ignored
Clippy	✅ Pass	No warnings on pallet code

---

Scope Assessment

Criterion	Status
Minimal changes	✅ 3 files, 33 additions, 0 deletions
No scope creep	✅ All changes support stated objective
Follows conventions	✅ Change file, test, commit message format
No investigation artifacts	✅ Clean

---

Action Items

Should Address (Recommended):

• Consider using Self::get_transaction_cost() helper for consistency (M1)
• Consider adding log::warn! for cost-computation failures (M2)

Nice to Have (Optional):

• Add a relative ordering test if fixtures with different costs are available (G2)
• Add brief comment explaining Self::get_block_context() usage vs parameter (L1)