Skip to content

Conversation

@codomposer
Copy link
Contributor

@codomposer codomposer commented Dec 8, 2025

Description of the Change

This PR adds comprehensive unit tests for the async MEV Shield extrinsics module (bittensor/core/extrinsics/asyncex/mev_shield.py).

New test file: tests/unit_tests/extrinsics/asyncex/test_mev_shield.py

Tests implemented:

  • test_submit_encrypted_extrinsic_success - Verifies successful encryption and submission with commitment and ciphertext generation
  • test_submit_encrypted_extrinsic_sign_with_hotkey - Verifies correct signer selection when using hotkey
  • test_wait_for_extrinsic_by_hash_success - Verifies successful extrinsic discovery in subsequent blocks
  • test_wait_for_extrinsic_by_hash_decryption_failure - Verifies markDecryptionFailed event detection
  • test_wait_for_extrinsic_by_hash_timeout - Verifies None return when extrinsic not found within timeout
  • test_submit_encrypted_extrinsic_invalid_signer - Verifies AttributeError raised for invalid sign_with parameter

Alternate Designs

N/A - This is a test implementation following existing pytest patterns in the codebase.

Possible Drawbacks

None - This PR only adds tests without modifying production code.

Verification Process

  • Ran python -m pytest tests/unit_tests/extrinsics/asyncex/test_mev_shield.py -v - All 6 tests pass
  • Tests follow existing async test patterns using @pytest.mark.asyncio and AsyncMock
  • Tests use realistic Bittensor network values for hashes and commitments (32-byte hex strings)

Release Notes

N/A - Test-only changes, not user-facing.

Branch Acknowledgement

[x] I am acknowledging that I am opening this branch against SDKv10

Contribution by Gittensor, learn more at https://gittensor.io/

@codomposer
Copy link
Contributor Author

codomposer commented Dec 8, 2025

@basfroman Could you please take a look at this PR when you are free?

@basfroman
Copy link
Collaborator

@basfroman Could you please take a look at this PR when you are free?

I don't see any test power or load in these tests. If you generate tests with AI, please first try to understand the essence of the tests we write.

If you're writing a UNIT test for the submit_encrypted_extrinsic function, your ultimate goal is to verify that the following internal calls were made, with what parameters (if any) and how many times (depending on the parameters of the main call):

  • ExtrinsicResponse.from_exception
  • ExtrinsicResponse.unlock_wallet
  • subtensor.get_mev_shield_next_key
  • subtensor.substrate.get_account_next_index
  • subtensor.substrate.create_signed_extrinsic
  • get_mev_commitment_and_ciphertext
  • MevShield.submit_encrypted
  • subtensor.sign_and_send_extrinsic
  • wait_for_extrinsic_by_hash
  • format_error_message

In negative scenarios, it's worth checking what types of errors were raised.

And verification as:

  assert result.success is True
  assert result.data["commitment"] == MOCK_COMMITMENT
  assert result.data["ciphertext"] == MOCK_CIPHERTEXT
  assert result.data["ml_kem_768_public_key"] == b"x" * ML_KEM_768_KEY_SIZE
  assert result.data["payload_core"] == MOCK_PAYLOAD_CORE
  assert result.data["signed_extrinsic_hash"] == MOCK_EXTRINSIC_HASH_HEX

doesn't provide any benefit for unit testing.
You assigned these values ​​above, and you'll also check that the variables have these values. This is not the correct approach.

Take this test, for example. There's no assignment to real data at all. Everything is checked through mocks. But we know exactly which arguments are used in the internal calls.

Also, we don't write tests for just one implementation (synchronous or asynchronous). If you have both, you need to implement tests for both. You've only implemented tests for the async one.

@basfroman
Copy link
Collaborator

@basfroman Could you please take a look at this PR when you are free?

Hey @codomposer , pls take a look this pr with tests. This is how I expect to see the coverage. Now the coverage is 100% for sync and async extrinsic modules.
You can close this PR.

@codomposer codomposer closed this Dec 9, 2025
@basfroman
Copy link
Collaborator

@basfroman Could you please take a look at this PR when you are free?

Hey @codomposer , pls take a look this pr with tests. This is how I expect to see the coverage. Now the coverage is 100% for sync and async extrinsic modules. You can close this PR.

I am currently updating the test script

SDKv10 already has 100% coverage for mev extrinsic modules. Pls take a look the PR from my prev comment. Current logic in this PR is redundant.
MeV logic is a priority, and we didn't have time to wait for a proper PR.
Your PR is still unfinished. Read all the comments above.

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