Skip to content

Conversation

@shahnami
Copy link
Member

@shahnami shahnami commented Jan 26, 2026

Summary

Testing Process

Checklist

  • Add a reference to related issues in the PR description.
  • Add unit tests if applicable.
  • Add integration tests if applicable.
  • Add property-based tests if applicable.
  • Update documentation if applicable.

Note

If you are using Monitor in your stack, consider adding your team or organization to our list of Monitor Users in the Wild!

Summary by CodeRabbit

  • New Features

    • Signature validation now enforces blockchain-specific formatting rules (parentheses required for EVM, Stellar, and Midnight; optional for Solana).
  • Improvements

    • Reorganized monitor validation flow for improved error reporting.
    • Added clear blockchain type names for better readability.

✏️ Tip: You can customize this high-level summary in your review settings.

@shahnami shahnami requested a review from a team as a code owner January 26, 2026 16:36
@coderabbitai
Copy link

coderabbitai bot commented Jan 26, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This change refactors signature validation by introducing blockchain-aware signature rules and relocating validation logic from the config module to the repository layer. A new SignatureRules struct determines signature format requirements per blockchain type, enabling network-specific validation during monitor reference verification.

Changes

Cohort / File(s) Summary
Blockchain Type Signature Rules
src/models/blockchain/mod.rs
Adds SignatureRules struct with requires_parentheses field, implements signature_rules() method on BlockChainType (returns true for EVM/Stellar/Midnight, false for Solana), and adds Display trait for human-readable blockchain names with tests.
Validation Logic Migration
src/models/config/monitor_config.rs
Removes signature format validation that previously enforced parentheses in function and event signatures for non-Solana monitors.
Monitor Repository Validation
src/repositories/monitor.rs
Adds private validate_monitor_signatures() helper that validates monitor signatures against network-specific signature rules, integrates validation into validate_monitor_references() flow, and includes comprehensive tests covering EVM/Solana signature validation scenarios.
Test Coverage Adjustments
tests/properties/repositories/monitor.rs
Removes two test scenarios that validated invalid function/event signatures in monitor config.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • feat: Add Solana #408 — Solana integration that introduces blockchain-specific signature handling and monitor validation logic coordinated with this refactoring.

Suggested labels

cla: signed

Suggested reviewers

  • NicoMolinaOZ

Poem

🐰 From config's grip, validation takes a hop,
To repository layer, where blockchain rules pop!
Solana says "no," while Stellar says "yes,"
Signatures now dance in their rightful address. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description uses the template structure but lacks substance. The Summary and Testing Process sections are present but entirely empty, providing no details about the changes, rationale, or testing performed. Fill in the Summary section with a detailed explanation of the changes made, including what was refactored and why. Describe the testing process and indicate which checklist items were completed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: improving signature validation logic specific to Solana. It clearly summarizes the primary objective of refactoring validation across the blockchain models, config, and repository layers.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-hardcoded-solana-prefix

Comment @coderabbitai help to get the list of available commands and usage tips.

@shahnami shahnami linked an issue Jan 26, 2026 that may be closed by this pull request
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/repositories/monitor.rs`:
- Around line 65-104: The current validate_monitor_signatures function only
checks presence of '(' and ')' which allows malformed orders like
"transfer)address("; update the function and both validation loops (the checks
for func.signature and event.signature) to ensure the '(' appears before the
')', the signature ends with ')' (signature.ends_with(')')), and that there is
at least one non-empty identifier before '(' and at least zero chars between
parentheses (i.e., let open_idx = signature.find('('), close_idx =
signature.rfind(')') and require Some(open) < Some(close) and open > 0); when
any of these fail, push the same formatted validation_errors message for the
offending func.signature or event.signature.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Jan 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.5%. Comparing base (9ace8f2) to head (3c783e7).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff           @@
##            main    #429    +/-   ##
======================================
  Coverage   95.5%   95.5%            
======================================
  Files         96      96            
  Lines      28100   28272   +172     
======================================
+ Hits       26837   27013   +176     
+ Misses      1263    1259     -4     
Flag Coverage Δ
integration 60.0% <47.2%> (-0.1%) ⬇️
properties 26.7% <61.8%> (+0.1%) ⬆️
unittests 86.2% <100.0%> (+0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix hardcoded solana_ prefix in monitor config validation

2 participants