Fix: Connected accounts can now claim via whitelisted root (#24)

[Ryjen1]

The problem was that the SDK was using the secondary wallet address for the entitlement check, but the UBI contract actually requires the main whitelisted "root" address. I've updated the ClaimSDK to resolve that root address automatically behind the scenes.

What's in this PR:

• The Fix: Added a helper logic in ClaimSDK that finds the main whitelisted address for any connected wallet so the claim works correctly.
• Docs: Updated the README with a new section on how connected accounts flow works, including some updated diagrams.
• Testing Tool: I've added a little standalone test script (test-connected-accounts.ts) that makes it easy to verify this logic without needing a complex setup.

Verification:
I've run through the main scenarios (whitelisted, connected, and non-whitelisted) and everything is passing locally. Also confirmed that yarn build still works perfectly across the repo with no breaks.

Fixed issue #24.

[sourcery-ai]

Hey - I've found 3 issues, and left some high level feedback:

• The getWhitelistedRootAddress JSDoc says it may throw when unable to resolve the whitelisted root, but the implementation never throws (e.g., for a 0x0 root); consider explicitly handling the non-whitelisted case and either throwing or returning a clearer error to match the contract and avoid calling checkEntitlement with 0x0.
• The hardcoded contract addresses in test-connected-accounts.js duplicate environment-specific config that likely already exists in the SDK; consider importing or reusing a shared config source so these addresses cannot silently diverge from the main code.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `getWhitelistedRootAddress` JSDoc says it may throw when unable to resolve the whitelisted root, but the implementation never throws (e.g., for a `0x0` root); consider explicitly handling the non-whitelisted case and either throwing or returning a clearer error to match the contract and avoid calling `checkEntitlement` with `0x0`.
- The hardcoded contract addresses in `test-connected-accounts.js` duplicate environment-specific config that likely already exists in the SDK; consider importing or reusing a shared config source so these addresses cannot silently diverge from the main code.

## Individual Comments

### Comment 1
<location> `packages/citizen-sdk/src/sdks/viem-claim-sdk.ts:168-187` </location>
<code_context>
+   * @returns The whitelisted root address to use for entitlement checks.
+   * @throws If unable to resolve the whitelisted root.
+   */
+  private async getWhitelistedRootAddress(): Promise<Address> {
+    // Return cached value if available
+    if (this.whitelistedRootCache) {
+      return this.whitelistedRootCache
+    }
+
+    // Resolve the whitelisted root for this account
+    const { root } = await this.identitySDK.getWhitelistedRoot(this.account)
+
+    // Cache the result
</code_context>

<issue_to_address>
**suggestion:** Clarify behavior when getWhitelistedRoot fails or returns an unexpected root.

The docs say this throws when the root can’t be resolved, but the method currently just returns whatever `identitySDK.getWhitelistedRoot` gives you. If that call can return a non-whitelisted address, `null`/`undefined`, or throw domain-specific errors, consider handling and normalizing those cases here (e.g., throw a specific error when no root exists or the user isn’t whitelisted) so callers see predictable, documented failure modes.

```suggestion
  /**
   * Resolves the whitelisted root address for the connected account.
   * This enables connected accounts to claim on behalf of their main whitelisted account.
   *
   * Failure modes are normalized so callers see predictable behavior:
   * - Throws when no whitelisted root exists for the connected account.
   * - Throws when the SDK cannot resolve a whitelisted root (network / domain errors).
   *
   * @returns The whitelisted root address to use for entitlement checks.
   * @throws Error If no whitelisted root exists or resolution fails for any reason.
   */
  private async getWhitelistedRootAddress(): Promise<Address> {
    // Return cached value if available
    if (this.whitelistedRootCache) {
      return this.whitelistedRootCache
    }

    try {
      // Resolve the whitelisted root for this account
      const result = await this.identitySDK.getWhitelistedRoot(this.account)
      const root = result?.root as Address | undefined

      // Normalize "no root" / "not whitelisted" cases
      if (!root) {
        throw new Error(
          'No whitelisted root address found for connected account; the user may not be whitelisted.'
        )
      }

      // Cache the result
      this.whitelistedRootCache = root

      return root
    } catch (error) {
      // Normalize SDK and transport errors into a predictable failure mode
      const message =
        error instanceof Error && error.message
          ? error.message
          : String(error)

      throw new Error(
        `Unable to resolve whitelisted root address for connected account: ${message}`
      )
    }
  }
```
</issue_to_address>

### Comment 2
<location> `packages/citizen-sdk/test-connected-accounts.js:225-227` </location>
<code_context>
+}
+
+// Run tests
+testConnectedAccounts().catch((error) => {
+    log(`\n❌ Test execution failed: ${error.message}\n`, 'red')
+    console.error(error)
+    process.exit(1)
+})
</code_context>

<issue_to_address>
**nitpick:** Guard against non-Error rejections when accessing `error.message`

Here `error` might not be an `Error` (e.g. string or custom object), so `error.message` can throw and hide the real failure. Use a safe fallback, e.g.:

```ts
const message = error instanceof Error ? error.message : String(error)
log(`\n❌ Test execution failed: ${message}\n`, 'red')
```

This keeps logging robust regardless of what the promise rejects with.
</issue_to_address>

### Comment 3
<location> `packages/citizen-sdk/README-ClaimSDK.md:10` </location>
<code_context>
-4. **Submit claim** – send the `claim()` transaction and wait for confirmation.
+2. **Resolve whitelisted root** – for connected accounts, resolve the main whitelisted address.
+3. **Check entitlement** – determine the claimable amount on the active chain using the whitelisted root address.
+4. **Look for fallbacks** – if no entitlement on current chain, check alternatives (Fuse ⇄ Celo ⇄ XDC).
+5. **Trigger faucet (optional)** – tops up the claim contract if required.
+6. **Submit claim** – send the `claim()` transaction and wait for confirmation.
</code_context>

<issue_to_address>
**nitpick (typo):** Consider adding "the" before "current chain" for smoother grammar.

Reword to: “if no entitlement on the current chain, check alternatives …” for a more natural reading.

```suggestion
4. **Look for fallbacks** – if no entitlement on the current chain, check alternatives (Fuse ⇄ Celo ⇄ XDC).
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[L03TJ3]

@Ryjen1 also, seeing an update to the package.json, yarn.lock update should also be included

[L03TJ3]

You are not running prettier which is configured for the repo.
not sure what your setup is, but make sure to take note of this:https://github.com/GoodDollar/.github/blob/master/CONTRIBUTING.md#6-opening-a-pull-request
the 'vscode' formatting tip.
its vscode example but similar setups would work in most other environments.

[L03TJ3]

Very explanatory for a pretty straightforward script.
if you add a readme for a script like this, keep it simple.

1. Purpose (1-2 lines)
2. Exact run command(s)
3. Required env vars (and which are truly required)
4. What pass/fail means in practice

another way is to include it as starting comment in the script itself, simple and to the point

[vercel]

Deployment failed with the following error:

Creating the Deployment Timed Out.

[Ryjen1]

@sirpy @L03TJ3....I have made a request

[sirpy]

@Ryjen1 also please address the open comments by @L03TJ3 , explain how you addressed them.

[Ryjen1]

Hi @sirpy , I’ve addressed all the feedback, including the points raised by @L03TJ3

Engagement App

• Fixed lint issues by removing the unused actionTypes
• Reverted the redundant refactors to keep the diff clean

Claim SDK

• Removed the caching logic
• Updated root address resolution to return zeroAddress instead of throwing

Testing & Infrastructure

• Migrated the connected accounts test suite to Vitest (now located in packages/citizen-sdk)
• Added validation that throws if required env vars are missing
• Reduced the test documentation as requested

Formatting

• Ran yarn format across the repository to align with the project’s Prettier configuration

All 4 tests are passing
Build is successful

[L03TJ3]

Besides reverting unnecessary changes, please update identityCard so that it uses getWhitelistedRoot and makes the demo work as expected.

other then that, approved