Skip to content

Conversation

@1yam
Copy link
Member

@1yam 1yam commented Oct 22, 2025

In case of store message, the related content (file) isn't fetched during the fetch pipeline but is direcly fetched while processing it, making the processing block for no reason.

Related Clickup or Jira tickets : ALEPH-XXX

Self proofreading checklist

  • Is my code clear enough and well documented
  • Are my files well typed
  • Are there enough tests

Changes

This pull request refactors the message verification and content fetching flow to ensure that database sessions are consistently passed and that related content is fetched at the appropriate stage. It also updates tests to reflect these changes and ensures required files are present in the database before processing messages. The changes improve reliability and maintainability by enforcing a clearer separation of concerns between message verification, balance checking, and content fetching.

Core handler refactor:

  • The verify_message method in MessageHandler now requires a session argument and performs a balance pre-check within the method, streamlining the verification and pre-check process. (src/aleph/handlers/message_handler.py)
  • The process method no longer performs a separate balance pre-check or fetches related content; these responsibilities have been moved to the verification stage, simplifying the processing pipeline. (src/aleph/handlers/message_handler.py)

Content fetching pipeline update:

  • The pending message fetch job now explicitly fetches related content (such as IPFS files) after message verification, ensuring all dependencies are loaded before further processing. (src/aleph/jobs/fetch_pending_messages.py)

Test suite updates:

  • Multiple tests now insert required files into the database before processing messages and fetch related content after verification, ensuring test reliability and alignment with the new handler logic. (tests/message_processing/test_process_forgets.py, tests/message_processing/test_process_stores.py, tests/test_network.py) [1] [2] [3] [4] [5] [6] [7] [8] [9]

Imports and setup:

  • Added missing imports for file upsert and file type in test modules to support new test setup requirements. (tests/message_processing/test_process_forgets.py, tests/message_processing/test_process_stores.py) [1] [2] [3]

Notes

There is still an issue with the pinning of message blocking the pipeline, but better to be block during the fetch than the processing

@1yam 1yam requested a review from odesenfans October 22, 2025 18:32
Copy link
Collaborator

@odesenfans odesenfans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Remove the changes inherited by #871, they're out of focus
  2. Check permissions in the fetch pipeline
  3. To discuss: IMO we should keep on pinning files in the processing pipeline as well, I see the fetch pipeline as a way to speed things up but the processing pipeline should have the full logic to be 100% sure that files get stored properly.

@1yam 1yam force-pushed the 1yam-fix-fetch-pipeline branch from 856f6f4 to 6fbc36e Compare October 23, 2025 09:23
@1yam 1yam force-pushed the 1yam-fix-fetch-pipeline branch from 6fbc36e to 8d1fedc Compare October 23, 2025 09:24
@1yam 1yam requested a review from odesenfans October 23, 2025 09:55
Copy link
Collaborator

@odesenfans odesenfans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking pretty good, just one more comment.

@1yam 1yam enabled auto-merge (squash) October 24, 2025 09:38
@1yam 1yam disabled auto-merge October 24, 2025 09:38
@1yam 1yam merged commit 3f27e06 into main Oct 24, 2025
5 checks passed
odesenfans pushed a commit that referenced this pull request Nov 26, 2025
* Fix: messages & pre check balance should be done on fetch pipeline

* Fix: unit test missing file in db due to fetching adding the pin in db instead of processing

* Refactor: `verify_message` renamed to `verify_and_fetch_message`. now fetch related content & verify permissions

* fix: processing pipeline should still fetch the file if the fetch pipeline didn't

* fix: remove useless change from test
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.

3 participants