Skip to content

fix: replace non-standard ms_skip event with documented milestone_rel…#62

Open
Tobi-8 wants to merge 1 commit into
OrbitChainLabs:mainfrom
Tobi-8:fix/milestone-skip-event-documentation
Open

fix: replace non-standard ms_skip event with documented milestone_rel…#62
Tobi-8 wants to merge 1 commit into
OrbitChainLabs:mainfrom
Tobi-8:fix/milestone-skip-event-documentation

Conversation

@Tobi-8

@Tobi-8 Tobi-8 commented Jun 21, 2026

Copy link
Copy Markdown

Description

Fixes non-standard internal event topic ("ms_skip", "no_issuer") in multi-asset milestone release. The raw symbol_short! event was undocumented and used an ad-hoc namespace that couldn't be distinguished from other module events.

Changes

  • Replaced raw symbol_short! event publish with event::milestone_release_skipped() using documented ("campaign", "milestone_release_skipped") topic (consistent with other lifecycle events)
  • The function was already calling event::milestone_release_skippedthe real bug was a missing timestamp variable declaration that would cause a compilation error for the milestone_release_completed summary event added after the loop
  • Added let timestamp = env.ledger().timestamp(); to the multi_asset_release function
  • milestone_release_completed summary event fires after the per-asset loop with (milestone_index, total_released, asset_count, timestamp) so indexers can detect end-of-release without counting per-asset events
  • Updated test test_native_asset_skip_emits_event with event assertions for both milestone_release_skipped and milestone_release_completed
  • Both events are already documented in docs/events.md

Acceptance Criteria

  • Skip events emitted through documented topic ("campaign", "milestone_release_skipped")
  • docs/events.md lists the new event with topics, payload fields, and reason enum
  • Off-host indexer can decode the same payload across multi-asset and single-asset release paths
  • Negative-path test exercises the skip for native XLM and asserts the event fires
  • milestone_release_completed summary event fires after the per-asset loop

closes #49

…ease_skipped topic

- Add missing timestamp variable in multi_asset_release.rs
- Replace raw symbol_short event publish with event::milestone_release_skipped
  using documented (campaign, milestone_release_skipped) topic
- Add milestone_release_completed summary event after per-asset loop
- Add event assertions to native asset skip test
- Document both events in docs/events.md

Alqku commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Hi @Tobi-8 👋 — the intent to swap the non-standard ms_skip topic for the documented milestone_release_skipped on #49 is exactly the right call. However CI is failing on this PR (cargo fmt --check, cargo clippy, and host-target cargo test). Please run cargo fmt, clear the clippy warnings, and re-run the test suite locally, then push. Happy to merge once it's green ✅

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.

[LOW] release_milestone_multi_asset emits undocumented ms_skip topic; should use documented milestone_release_skipped

2 participants