Skip to content

Conversation

@RedaRahmani
Copy link

Title

Enforce Activated status before suspend + add negative tests

Summary

This PR makes the suspend logic in the serviceability program a bit stricter and more consistent.

Right now, some *_suspend handlers don’t check the current status of the entity before allowing a suspend. That means it’s possible to try to suspend something that is still Pending or already Suspended, while link/suspend.rs already enforces that only Activated entities can be suspended.

Here I align all suspend handlers with that same rule and add one negative test per entity type to make sure we don’t regress in the future.

This addresses #2221.

What changed

  1. Enforce Activated before suspend

For the following handlers:

location/suspend.rs

exchange/suspend.rs

contributor/suspend.rs

device/suspend.rs

user/suspend.rs

multicastgroup/suspend.rs

I now:

Deserialize the entity

Check that entity.status == EntityStatus::Activated

Return DoubleZeroError::InvalidStatus otherwise (with a small debug log under #[cfg(test)])

The check follows the same pattern already used in link/suspend.rs:

if entity.status != EntityStatus::Activated {
    #[cfg(test)]
    msg!("{:?}", entity);
    return Err(DoubleZeroError::InvalidStatus.into());
}

This doesn’t change behaviour for valid suspends (entities already Activated). It just tightens validation when the status is not valid for a suspend.

  1. New negative tests

I added one negative test per entity type to make the new checks explicit and guarded by tests:

test_suspend_location_from_suspended_fails

test_suspend_exchange_from_suspended_fails

test_suspend_contributor_from_suspended_fails

test_suspend_device_from_pending_fails

test_suspend_user_from_suspended_fails

test_suspend_multicastgroup_from_pending_fails

Each test:

Puts the entity in an invalid state for suspend (Pending or Suspended depending on the case),

Calls the corresponding *_suspend processor,

Asserts that it fails with DoubleZeroError::InvalidStatus.

This way each handler’s status check is covered and future changes will be forced to keep this behaviour.

  1. Fix in device activation test setup

While working on the tests, I also fixed the account list passed to device activation.

device/activate expects:

device

globalstate

payer

system_program

The previous test setup was missing some of these, so it didn’t really mirror the on-chain invocation. I updated the test to provide the full, correct account list.

This doesn’t change behaviour, but it makes the test more realistic and closer to what actually happens at runtime.

Testing

From the smartcontract workspace:

# During development: specific suspend tests
cargo test -p doublezero-serviceability --test user_tests test_suspend --test-threads=1

# All suspend-related negative tests
cargo test -p doublezero-serviceability "test_suspend.*fails" --test-threads=1

# Full test suite for the serviceability crate
cargo test -p doublezero-serviceability

# Lint
cargo clippy -p doublezero-serviceability

Results:

All 6 new negative tests pass

All existing tests in doublezero-serviceability pass

cargo clippy clean (no new warnings)

@RedaRahmani RedaRahmani force-pushed the fix/suspend-only-when-activated-2221 branch from d3884fd to e45c263 Compare December 11, 2025 20:03
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.

1 participant