Skip to content

test(swift-sdk): run SDKMethodTests against the offline mock SDK instead of live testnet - #5009

Merged
QuantumExplorer merged 1 commit into
v4.2-devfrom
claude/hopeful-turing-0dbc1b
Sep 26, 2026
Merged

QuantumExplorer merged 1 commit into
v4.2-devfrom
claude/hopeful-turing-0dbc1b

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

The required job "Swift SDK build / Swift SDK build + tests (warnings as errors)" ran two unit tests in SDKMethodTests that built SDK(network: .testnet). One of them, testSimpleIdentityFetch, read a live testnet identity. On 2026-09-25 testnet Platform stopped producing blocks at height 601636. From about 22:35 UTC, every PR's Swift job failed on that test, including unrelated ones such as #5006 (run 36199315296) and branch fix/dpns-name-pick-survives-sync (run 36198430696):

internalError("received invalid time: expected ...ms, received 1790373853478 ms, tolerance 1860000 ms; try another server")

That is the SDK's stale-block time check working as intended. Unit tests must not touch the network (CLAUDE.md, book/src/contributing/coding-conventions.md), and Package.swift already labels this target "Unit tests (offline, hermetic)".

The second test, testDirectMethodCall, also built a testnet SDK, but it never reached the call it meant to test. Its all-zero private key is not a valid secp256k1 scalar, so dash_sdk_signer_create_from_private_key failed and the test printed "Failed to create signer" and returned early, passing without checking anything.

What was done?

  • SDK.init(mockVectorsDirectory:) (internal, in Sources/SwiftDashSDK/SDK.swift) wraps the existing FFI constructor dash_sdk_create_handle_with_mock. It adds no new FFI. The mock SDK has no transport: its DAPI client answers only from recorded msg_*.json responses, still verifies their proofs against the recorded quorum keys, and fails any request it has no recording for. SDK(network:) is unchanged. It always goes through dash_sdk_create_trusted, which falls back to the seed nodes, so Swift previously had no way to reach the mock.
  • testSimpleIdentityFetch now uses SDK(mockVectorsDirectory:) with rs-sdk's recorded packages/rs-sdk/tests/vectors/test_identity_read vectors. It fetches identity [1; 32] through identityGet, so the recorded response is replayed, its proof is verified, and the result is decoded into the Swift dictionary. It then asserts the id and that public keys are present.
  • testDirectMethodCall now uses the mock with no vectors and a valid key (0x01 repeated 32 times). It fails if signer creation fails, and it requires SDKError.internalError containing Invalid to_identity_id, because the FFI refuses "test2" while parsing the identifier. Any other error fails the test, so a network failure can no longer make it pass.
  • The live testnet read moves to SwiftTests/SwiftDashSDKIntegrationTests/Platform/TestnetIdentityFetchTests.swift. It is skipped unless RUN_TESTNET_TESTS=1 is set, and it keeps the original check (SDK(network: .testnet), identityGet("5DbLwAxGBzUzo81VewMUwn4b5P4bpv9FNFybi25XB5Bk")) plus an id assertion. It is a plain XCTestCase because IntegrationTestCase starts a local devnet.

Before / after

swift test --filter SDKMethodTests while testnet was halted:

Before:

Test Case '-[SwiftDashSDKTests.SDKMethodTests testDirectMethodCall]' passed   (printed "Failed to create signer"; transferCredits never ran)
Test Case '-[SwiftDashSDKTests.SDKMethodTests testSimpleIdentityFetch]' failed
  internalError("received invalid time: expected ...ms, received 1790373853478 ms, tolerance 1860000 ms; try another server")
Executed 3 tests, with 1 failure

After:

testDirectMethodCall     passed (transferCredits refused "test2": Invalid to_identity_id)
testSimpleIdentityFetch  passed (identity 4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi, 3 public keys, proof verified from vectors)
Executed 3 tests, with 0 failures

With RUN_TESTNET_TESTS unset, TestnetIdentityFetchTests is skipped. With RUN_TESTNET_TESTS=1, it reads testnet as before.

Worth knowing

The test_identity_read vectors carry GroveDB V0 proofs. They verify because the mock SDK is built for mainnet, whose SDK protocol version floor (min_protocol_version(Mainnet)) is 13. If that floor rises to 14, testSimpleIdentityFetch will fail until the vectors are regenerated with scripts/generate_test_vectors.sh, as rs-sdk's own identity offline tests already note. That breakage comes from a code change in this repository, not from network state.

How Has This Been Tested?

The tests were run on macOS from packages/swift-sdk with the branch at 7829c84f2a. It was then fast-forwarded onto 1f5069590a, which adds #5005, #5006 and #5008. None of those touch packages/swift-sdk, and #5008's rs-sdk-ffi change is outside the mock path, so the tests were not re-run after the fast-forward.

  • bash build_ios.sh --target tests --profile dev: OK.
  • swift build --build-tests: OK with no warnings, including the integration target, which compiles with -warnings-as-errors.
  • swift test --filter SDKMethodTests: 3 tests, 0 failures. The baseline before the change was 1 failure, the error above.
  • No-network check:
    sandbox-exec -p '(version 1)(allow default)(deny network-outbound (remote ip))(deny network-inbound (local ip))' \
      xcrun xctest -XCTest SwiftDashSDKTests.SDKMethodTests .build/debug/SwiftDashSDKPackageTests.xctest
    
    3 tests, 0 failures. As a control, the testnet test fails in the same sandbox with tcp connect error ... Operation not permitted.
  • RUN_TESTNET_TESTS=1 swift test --filter TestnetIdentityFetchTests: passed once testnet was producing blocks again. Without the variable, it is skipped.
  • Full swift test: 735 tests, 18 skipped, 0 failures. One of three runs had an unrelated database is locked failure in DashModelMigrationTests, which passed 3 of 3 times when run on its own.
  • xcodebuild test -project SwiftExampleApp/SwiftExampleApp.xcodeproj -scheme SwiftExampleApp -skip-testing:SwiftExampleAppUITests: 206 passed, 15 skipped, 0 failed.

Breaking Changes

None. Test-only change plus one internal initializer.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed
  • If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · 0779964

  • Bots — coderabbitai ✓ · thepastaclaw not yet — /skip-bots proceeds without the ones not yet reported
  • Self-review — post /self-reviewed once the bots are done
  • Within your 5 open PRs — this one is beyond the limit; it waits until one merges
  • Build running
  • Approvals
    • swift-sdk (packages/swift-sdk/Package.swift, packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift, packages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Platform/TestnetIdentityFetchTests.swift and 1 more) — llbartekll or romchornyi

When every box is checked the PR Hygiene check passes and this can merge.

Summary by CodeRabbit

  • Tests
    • Added an opt-in testnet integration test that fetches a known identity and verifies its ID.
    • Expanded offline SDK test coverage using recorded responses, including identity retrieval and validation of malformed recipient IDs.
    • Existing offline tests use a mock rather than making live network requests; live testnet coverage runs only when explicitly enabled.

…ead of live testnet

testSimpleIdentityFetch and testDirectMethodCall built SDK(network: .testnet),
so the required Swift SDK CI job failed on every PR whenever testnet was
unreachable or stopped producing blocks (the SDK's stale-block time check
refused the response). Unit tests must not touch the network.

- Add an internal SDK(mockVectorsDirectory:) initializer over the existing
  FFI dash_sdk_create_handle_with_mock. The mock has no transport; it
  replays recorded DAPI responses and still verifies their proofs.
- testSimpleIdentityFetch now fetches identity [1; 32] through identityGet
  from rs-sdk's recorded test_identity_read vectors and checks its id and
  public keys.
- testDirectMethodCall now runs on the mock with a valid private key. The
  all-zero key it used is not a valid secp256k1 scalar, so signer creation
  failed and the test returned before calling transferCredits. It now
  requires the "Invalid to_identity_id" refusal instead of any error.
- The live testnet identity read moves to
  SwiftDashSDKIntegrationTests/Platform/TestnetIdentityFetchTests.swift,
  skipped unless RUN_TESTNET_TESTS=1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added this to the v4.2.0 milestone Sep 26, 2026
@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Sep 26, 2026
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: dashpay/platform/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: fc01da1f-44df-47d0-8253-6d555e847f59

📥 Commits

Reviewing files that changed from the base of the PR and between 1f50695 and 0779964.

📒 Files selected for processing (4)
  • packages/swift-sdk/Package.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift
  • packages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Platform/TestnetIdentityFetchTests.swift
  • packages/swift-sdk/SwiftTests/SwiftDashSDKTests/SDKMethodTests.swift

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Swift SDK tests now use an FFI mock and recorded response vectors for method checks. A separate integration test fetches a known identity from testnet when RUN_TESTNET_TESTS=1.

Changes

Swift SDK tests

Layer / File(s) Summary
Mock-backed SDK method tests
packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift, packages/swift-sdk/SwiftTests/SwiftDashSDKTests/SDKMethodTests.swift
An internal initializer creates an FFI mock handle, optionally using a vectors directory. Unit tests use the mock to check malformed recipient rejection and fetch an identity from recorded vectors.
Opt-in testnet identity fetch
packages/swift-sdk/Package.swift, packages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Platform/TestnetIdentityFetchTests.swift
A comment documents the RUN_TESTNET_TESTS=1 gate. The integration test skips unless enabled, then fetches a known identity and checks its ID.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: zocolini

Merge Risk: ⚪ Minimal · up to 07799

The offline tests and opt-in testnet test are ready to merge after normal checks; no concrete merge-blocking issue remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: SDKMethodTests now use the offline mock SDK instead of live testnet access.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thepastaclaw

thepastaclaw commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

🕓 Queued for automated review — 1st in line, estimated start in ~15 min (commit 0779964)
Estimated review time once started: ~40 min (two-phase automated review; median of recent runs).

  • Request priority review — click to move this review to the front of the queue.

@QuantumExplorer
QuantumExplorer merged commit 0621670 into v4.2-dev Sep 26, 2026
22 of 23 checks passed
@QuantumExplorer
QuantumExplorer deleted the claude/hopeful-turing-0dbc1b branch September 26, 2026 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client Only waiting-bots Waiting for the review bots to report on this head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants