feat: harden StarkNet client typing and add production client tests (#58)

[daveades]

Summary

• Replace broad any casts in campaignFactory.starknet.ts with narrow local types: StarknetEvent, StarknetReceipt, ExecuteResult, and StarknetDeps
• Add optional _deps injection parameter to createStarknetCampaignClient (exported StarknetDeps type) enabling test-time mocking without live RPC access
• Add 14 focused tests in cairo.client.starknet.test.ts covering: all 4 missing required env vars, missing transaction hash, both transaction_hash/transactionHash field name fallbacks, event campaign-ID extraction, case-insensitive event key matching, empty-data fallback, and retry success/exhaustion behavior

Test plan

• bun run type-check passes (no any type errors)
• All 14 new tests pass with tsx --test
• eslint reports no errors on changed files
• Existing mock client test (cairo.client.mock.test.ts) unaffected
• Production behavior is exercisable without live RPC access via injected deps

Closes #58

Summary by CodeRabbit

• Bug Fixes

  • Improved campaign creation to handle differing transaction-hash field names returned by execution.
  • Made campaign ID derivation more robust, including case-insensitive event-key matching with reliable fallback behavior.
  • Updated retry behavior: contract accessibility checks retry transient issues, while campaign execution failures now fail fast.

• Tests

  • Added a Node test suite covering missing configuration validation, transaction-hash extraction, event-based campaign ID parsing/fallbacks, and retry behavior.

[drips-wave]

@daveades Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds typed StarkNet receipt and execution helpers, optional dependency injection for the campaign client, and tests for env validation, transaction-hash extraction, event-based campaign ID parsing, and retry behavior.

Changes

StarkNet Client Hardening

Layer / File(s)	Summary
Types, dep injection, and createCampaign call update src/services/cairo/campaignFactory.starknet.ts	Defines typed StarkNet receipt and execute-result shapes, exports StarknetDeps, updates createStarknetCampaignClient to accept injected deps, tightens event parsing, and removes retry wrapping from account.execute.
Test scaffold, env validation, and tx hash tests src/__tests__/cairo.client.starknet.test.ts	Sets up mock deps and env helpers, verifies missing CAIRO env vars throw, and checks transaction-hash extraction and fallback behavior.
Campaign ID event parsing tests src/__tests__/cairo.client.starknet.test.ts	Covers event-key-driven campaignId extraction, fallback behavior, and case-insensitive matching.
Retry behavior tests src/__tests__/cairo.client.starknet.test.ts	Verifies contract accessibility retries and confirms account.execute failures are not retried.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hop through types with a gentle grin,
No loose any slips sneaking in.
The envs are checked, the events align,
And retries behave just fine.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the summary and test plan, but it omits required template sections like Area, Scope, Verification, Indexer Safety, and Notes.	Add the missing template sections, including Area, Scope, Verification checkboxes, Indexer Safety notes, and a populated closing issue reference.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main change: hardened StarkNet typing plus production tests.
Linked Issues check	✅ Passed	The changes satisfy the linked issue by narrowing StarkNet types and adding the requested failure, parsing, and retry tests.
Out of Scope Changes check	✅ Passed	No clear out-of-scope changes are introduced; the diff stays focused on StarkNet client typing and test coverage.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/services/cairo/campaignFactory.starknet.ts`:
- Around line 93-96: The submission path in create_campaign is incorrectly
wrapped in withRetry around account.execute, which can resubmit a state-changing
transaction after transient RPC errors; update the campaignFactory.starknet.ts
flow so retries remain only around read-only validation steps, and in the create
campaign submission path call account.execute directly or make any resubmission
logic explicitly nonce-aware. Use the existing create_campaign invocation and
logger.info context to locate the submit step, and ensure the retry helper is
not applied to the final transaction send.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 79c82866-d16b-4b73-97e9-7b51d0eb8867

📥 Commits

Reviewing files that changed from the base of the PR and between a91569c and 606b1e1.

📒 Files selected for processing (2)

• src/__tests__/cairo.client.starknet.test.ts
• src/services/cairo/campaignFactory.starknet.ts

[pragmaticAweds]

Hi @daveades

Thank you for your awesome contribution, however after analyzing your implementation, there are some minor fixes to be done. Kindly fix them to merge your PR asap.

Also do not forget to use fundable.finance to offramp.

[coderabbitai]

🧹 Nitpick comments (1)

src/services/cairo/campaignFactory.starknet.ts (1)

54-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Defer env validation until the fallback path actually needs it.

createStarknetCampaignClient(deps) still reads CAIRO_RPC_URL, CAIRO_ACCOUNT_ADDRESS, and CAIRO_PRIVATE_KEY before it consults the injected provider/account, so the new DI hook remains coupled to process env. Lazily resolve those vars only when _deps does not supply the dependency, and consider injecting factoryAddress too if you want this constructor to be fully testable without env scaffolding.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/cairo/campaignFactory.starknet.ts` around lines 54 - 68, The
`createStarknetCampaignClient` constructor still eagerly calls `getRequiredEnv`
for `CAIRO_RPC_URL`, `CAIRO_ACCOUNT_ADDRESS`, and `CAIRO_PRIVATE_KEY` before
checking `_deps`, so the injected `provider` and `account` are not actually
decoupled from process env. Move those env reads behind the fallback branches in
`createStarknetCampaignClient`, only resolving them when `_deps.provider` or
`_deps.account` is missing, and consider applying the same pattern to
`factoryAddress` if you want the client setup to be fully testable without env
scaffolding.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/services/cairo/campaignFactory.starknet.ts`:
- Around line 54-68: The `createStarknetCampaignClient` constructor still
eagerly calls `getRequiredEnv` for `CAIRO_RPC_URL`, `CAIRO_ACCOUNT_ADDRESS`, and
`CAIRO_PRIVATE_KEY` before checking `_deps`, so the injected `provider` and
`account` are not actually decoupled from process env. Move those env reads
behind the fallback branches in `createStarknetCampaignClient`, only resolving
them when `_deps.provider` or `_deps.account` is missing, and consider applying
the same pattern to `factoryAddress` if you want the client setup to be fully
testable without env scaffolding.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 45421c38-ce9b-4f87-8f86-d837772ad440

📥 Commits

Reviewing files that changed from the base of the PR and between 606b1e1 and 5b462e7.

📒 Files selected for processing (2)

• src/__tests__/cairo.client.starknet.test.ts
• src/services/cairo/campaignFactory.starknet.ts

[daveades]

@pragmaticAweds I have now implemented the fixes