Skip to content

test(subscription): verify cancel then re-subscribe replaces stale st…#552

Merged
nanaf6203-bit merged 1 commit into
MettaChain:mainfrom
FaveTeamz:fix/189-cancel-and-resubscribe
Jun 25, 2026
Merged

test(subscription): verify cancel then re-subscribe replaces stale st…#552
nanaf6203-bit merged 1 commit into
MettaChain:mainfrom
FaveTeamz:fix/189-cancel-and-resubscribe

Conversation

@FaveTeamz

Copy link
Copy Markdown
Contributor

…orage (#189)

  • Add test_cancel_and_resubscribe_replaces_record: subscribes with amount=50/interval=600, cancels, resubscribes with amount=99/ interval=1200 and asserts new record has updated values with no stale state from the first subscription
  • Assert get_subscription returns None immediately after cancel
  • Add test_cancel_nonexistent_subscription_returns_error (negative case)
  • Add test_double_subscribe_returns_error (guard test)
  • Register contracts/subscription in workspace Cargo.toml

closes #189

…orage (MettaChain#189)

- Add test_cancel_and_resubscribe_replaces_record: subscribes with
  amount=50/interval=600, cancels, resubscribes with amount=99/
  interval=1200 and asserts new record has updated values with no
  stale state from the first subscription
- Assert get_subscription returns None immediately after cancel
- Add test_cancel_nonexistent_subscription_returns_error (negative case)
- Add test_double_subscribe_returns_error (guard test)
- Register contracts/subscription in workspace Cargo.toml

Copy link
Copy Markdown
Contributor

Review: test(subscription): verify cancel then re-subscribe replaces stale subscription

Nice regression test - cancel-then-resubscribe is exactly the kind of state-machine corner case that gets reintroduced.

Hardening suggestions:

  1. Boundary coverage. Tests rely on the now < sub.next_payment comparison inside execute_payment. There is no test for the exact boundary (now == sub.next_payment). Please add one - I want the upper-bound semantics pinned down by a test so a future refactor does not flip them silently.
  2. Hardcoded ledger timestamps. Fixed sequences (100, 101, ...) are fine and deterministic, but please add a top-of-module comment mapping the sequences to intent (ledger 100 = baseline subscribe, ledger 101 = cancel, etc.) so the next contributor can decode the intent without re-tracing the diff.
  3. CI status heads-up. I see 12 of 14 checks failing on this branch (build WASM, clippy, rustfmt, security audit, several cargo test runs - contracts/fractional, contracts/insurance, contracts/lending, contracts/oracle). Note that the new test is in contracts/subscription which builds green per the workspace test, so the cross-contract failures appear to be baseline issues on the branch rather than caused by this change - flagging in case you want to confirm before rebase.

@nanaf6203-bit nanaf6203-bit merged commit bd245e7 into MettaChain:main Jun 25, 2026
2 of 14 checks 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.

Property Token: Add batch transfer functionality

2 participants