Skip to content

Conversation

@horologger
Copy link
Contributor

Fix panics caused by data inconsistency from Expired space revocations

Summary

This PR fixes two related panics that occurred when interacting with spaces that were revoked due to expiration. Both issues stemmed from the same root cause: data inconsistency where RevokeReason::Expired only removed the Outpoint->Spaceout mapping but left the Space->Outpoint mapping, creating orphaned entries.

Issues Fixed

1. Panic when querying revoked spaces (getspace command)

Location: client/src/store.rs and client/src/client.rs

  • Problem: When querying a space that was revoked, get_space_info would panic with "should exist if outpoint exists" because the Space->Outpoint mapping existed but the Spaceout was missing.
  • Root cause: RevokeReason::Expired handler only removed Outpoint->Spaceout mapping but didn't remove Space->Outpoint mapping.
  • Fix:
    • Added removal of Space->Outpoint mapping in Expired revocation handler
    • Made get_space_info handle inconsistencies gracefully by returning None and cleaning up orphaned mappings instead of panicking

2. Panic in open subcommand (spaces open)

Location: protocol/src/script.rs

  • Problem: Opening a space that was revoked would panic with "spaceout exists" when the outpoint existed but spaceout was missing.
  • Root cause: Same data inconsistency - orphaned Space->Outpoint mapping from Expired revocations.
  • Fix: Replaced .expect("spaceout exists") with proper match statement to handle None case gracefully, treating missing spaceout as a new space (since the space was effectively revoked/unregistered).

Changes

  • client/src/client.rs: Fixed RevokeReason::Expired to remove both Space->Outpoint and Outpoint->Spaceout mappings
  • client/src/store.rs: Added graceful handling of data inconsistencies in get_space_info with automatic cleanup
  • protocol/src/script.rs: Fixed prepare_open to handle missing spaceout gracefully instead of panicking

Testing

Both fixes have been tested and verified:

  • getspace command now returns gracefully instead of panicking
  • spaces open command now handles revoked spaces correctly instead of panicking

Impact

  • Breaking changes: None
  • Backward compatibility: Maintained - fixes handle both new inconsistencies (prevented) and existing inconsistent data (cleaned up gracefully)

Fix data inconsistency in RevokeReason::Expired handling that caused
panics when querying spaces that were revoked due to expiration.

Root cause: Expired revocations only removed Outpoint->Spaceout mapping
but left Space->Outpoint mapping, creating inconsistent state.

Changes:
- Remove Space->Outpoint mapping in Expired revocation handler
- Handle inconsistencies gracefully in get_space_info by returning None
  and cleaning up orphaned Space->Outpoint mappings instead of panicking
Fix data inconsistency handling in prepare_open that caused panics
when opening spaces that were revoked due to expiration.

Root cause: When an outpoint exists but spaceout doesn't (due to
inconsistent state from Expired revocations), the code would panic
with 'spaceout exists' instead of handling it gracefully.

Changes:
- Replace expect() with match statement to handle None case
- Treat missing spaceout as new space (space was revoked, so it's
  effectively not registered anymore)
Copy link
Member

@buffrr buffrr left a comment

Choose a reason for hiding this comment

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

Great fix thanks!

@buffrr buffrr merged commit c64626b into spacesprotocol:main Dec 13, 2025
1 check passed
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