Skip to content

Conversation

@mariyade
Copy link
Contributor

@mariyade mariyade commented Dec 11, 2025

Summary:

  • added Zswap ledger event subscription coverage in integration and e2e tests
  • validated correct ordering between Zswap events and the subsequent DustSpendProcessed event after shielded transaction
  • improved logging in Dust ledger event collection function for better debugging
  • updated Dust ledger event subscription tests to validate ledger order rather than contiguous order
  • increased timeout in subscribeToUnshieldedTransactionEvents from 500ms → 5000ms to reduce flakiness
  • added validation for shielded transactions to confirm no unshielded outputs or fees are exposed
  • re-enabled the 'dust-queries.test' test following completion of PM-20789

@mariyade mariyade marked this pull request as ready for review December 12, 2025 13:37
@mariyade mariyade requested a review from a team as a code owner December 12, 2025 13:37
@mariyade mariyade force-pushed the test/zswap-subscriptions-tests branch from 7672649 to a2f08a6 Compare December 15, 2025 08:46
@mariyade mariyade force-pushed the test/zswap-subscriptions-tests branch from a2f08a6 to 9717d44 Compare December 17, 2025 16:22
Copy link
Contributor

@THErealARETE THErealARETE left a comment

Choose a reason for hiding this comment

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

  1. Good Test Coverage: Comprehensive integration and e2e tests for zswap subscriptions
  2. Clear Documentation: Test descriptions follow Given/When/Then format with good comments
  3. Consistent Patterns: Zswap utilities follow the same pattern as dust utilities
  4. Ordering Validation: Proper validation of event ordering between Zswap and Dust events
  5. Improved Logging: Added debug logging for better debugging capabilities
  6. Proper Cleanup: Tests properly unsubscribe and close connections
  7. Good Refactoring: Renaming dust-utils.ts to dust-ledger-utils.ts clarifies purpose

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.

3 participants