Skip to content

Cap channel field length at 128 characters#148

Merged
odesenfans merged 2 commits into
mainfrom
feat/cap-channel-length
Apr 21, 2026
Merged

Cap channel field length at 128 characters#148
odesenfans merged 2 commits into
mainfrom
feat/cap-channel-length

Conversation

@odesenfans
Copy link
Copy Markdown
Contributor

Bound BaseMessage.channel and ChainRef.channel to 128 characters and hoist the shared forget/channel constants above the first model that uses them.

Test plan

  • hatch -e testing run pytest (unchanged — same 4 pre-existing network-flaky tests fail on main)

Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

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

The PR correctly adds channel length validation to BaseMessage.channel and ChainRef.channel. However, it defines MAX_FORGET_TARGETS and MAX_FORGET_REASON_LENGTH constants that are never used. The PR description mentions hoisting 'forget/channel constants', implying they should constrain ForgetContent fields (hashes list and reason string), but this was not implemented. Either apply these constraints to ForgetContent or remove the unused constants.

aleph_message/models/__init__.py (line 24): MAX_FORGET_TARGETS and MAX_FORGET_REASON_LENGTH are defined but never used. The PR description mentions 'hoist the shared forget/channel constants' suggesting they should constrain ForgetContent. Either:

  1. Apply max_length=MAX_FORGET_REASON_LENGTH to ForgetContent.reason (line 178)
  2. Add a validator to limit ForgetContent.hashes to MAX_FORGET_TARGETS items
  3. Or remove these unused constants if they're not needed

aleph_message/models/__init__.py (line 173): ForgetContent should likely use the defined constants:

  • reason field should have max_length=MAX_FORGET_REASON_LENGTH
  • hashes list should be limited to MAX_FORGET_TARGETS items via a field_validator

aleph_message/tests/test_models.py (line 1): Consider adding a test for channel length validation, e.g., test that a channel > 128 chars raises ValidationError. This would verify the new constraint works correctly.

Bound BaseMessage.channel and ChainRef.channel to 128 characters and hoist the shared forget/channel constants above the first model that uses them.
@odesenfans odesenfans force-pushed the feat/cap-channel-length branch from 5664b31 to d7b5301 Compare April 21, 2026 21:44
Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

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

This PR correctly adds a 128-character limit to the channel field in both BaseMessage and ChainRef models using Pydantic's Field(max_length=...). The constants are properly hoisted to module level before first use. The validation will protect against excessively long channel names that could cause storage or DoS issues. The test plan notes no changes needed since existing tests validate the models work correctly.

aleph_message/models/__init__.py (line 24): Note: MAX_FORGET_TARGETS and MAX_FORGET_REASON_LENGTH are defined but not yet used. This is fine as a forward-looking change, but consider adding validation to ForgetContent.hashes (limit list length) and ForgetContent.reason (limit string length) in a follow-up PR, or remove these constants if they won't be used soon.

Comment thread aleph_message/models/__init__.py Outdated
@odesenfans odesenfans merged commit 41c3b7c into main Apr 21, 2026
10 checks passed
@odesenfans odesenfans deleted the feat/cap-channel-length branch April 21, 2026 22:02
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.

2 participants