Skip to content

feat: add custom check to prevent is with enums#1450

Merged
glevco merged 1 commit intomasterfrom
feat/prevent-is-enum
Oct 10, 2025
Merged

feat: add custom check to prevent is with enums#1450
glevco merged 1 commit intomasterfrom
feat/prevent-is-enum

Conversation

@glevco
Copy link
Contributor

@glevco glevco commented Oct 8, 2025

Motivation

Using is to compare values with enums is dangerous because it's possible valid values are not actually enum options. For example, 2 == TokenVersion.FEE returns True, but 2 is TokenVersion.FEE does not. This is specially dangerous when there's deserialization involved, because we have to make sure the raw value is converted to the enum, which is not always enforced by mypy.

Acceptance Criteria

  • Add line number to all grep custom checks.
  • Create a new custom check to catch usages of is when comparing with enums.
  • Update all usages to use == instead of is.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@glevco glevco self-assigned this Oct 8, 2025
@glevco glevco moved this from Todo to In Progress (WIP) in Hathor Network Oct 8, 2025
@glevco glevco force-pushed the feat/prevent-is-enum branch from 0d9f928 to f7bbc70 Compare October 8, 2025 01:36
@glevco glevco moved this from In Progress (WIP) to In Progress (Done) in Hathor Network Oct 8, 2025
@github-actions
Copy link

github-actions bot commented Oct 8, 2025

🐰 Bencher Report

Branchfeat/prevent-is-enum
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
minutes (m)
(Result Δ%)
Lower Boundary
minutes (m)
(Limit %)
Upper Boundary
minutes (m)
(Limit %)
sync-v2 (up to 20000 blocks)📈 view plot
🚷 view threshold
1.98 m
(+14.38%)Baseline: 1.73 m
1.56 m
(78.68%)
2.08 m
(95.32%)
🐰 View full continuous benchmarking report in Bencher

@raul-oliveira raul-oliveira self-requested a review October 8, 2025 01:45
raul-oliveira
raul-oliveira previously approved these changes Oct 8, 2025
@codecov
Copy link

codecov bot commented Oct 8, 2025

Codecov Report

❌ Patch coverage is 65.00000% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.02%. Comparing base (6ea3dc1) to head (48b560f).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
hathor/p2p/sync_v2/streamers.py 37.50% 4 Missing and 1 partial ⚠️
hathor/transaction/storage/transaction_storage.py 20.00% 1 Missing and 3 partials ⚠️
hathor/transaction/resources/mempool.py 0.00% 2 Missing and 1 partial ⚠️
hathor/indexes/rocksdb_utxo_index.py 66.66% 0 Missing and 2 partials ⚠️
hathor/nanocontracts/runner/runner.py 33.33% 0 Missing and 2 partials ⚠️
hathor/p2p/sync_version.py 0.00% 0 Missing and 2 partials ⚠️
hathor/transaction/resources/graphviz.py 0.00% 0 Missing and 2 partials ⚠️
hathor/websocket/streamer.py 33.33% 0 Missing and 2 partials ⚠️
hathor/builder/builder.py 0.00% 0 Missing and 1 partial ⚠️
hathor/dag_builder/default_filler.py 85.71% 0 Missing and 1 partial ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1450      +/-   ##
==========================================
+ Coverage   85.97%   86.02%   +0.05%     
==========================================
  Files         434      436       +2     
  Lines       33829    33929     +100     
  Branches     5315     5299      -16     
==========================================
+ Hits        29083    29188     +105     
+ Misses       3697     3694       -3     
+ Partials     1049     1047       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@glevco glevco force-pushed the feat/prevent-is-enum branch from fcdcfbe to 48b560f Compare October 10, 2025 18:58
@glevco glevco mentioned this pull request Oct 10, 2025
2 tasks
@github-project-automation github-project-automation bot moved this from In Progress (Done) to In Review (WIP) in Hathor Network Oct 10, 2025
@glevco glevco merged commit 6047f92 into master Oct 10, 2025
8 checks passed
@glevco glevco deleted the feat/prevent-is-enum branch October 10, 2025 20:18
@github-project-automation github-project-automation bot moved this from In Review (WIP) to Waiting to be deployed in Hathor Network Oct 10, 2025
@jansegre jansegre moved this from Waiting to be deployed to Done in Hathor Network Oct 15, 2025
@jansegre jansegre mentioned this pull request Oct 16, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants