Skip to content

Conversation

@odesenfans
Copy link
Collaborator

@odesenfans odesenfans commented May 16, 2022

Added new Pydantic models/schemas to validate Aleph messages
coming from the network. These models are similar to the ones
implemented in aleph-message, but focus on validating messages
before processing them.

This validation replaces the validation in check_message
and will later be used to use objects as early as possible
in the processing of messages.

Added tests for the major types of messages.

Breaking changes (to validate):

  • the item_type field is now mandatory for all message types
  • the address and time fields are now mandatory for the content of all messages.
  • PROGRAM: allow_amend and code.comment are now mandatory.

@odesenfans odesenfans marked this pull request as draft May 16, 2022 20:45
@odesenfans
Copy link
Collaborator Author

Depends on #271.

@odesenfans odesenfans force-pushed the od-improve-message-validation branch from 23520c3 to c8cddfe Compare May 16, 2022 21:23
@odesenfans odesenfans added this to the May release milestone May 17, 2022
@odesenfans odesenfans force-pushed the od-improve-message-validation branch 2 times, most recently from 0aebd56 to e536c4b Compare May 18, 2022 08:41
@odesenfans odesenfans self-assigned this May 23, 2022
@odesenfans odesenfans force-pushed the od-improve-message-validation branch from e536c4b to fd488ae Compare May 24, 2022 10:00
@odesenfans odesenfans assigned hoh and unassigned odesenfans May 24, 2022
@odesenfans odesenfans requested a review from hoh May 24, 2022 10:02
@odesenfans odesenfans marked this pull request as ready for review May 24, 2022 10:02
message["item_type"] = item_type_from_hash(message["item_hash"]).value
except ValueError as error:
LOGGER.warning(error)
_ = parse_message(message)
Copy link
Member

Choose a reason for hiding this comment

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

Not a fan of _ =

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Me neither, but it's to show that I'm not ignoring the return value until this line goes away in #277.


return values

@root_validator()
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be a root validator instead of a simple validator ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, if only because we're accessing other fields. The main problem is that check_item_hash uses item_type and check_item_type uses item_hash, so at least one of the functions has to be a root validator.

Comment on lines +101 to +124
@validator("item_hash")
def check_item_hash(cls, v, values):
"""
For inline item types, check that the item hash is equal to
the hash of the item content.
"""

item_type = values["item_type"]
if item_type == ItemType.inline:
item_content: str = values["item_content"]

computed_hash: str = sha256(item_content.encode()).hexdigest()
if v != computed_hash:
raise ValueError(
"'item_hash' do not match 'sha256(item_content)'"
f", expecting {computed_hash}"
)
elif item_type == ItemType.ipfs:
# TODO: CHeck that the hash looks like an IPFS multihash
pass
else:
assert item_type == ItemType.storage
return v
Copy link
Member

Choose a reason for hiding this comment

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

This code is identical to the one in aleph-message. Can we do something like this, or do the decorators get in the way ?

Suggested change
@validator("item_hash")
def check_item_hash(cls, v, values):
"""
For inline item types, check that the item hash is equal to
the hash of the item content.
"""
item_type = values["item_type"]
if item_type == ItemType.inline:
item_content: str = values["item_content"]
computed_hash: str = sha256(item_content.encode()).hexdigest()
if v != computed_hash:
raise ValueError(
"'item_hash' do not match 'sha256(item_content)'"
f", expecting {computed_hash}"
)
elif item_type == ItemType.ipfs:
# TODO: CHeck that the hash looks like an IPFS multihash
pass
else:
assert item_type == ItemType.storage
return v
check_item_hash = aleph_message.models.BaseMessage.check_item_hash
Suggested change
@validator("item_hash")
def check_item_hash(cls, v, values):
"""
For inline item types, check that the item hash is equal to
the hash of the item content.
"""
item_type = values["item_type"]
if item_type == ItemType.inline:
item_content: str = values["item_content"]
computed_hash: str = sha256(item_content.encode()).hexdigest()
if v != computed_hash:
raise ValueError(
"'item_hash' do not match 'sha256(item_content)'"
f", expecting {computed_hash}"
)
elif item_type == ItemType.ipfs:
# TODO: CHeck that the hash looks like an IPFS multihash
pass
else:
assert item_type == ItemType.storage
return v
@validator("item_hash")
def check_item_hash(cls, v, values):
return aleph_message.models.BaseMessage.check_item_hash(v, values)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just tried it, the validator does not get in the way but I removed the hash_type member of the message model, if only because we will probably never use it (better to use multihash/multicodec instead). If that's okay for you, I think I'll leave it like that for the moment, and we'll do a large update of aleph_message once we're fixed on the models.

I basically want to have a class tree like this:

  • PendingMessage: base class, = the data sent by aleph-client or similar
  • DbMessage: inherits PendingMessage (or a common base class), = the schema of the data stored in MongoDB.
  • ApiMessage: inherits DbMessage (or a common base class), = the schema of the API response. Similar to the current aleph_message Message.

@hoh hoh assigned odesenfans and unassigned hoh May 24, 2022
@odesenfans odesenfans assigned hoh and unassigned odesenfans May 24, 2022
Added new Pydantic models/schemas to validate Aleph messages
coming from the network. These models are similar to the ones
implemented in aleph-message, but focus on validating messages
before processing them.

This validation replaces the validation in check_message
and will later be used to use objects as early as possible
in the processing of messages.

Added tests for the major types of messages.
@odesenfans odesenfans force-pushed the od-improve-message-validation branch from 1a85141 to d29670f Compare May 25, 2022 09:32
@odesenfans odesenfans merged commit 53e2b42 into aleph-im:dev May 25, 2022
@odesenfans odesenfans deleted the od-improve-message-validation branch May 25, 2022 09:39
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