Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cosmwasm: added shutdown contracts tests #4257

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kakucodes
Copy link
Collaborator

@kakucodes kakucodes commented Feb 3, 2025

Description

This PR adds comprehensive test coverage for the shutdown cw-wormhole contract, which serves as a safety mechanism for temporarily disabling the main contract in case of security incidents. The changes include both interchain integration tests and cw-multi-test based tests.

Changes

  • Added interchain integration tests validating disabled functionality post-shutdown
  • Implemented cw-multi-test based unit tests for the shutdown contract

Purpose

This test coverage addition is a preliminary step before upcoming modifications to the cw-wormhole contract's state management. Having thorough test coverage in place will help ensure the safety and correctness of the future state management changes that are needed.

Test Scenarios

  • Contract migration to shutdown state
  • Verification of disabled message handlers
  • State preservation during shutdown

This pr was created jointly with @joelsmith-2019 where he contributed the ICT tests and I focused on the cw-multi-test

@kakucodes kakucodes force-pushed the chore/add-shutdown-cw-wormhole-testing branch from b902cc2 to daedbab Compare February 3, 2025 20:13
@kakucodes kakucodes marked this pull request as ready for review February 3, 2025 20:55
joelsmith-2019
joelsmith-2019 previously approved these changes Feb 3, 2025
Copy link
Collaborator

@joelsmith-2019 joelsmith-2019 left a comment

Choose a reason for hiding this comment

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

Only reviewed the CosmWasm tests as I was responsible for writing the interchain tests.

@kakucodes kakucodes force-pushed the chore/add-shutdown-cw-wormhole-testing branch from daedbab to 7733aca Compare February 10, 2025 20:09
Copy link
Collaborator

@joelsmith-2019 joelsmith-2019 left a comment

Choose a reason for hiding this comment

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

Re-approving the CosmWasm tests

Copy link
Collaborator

@djb15 djb15 left a comment

Choose a reason for hiding this comment

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

Could you just confirm the reason for adding the cfg_attr lines in the source files? I'm assuming it's to prevent the compiler from optimising those functions out for the tests?

@kakucodes
Copy link
Collaborator Author

kakucodes commented Feb 13, 2025

Could you just confirm the reason for adding the cfg_attr lines in the source files? I'm assuming it's to prevent the compiler from optimising those functions out for the tests?

Hey @djb15, the cfg_attrs are to quell the lint/clippy warnings that were arising when running the tests with no default features where those code paths are unused.

Copy link
Collaborator

@djb15 djb15 left a comment

Choose a reason for hiding this comment

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

Tests look good! Also confirmed that cfg_attr does what's intended (adding allow(dead_code) when default features disabled for testing)

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