Skip to content

Conversation

morph-dev
Copy link
Collaborator

@morph-dev morph-dev commented May 30, 2025

What was wrong?

When we are offered multiple content, the overlay service does all of the following on per-content basis:

  • checks whether they should be accepted
  • validates them
  • stores them

This doesn't work for EphemeralHeaders, which we have to process together (at least checking which content to accept and validating). This PR adds support for that.

How was it fixed?

Added should_we_store_batch function that checks whether multiple content items should be stored.

Refactored Validator trait in the following ways:

  • removed additional_content_to_propagate from ValidationResult (it's not used)
  • made ValidationResult an enum
    • we can consider changing Invalid(String) variant into more descriptive type, e.g. Invalid(ValidationFailureReason) where:
      enum ValidationFailureReason {
          ContentValueNotDecodable,
          Invalid(<reason?>),  // e.g. merkle proof not valid
          FetchingAnchorContentFailuer(<reason?>), // e.g. failed to fetch Header when verifying BlockBody
          ...
      }
  • added support for validating multiple content items

Refactored OverlayService::handle_offer to use these functions instead

Remaining work

We would still do a fallback fetching in request fails or content is not validated.
I would have to think more whether this is desired for EphemeralContent, and if not, how exactly to distinguish/handle this case. This is something that I plan to do in a followup PR.

Modify history Store and Validator to actually implement custom logic for handling multiple content items (also in the followup PR).

@morph-dev morph-dev requested review from KolbyML and carver May 30, 2025 01:11
@morph-dev morph-dev self-assigned this May 30, 2025
@morph-dev
Copy link
Collaborator Author

morph-dev commented May 30, 2025

If desired, I could probably split this into 2 smaller PRs:

  • refactor Validator and ValidationResult
    • most of these changes are simple (but not trivial) where instead of returning anyhow::Error, we return ValidationResult::Invalid(...), but they affect a lot of lines of code
  • changes in portalnet (i.e. offer.rs) to actually use functions for accepting and validating multiple content together

@KolbyML
Copy link
Member

KolbyML commented May 30, 2025

If desired, I could probably split this into 2 smaller PRs:

  • refactor Validator and ValidationResult

    • most of these changes are simple (but not trivial) where instead of returning anyhow::Error, we return ValidationResult::Invalid(...), but they affect a lot of lines of code
  • changes in portalnet (i.e. offer.rs) to actually use functions for accepting and validating multiple content together

I am fine reviewing this in one PR no worries. I will probably do it when I wake up tomorrow

@morph-dev
Copy link
Collaborator Author

Closing in favor of #1869 and #1870

@morph-dev morph-dev closed this May 30, 2025
@morph-dev morph-dev deleted the batch_validate branch May 31, 2025 21:58
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