diff --git a/.github/workflows/validate-contract-events.yml b/.github/workflows/validate-contract-events.yml index 7f1d91be..f5411519 100644 --- a/.github/workflows/validate-contract-events.yml +++ b/.github/workflows/validate-contract-events.yml @@ -4,13 +4,19 @@ on: pull_request: paths: - "contract/**/events.rs" + - "contract/**/*.rs" - "docs/CONTRACT_EVENTS.md" - "scripts/validate-contract-events.py" + - "scripts/check_contract_event_conventions.py" + - "scripts/test_contract_event_conventions.py" push: branches: [main] paths: - "contract/**/events.rs" + - "contract/**/*.rs" - "docs/CONTRACT_EVENTS.md" + - "scripts/check_contract_event_conventions.py" + - "scripts/test_contract_event_conventions.py" concurrency: group: event-docs-${{ github.ref }} @@ -36,3 +42,8 @@ jobs: - name: Validate Contract Event Documentation Drift run: | python3 scripts/validate-contract-events.py + + - name: Test contract event naming and topic conventions + run: | + python3 -m unittest discover -s scripts -p 'test_contract_event_conventions.py' + python3 scripts/check_contract_event_conventions.py diff --git a/docs/CONTRACT_EVENTS.md b/docs/CONTRACT_EVENTS.md index 3f2c3a5c..b1015d70 100644 --- a/docs/CONTRACT_EVENTS.md +++ b/docs/CONTRACT_EVENTS.md @@ -194,3 +194,18 @@ Run the documentation drift validation script: ```bash python3 scripts/validate-contract-events.py ``` + +## 4. Naming and Topic Conventions + +- Declare event structs with unique `PascalCase` names. Soroban exposes the generated event name in `snake_case`. +- Use `snake_case` for event fields. +- Mark indexed fields with `#[topic]` and place all indexed fields before unindexed data fields. +- Declare no more than four indexed fields per event. +- When both `category` and `priority` are indexed, keep them as the final two topics in that order. + +Run the naming and topic convention tests and validator: + +```bash +python3 -m unittest discover -s scripts -p 'test_contract_event_conventions.py' +python3 scripts/check_contract_event_conventions.py +``` diff --git a/scripts/__pycache__/check_contract_event_conventions.cpython-314.pyc b/scripts/__pycache__/check_contract_event_conventions.cpython-314.pyc new file mode 100644 index 00000000..bf85cf23 Binary files /dev/null and b/scripts/__pycache__/check_contract_event_conventions.cpython-314.pyc differ diff --git a/scripts/__pycache__/test_contract_event_conventions.cpython-314.pyc b/scripts/__pycache__/test_contract_event_conventions.cpython-314.pyc new file mode 100644 index 00000000..e538d18b Binary files /dev/null and b/scripts/__pycache__/test_contract_event_conventions.cpython-314.pyc differ diff --git a/scripts/check_contract_event_conventions.py b/scripts/check_contract_event_conventions.py new file mode 100644 index 00000000..98f5fd7c --- /dev/null +++ b/scripts/check_contract_event_conventions.py @@ -0,0 +1,142 @@ +#!/usr/bin/env python3 +"""Validate Soroban contract event names and indexed topic layouts.""" + +import re +import sys +from dataclasses import dataclass +from pathlib import Path + + +EVENT_PATTERN = re.compile( + r"#\[contractevent(?:[^\]]*)\]\s*(?:#\[derive[^\]]*\]\s*)*" + r"pub struct\s+([A-Za-z_][A-Za-z0-9_]*)\s*\{([^}]*)\}" +) +FIELD_PATTERN = re.compile(r"pub\s+([A-Za-z_][A-Za-z0-9_]*)\s*:") +EVENT_NAME_PATTERN = re.compile(r"^[A-Z][a-zA-Z0-9]*$") +FIELD_NAME_PATTERN = re.compile(r"^[a-z][a-z0-9]*(?:_[a-z0-9]+)*$") +COMMENT_PATTERN = re.compile(r"//[^\n]*|/\*[\s\S]*?\*/") +MAX_TOPICS = 4 + + +@dataclass(frozen=True) +class ContractEvent: + name: str + fields: tuple[tuple[str, bool], ...] + path: str + line: int + + +def extract_contract_events(source: str, path: str = "") -> list[ContractEvent]: + source_without_comments = COMMENT_PATTERN.sub( + lambda match: "\n" * match.group(0).count("\n"), source + ) + events = [] + + for match in EVENT_PATTERN.finditer(source_without_comments): + body = match.group(2) + fields = [] + topic_pending = False + for line in body.splitlines(): + stripped = line.strip() + if stripped == "#[topic]": + topic_pending = True + continue + + field_match = FIELD_PATTERN.search(stripped) + if field_match: + fields.append((field_match.group(1), topic_pending)) + topic_pending = False + + events.append( + ContractEvent( + name=match.group(1), + fields=tuple(fields), + path=path, + line=source_without_comments.count("\n", 0, match.start()) + 1, + ) + ) + + return events + + +def validate_event_definitions(events: list[ContractEvent]) -> list[str]: + errors = [] + seen_names = {} + + for event in events: + location = f"{event.path}:{event.line}" + if not EVENT_NAME_PATTERN.fullmatch(event.name): + errors.append( + f"{location}: event {event.name!r} must use PascalCase " + "(for example, NotificationScheduled)" + ) + + previous = seen_names.get(event.name) + if previous: + errors.append( + f"{location}: event name {event.name!r} is also defined at {previous}; " + "rename one event to keep generated event names unique" + ) + else: + seen_names[event.name] = location + + topic_names = [name for name, is_topic in event.fields if is_topic] + if len(topic_names) > MAX_TOPICS: + errors.append( + f"{location}: event {event.name} declares {len(topic_names)} indexed fields; " + f"Soroban contract events support at most {MAX_TOPICS} (excluding the event name)" + ) + + data_field_seen = False + for name, is_topic in event.fields: + if not FIELD_NAME_PATTERN.fullmatch(name): + errors.append( + f"{location}: field {name!r} in event {event.name} must use snake_case" + ) + if is_topic and data_field_seen: + errors.append( + f"{location}: topic field {name!r} in event {event.name} appears after a data field; " + "move all #[topic] fields before unindexed fields" + ) + if not is_topic: + data_field_seen = True + + if "priority" in topic_names and ( + len(topic_names) < 2 or topic_names[-2:] != ["category", "priority"] + ): + errors.append( + f"{location}: event {event.name} indexes priority without category immediately before it; " + "place category and priority as the final two topics" + ) + + return errors + + +def main() -> int: + repo_root = Path(__file__).resolve().parent.parent + contract_root = repo_root / "contract" + events = [] + + for source_path in sorted(contract_root.rglob("*.rs")): + relative_path = source_path.relative_to(repo_root).as_posix() + events.extend( + extract_contract_events(source_path.read_text(encoding="utf-8"), relative_path) + ) + + if not events: + print(f"No #[contractevent] definitions found under {contract_root}", file=sys.stderr) + return 1 + + errors = validate_event_definitions(events) + if errors: + print("Contract event convention violations:", file=sys.stderr) + for error in errors: + print(f" - {error}", file=sys.stderr) + return 1 + + print(f"Validated naming and topic conventions for {len(events)} contract events.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) \ No newline at end of file diff --git a/scripts/test_contract_event_conventions.py b/scripts/test_contract_event_conventions.py new file mode 100644 index 00000000..2535d19d --- /dev/null +++ b/scripts/test_contract_event_conventions.py @@ -0,0 +1,102 @@ +import unittest + +from check_contract_event_conventions import ( + extract_contract_events, + validate_event_definitions, +) + + +class ContractEventConventionTests(unittest.TestCase): + def test_valid_event_name_and_four_topics_pass(self): + source = """ +#[contractevent] +pub struct NotificationScheduled { + #[topic] + pub creator: Address, + #[topic] + pub notification_id: BytesN<32>, + #[topic] + pub category: NotificationCategory, + #[topic] + pub priority: NotificationPriority, + pub payload_version: u32, +} +""" + events = extract_contract_events(source, "fixture.rs") + + self.assertEqual(validate_event_definitions(events), []) + + def test_pascal_case_and_snake_case_are_required(self): + source = """ +#[contractevent] +pub struct notification_scheduled { + #[topic] + pub creatorAddress: Address, +} +""" + errors = validate_event_definitions(extract_contract_events(source, "fixture.rs")) + + self.assertTrue(any("must use PascalCase" in error for error in errors)) + self.assertTrue(any("must use snake_case" in error for error in errors)) + + def test_topics_must_precede_data_fields(self): + source = """ +#[contractevent] +pub struct NotificationScheduled { + pub payload_version: u32, + #[topic] + pub creator: Address, +} +""" + errors = validate_event_definitions(extract_contract_events(source, "fixture.rs")) + + self.assertTrue(any("appears after a data field" in error for error in errors)) + + def test_too_many_topics_are_reported(self): + source = """ +#[contractevent] +pub struct TooManyTopics { + #[topic] + pub first: Address, + #[topic] + pub second: Address, + #[topic] + pub third: Address, + #[topic] + pub fourth: Address, + #[topic] + pub fifth: Address, +} +""" + errors = validate_event_definitions(extract_contract_events(source, "fixture.rs")) + + self.assertTrue(any("at most 4" in error for error in errors)) + + def test_priority_requires_category_as_the_previous_topic(self): + source = """ +#[contractevent] +pub struct PriorityEvent { + #[topic] + pub creator: Address, + #[topic] + pub priority: NotificationPriority, +} +""" + errors = validate_event_definitions(extract_contract_events(source, "fixture.rs")) + + self.assertTrue(any("final two topics" in error for error in errors)) + + def test_duplicate_event_names_are_reported(self): + source = """ +#[contractevent] +pub struct DuplicateEvent { pub id: u32 } +#[contractevent] +pub struct DuplicateEvent { pub id: u64 } +""" + errors = validate_event_definitions(extract_contract_events(source, "fixture.rs")) + + self.assertTrue(any("also defined at" in error for error in errors)) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file