Skip to content

Conversation

@letmehateu
Copy link

@letmehateu letmehateu commented Nov 11, 2025

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

  • Feature
  • Bugfix
  • Refactor
  • Format
  • Documentation
  • Testing
  • Other:

Description

This PR refactors the Kate RPC module to eliminate code duplication across query methods.

Problem:
Three methods (query_rows, query_proof, query_multiproof) contained duplicated validation and conversion logic:

  • Commitment validation was duplicated 3 times (18 lines)
  • Cell limit validation was duplicated 2 times (18 lines)
  • Cell-to-position conversion was duplicated 2 times (8 lines)

Solution:
Extracted common logic into three helper methods:

  • ensure_has_commitments() - validates that block has non-empty commitments
  • validate_cells_limit() - checks that requested cell count doesn't exceed configured limit
  • cells_to_positions() - converts Cells to position tuples (row, col)

Impact:

  • Removed 40 lines of duplicated code
  • Added 30 lines of reusable helper methods
  • Net reduction: 10 lines
  • Improved maintainability and reduced risk of inconsistent updates
  • No behavioral changes - pure refactoring

Related Issues

N/A

Testing Performed

  • Code reviewed for correctness
  • Verified no linter errors with cargo clippy
  • Confirmed behavioral equivalence - all methods maintain same logic flow
  • Validated that helper methods correctly encapsulate the extracted logic

Checklist

  • I have performed a self-review of my own code.
  • The tests pass successfully with cargo test.
  • The code was formatted with cargo fmt.
  • The code compiles with no new warnings with cargo build --release and cargo build --release --features runtime-benchmarks.
  • The code has no new warnings when using cargo clippy.
  • If this change affects documented features or needs new documentation, I have created a PR with a documentation update.

@letmehateu
Copy link
Author

@jakubcech hello, can you check please my PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant