chore: update hathorlib to v0.14.1, Python to 3.11 and modernize dependencies - #161
Merged
Conversation
luislhl
force-pushed
the
chore/update-hathorlib-0.14.1
branch
2 times, most recently
from
March 2, 2026 20:48
7d003be to
131fda2
Compare
glevco
approved these changes
Mar 2, 2026
github-project-automation
Bot
moved this from In Progress (Done)
to In Review (WIP)
in Hathor Network
r4mmer
approved these changes
Mar 12, 2026
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCI, Docker, and pyproject bumped to Python 3.11+; tests migrated off asynctest to asyncio-native patterns (unittest.IsolatedAsyncioTestCase or a new ClockedTestCase); tests/utils adds an advanceable synthetic clock to patch loop time. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Dockerfile (1)
16-24:⚠️ Potential issue | 🟠 MajorRun the final container as a non-root user.
The final stage has no
USERdirective, so it runs as root by default. This is a security hardening gap for production containers.🔒 Proposed hardening patch
FROM python:3.11-alpine +WORKDIR /app COPY --from=build /usr/local/lib/python3.11/site-packages /usr/local/lib/python3.11/site-packages RUN apk add libgcc COPY txstratum/ ./txstratum COPY main.py log.conf ./ +RUN addgroup -S app && adduser -S -G app app \ + && chown -R app:app /app +USER app + ENTRYPOINT ["python", "-m", "main"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 16 - 24, The Dockerfile final stage runs as root (no USER directive) which is insecure; create a non-root runtime user and group (e.g., add a user like "appuser"), ensure application files copied by COPY (/txstratum, main.py, log.conf and site-packages) are owned by that user (chown) and switch to it before ENTRYPOINT by adding a USER directive; update the Dockerfile around the COPY and ENTRYPOINT steps so ownership is set and the final image uses USER appuser to run the ENTRYPOINT ["python", "-m", "main"].pyproject.toml (1)
18-27:⚠️ Potential issue | 🔴 Critical
asynctest = "^0.13.0"is incompatible with Python 3.11+ and will break installation.Line 18 enforces
python = ">=3.11,<4.0", butasynctest0.13.0 only declares support for Python 3.5–3.7 per PyPI metadata. Either upgradeasynctestto a version supporting 3.11+, or remove it if no longer needed. Note:numpyis not present in the current dependency list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 18 - 27, pyproject.toml declares Python >=3.11 but lists asynctest = "^0.13.0", which is incompatible with 3.11; update pyproject.toml to either remove the asynctest dependency if it's unused or replace it with a Python-3.11-compatible test async library (e.g., switch to pytest-asyncio or a maintained fork) and update any test imports/usages accordingly (look for references to "asynctest" in tests and test config), then bump the dependency entry or delete the line so the declared python version and test dependency are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Dockerfile`:
- Around line 16-24: The Dockerfile final stage runs as root (no USER directive)
which is insecure; create a non-root runtime user and group (e.g., add a user
like "appuser"), ensure application files copied by COPY (/txstratum, main.py,
log.conf and site-packages) are owned by that user (chown) and switch to it
before ENTRYPOINT by adding a USER directive; update the Dockerfile around the
COPY and ENTRYPOINT steps so ownership is set and the final image uses USER
appuser to run the ENTRYPOINT ["python", "-m", "main"].
In `@pyproject.toml`:
- Around line 18-27: pyproject.toml declares Python >=3.11 but lists asynctest =
"^0.13.0", which is incompatible with 3.11; update pyproject.toml to either
remove the asynctest dependency if it's unused or replace it with a
Python-3.11-compatible test async library (e.g., switch to pytest-asyncio or a
maintained fork) and update any test imports/usages accordingly (look for
references to "asynctest" in tests and test config), then bump the dependency
entry or delete the line so the declared python version and test dependency are
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26353f1c-3900-4ce7-acf0-5338a3288ff6
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/main.ymlDockerfilepyproject.toml
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/test_prometheus.py (1)
56-66: Consider storing the task reference for cleaner semantics.The static analysis flags that
asyncio.ensure_future()return value is not stored. While the current polling approach works, storing the task reference would be more explicit.♻️ Optional refactor
async def _run_all_pending_events(self): """Run all pending events.""" async def _fn(): self.ran_all = True - asyncio.ensure_future(_fn()) + task = asyncio.ensure_future(_fn()) while getattr(self, "ran_all", False) is False: await asyncio.sleep(0.1) self.ran_all = False + return task🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_prometheus.py` around lines 56 - 66, The helper _run_all_pending_events currently calls asyncio.ensure_future(_fn()) without keeping the returned Task; change it to store the Task (e.g., task = asyncio.create_task(_fn()) or task = asyncio.ensure_future(_fn())) so the task reference is retained, and then either await task or cancel/cleanup it after use; update the logic around getattr(self, "ran_all", False) and setting self.ran_all = False to ensure the stored task is awaited or properly cancelled to avoid unreferenced background tasks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 27: The pyproject dependency declaration for "hathorlib" includes an
extras spec that may not exist in version 0.14.1; inspect the dependency line
referencing hathorlib (hathorlib = {version = "^0.14.1", extras = ["client"]})
and either remove the extras key entirely or verify upstream that version 0.14.1
provides the "client" extra and update the version to one that does if needed;
ensure the final pyproject.toml uses either hathorlib = "^0.14.1" or a
version+extras combination confirmed to exist.
- Line 26: The setuptools version constraint ("setuptools = \">=68.0,<71\"") is
overly restrictive; either relax it to allow modern stable releases (for example
change to "setuptools = \">=68.0,<83\"" or simply ">=68.0") or explicitly
document why the "<71" cap is required; update the pyproject.toml line for
"setuptools" accordingly or add a comment above that line describing the
compatibility reason and the affected dependency if the cap must remain.
In `@tests/test_filters.py`:
- Line 9: The import line in tests/test_filters.py includes an unused symbol
MagicMock; remove MagicMock from the import so only AsyncMock is imported (i.e.,
change "from unittest.mock import AsyncMock, MagicMock" to import only
AsyncMock) and run the tests to ensure there are no other references to
MagicMock in this file.
---
Nitpick comments:
In `@tests/test_prometheus.py`:
- Around line 56-66: The helper _run_all_pending_events currently calls
asyncio.ensure_future(_fn()) without keeping the returned Task; change it to
store the Task (e.g., task = asyncio.create_task(_fn()) or task =
asyncio.ensure_future(_fn())) so the task reference is retained, and then either
await task or cancel/cleanup it after use; update the logic around getattr(self,
"ran_all", False) and setting self.ran_all = False to ensure the stored task is
awaited or properly cancelled to avoid unreferenced background tasks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ebe9ab4-920f-42e5-94f9-4a7792d4c6ae
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
pyproject.tomltests/test_filters.pytests/test_healthcheck.pytests/test_manager.pytests/test_prometheus.pytests/test_time.pytests/utils.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
pyproject.toml (1)
27-27:⚠️ Potential issue | 🟠 Major
hathorlibextra"client"still looks unsupported in 0.14.1.Line 27 keeps
extras = ["client"], but PyPI metadata forhathorlib==0.14.1currently reportsprovides_extra: null. Please remove the extra or move to a release that explicitly exports it.
Source: https://pypi.org/pypi/hathorlib/0.14.1/json🔧 Proposed fix
-hathorlib = {version = "^0.14.1", extras = ["client"]} +hathorlib = "^0.14.1"#!/bin/bash # Verify whether hathorlib 0.14.1 exposes extras metadata curl -s https://pypi.org/pypi/hathorlib/0.14.1/json | jq '.info.version, .info.provides_extra, .info.requires_dist'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` at line 27, The dependency declaration hathorlib = {version = "^0.14.1", extras = ["client"]} is incorrect because version 0.14.1 does not expose the "client" extra; update pyproject.toml by removing the extras field from the Hathor dependency (change to hathorlib = "^0.14.1" or the equivalent table form without extras) or replace the version with a specific release that documents and provides the "client" extra; ensure you update the same dependency entry (hathorlib) and run dependency resolution to verify the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_prometheus.py`:
- Around line 62-65: Replace the fire-and-forget asyncio.ensure_future(_fn())
and the polling loop that waits on self.ran_all with an explicitly created task
and awaiting it: create the task with asyncio.create_task(_fn()) (keep reference
to the task variable), then await that task instead of the while/getattr loop so
task completion and exceptions propagate properly; update the test to remove the
sleep-poll loop and use the stored task reference to await completion and
optionally handle/inspect exceptions from the task.
In `@tests/utils.py`:
- Around line 65-71: The advance() helper currently uses a fixed for-loop of 100
iterations which can fail to drain deep callback chains; change it to repeatedly
await asyncio.sleep(0) until the event loop has no more ready callbacks.
Concretely, in advance() (which adjusts self.loop._test_clock_offset) replace
the for _ in range(100) loop with a while loop that checks the loop's ready
queue (e.g. inspect self.loop._ready or equivalent) and breaks only when that
queue is empty, yielding each iteration with await asyncio.sleep(0) so all
chained call_later()/callbacks are drained.
---
Duplicate comments:
In `@pyproject.toml`:
- Line 27: The dependency declaration hathorlib = {version = "^0.14.1", extras =
["client"]} is incorrect because version 0.14.1 does not expose the "client"
extra; update pyproject.toml by removing the extras field from the Hathor
dependency (change to hathorlib = "^0.14.1" or the equivalent table form without
extras) or replace the version with a specific release that documents and
provides the "client" extra; ensure you update the same dependency entry
(hathorlib) and run dependency resolution to verify the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65d1e14f-ea6b-416b-ba2a-2514f55d2334
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
pyproject.tomltests/test_filters.pytests/test_prometheus.pytests/test_time.pytests/utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_time.py
luislhl
requested review from
glevco and
r4mmer
and removed request for
msbrogli
March 17, 2026 22:56
…pdate tests using it;
luislhl
force-pushed
the
chore/update-hathorlib-0.14.1
branch
from
March 17, 2026 23:11
515033c to
c33034b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_prometheus.py (1)
56-67: Store the task reference to silence Ruff warning.The static analysis tool correctly identifies that the return value of
asyncio.create_task()should be stored. While the current implementation works, storing the reference is cleaner and prevents the task from being garbage collected prematurely in edge cases.Optional fix to store task reference
async def _run_all_pending_events(self): """Run all pending events.""" async def _fn(): self.ran_all = True - asyncio.create_task(_fn()) + task = asyncio.create_task(_fn()) while getattr(self, "ran_all", False) is False: await asyncio.sleep(0) self.ran_all = False + # Ensure task completed (should be instant since ran_all is True) + await task🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_prometheus.py` around lines 56 - 67, The helper method _run_all_pending_events creates a background coroutine with asyncio.create_task() but doesn't store the returned Task, triggering a Ruff warning; update _run_all_pending_events to assign the result of asyncio.create_task(_fn()) to a local variable (e.g., task) so the Task reference is retained until completion, leaving the rest of the loop logic (polling ran_all and resetting it) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/test_prometheus.py`:
- Around line 56-67: The helper method _run_all_pending_events creates a
background coroutine with asyncio.create_task() but doesn't store the returned
Task, triggering a Ruff warning; update _run_all_pending_events to assign the
result of asyncio.create_task(_fn()) to a local variable (e.g., task) so the
Task reference is retained until completion, leaving the rest of the loop logic
(polling ran_all and resetting it) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ef171d9-b953-495b-b363-40467ec1eb00
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.github/workflows/main.ymlDockerfilepyproject.tomltests/test_filters.pytests/test_healthcheck.pytests/test_manager.pytests/test_prometheus.pytests/test_time.pytests/utils.pytxstratum/utils.py
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/main.yml
- tests/test_manager.py
- tests/test_time.py
- tests/test_healthcheck.py
luislhl
force-pushed
the
chore/update-hathorlib-0.14.1
branch
from
March 18, 2026 15:46
c928ba5 to
3b048df
Compare
luislhl
changed the title
luislhl
changed the title
glevco
approved these changes
Mar 18, 2026
github-project-automation
Bot
moved this from In Progress (Done)
to In Review (WIP)
in Hathor Network
r4mmer
approved these changes
Mar 18, 2026
github-project-automation
Bot
moved this from In Review (Done)
to Waiting to be deployed
in Hathor Network
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Acceptance Criteria
--testnet, instead ofTXMINING_CONFIG_FILE, because of feat: pydantic settings hathor-core#1600Summary by CodeRabbit
Chores
Tests
Documentation