Skip to content

refactor: add is_already_voted predicate for AlreadyVoted rejections#101

Closed
dev-miro26 wants to merge 1 commit intoentrius:testfrom
dev-miro26:refactor/is-already-voted-predicate
Closed

refactor: add is_already_voted predicate for AlreadyVoted rejections#101
dev-miro26 wants to merge 1 commit intoentrius:testfrom
dev-miro26:refactor/is-already-voted-predicate

Conversation

@dev-miro26
Copy link
Copy Markdown

Closes: #100

Summary

  • Add is_already_voted(e) alongside is_contract_rejection in allways/contract_client.py — a single source of truth for detecting the AlreadyVoted contract-error variant, anchored to CONTRACT_ERROR_VARIANTS[3].
  • Replace two inline 'AlreadyVoted' in str(e) substring checks in allways/validator/forward.py with calls to the new predicate.
  • No behavior change — the detection semantics are identical today; this is purely a centralization so the substring literal lives in exactly one place.

Why

forward.py:254 and forward.py:395 were branching on if 'AlreadyVoted' in str(e):. The codebase already exports a structured CONTRACT_ERROR_VARIANTS map and a canonical is_contract_rejection(e) predicate from allways/contract_client.py, but the AlreadyVoted check bypassed both with a raw substring match. If the ContractError message format ever changes (e.g. variant rename, localisation, format tweak in decode_contract_error), the two call sites would silently stop matching with no lint or test signal.

Co-locating the predicate with is_contract_rejection also leaves room for a small documented predicate set (is_duplicate_source_tx, is_miner_reserved, …) if future code wants variant-specific branching.

Changes

  • allways/contract_client.py: add is_already_voted(e) -> bool next to is_contract_rejection; docstring points at CONTRACT_ERROR_VARIANTS[3] as the canonical variant source.
  • allways/validator/forward.py:
    • Import is_already_voted alongside the existing is_contract_rejection.
    • forward.py:254 (extend-reservation vote failure branch) — swap substring check for predicate.
    • forward.py:395 (extend-timeout vote failure branch) — swap substring check for predicate.
  • Net +8 / −2 lines; no signature changes.

Type of Change

  • Refactoring (centralisation of an existing predicate, no behavior change)

Test plan

  • ruff check allways/ neurons/ tests/ — clean
  • ruff format --check allways/ neurons/ tests/ — clean
  • pytest tests/291 passed; 9 pre-existing failures in tests/test_axon_handlers.py on upstream/test are unrelated ("too many values to unpack (expected 3)" in the SwapConfirm handler) and unchanged by this diff
  • rg "'AlreadyVoted' in str" — now returns exactly one hit (the predicate itself), zero remaining inline substring checks

@dev-miro26
Copy link
Copy Markdown
Author

Hi, @LandynDev
Could you please review my first PR?
Thank you.

@anderdc anderdc added the refactor Restructures code without changing behavior label Apr 20, 2026
@anderdc
Copy link
Copy Markdown
Collaborator

anderdc commented Apr 21, 2026

Closing — is_already_voted(e) has a single call site (try_extend_reservation) and replaces 'AlreadyVoted' in str(e) (one line) with is_already_voted(e) (one line). No call-site simplification, just an indirection. The neighbor is_contract_rejection and other error-string checks throughout contract_client.py still use raw in str(e) patterns — extracting one of them in isolation is churn rather than cleanup.

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

Labels

refactor Restructures code without changing behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: replace brittle 'AlreadyVoted' substring checks with a centralized predicate

2 participants