[pietro909]

Summary

• Add encodeArkContract call to render the URI-like arkcontract=… string and let users copy it
• Replace inline cardStyle div with Shadow border (consistent with the rest of the codebase)
• Show contract.label (when set) and createdAt as relative time on each card
• Split list into Active / Inactive labeled sections
• Add empty state when no contracts are found

Screenshot

Summary by CodeRabbit

• Improvements
  • Contracts are now organized into "Active" and "Inactive" sections for easier viewing
  • Contract parameters are now displayed for each contract entry
  • Enhanced contract card layout with improved visual formatting

[coderabbitai]

Warning

Review limit reached

@pietro909, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 20 minutes and 50 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1fff2af0-854e-4304-b6ab-ce5f504330d7

📥 Commits

Reviewing files that changed from the base of the PR and between b0fe0a7 and 60ee259.

📒 Files selected for processing (2)

• src/screens/Settings/Contracts.tsx
• src/test/screens/settings/contracts.test.tsx

Walkthrough

The Contracts.tsx file refactors how contracts are displayed and organized. It enables contract parameter encoding by importing encodeArkContract, adds supporting utilities for UI interactions, refactors the ContractCard component with new styling and parameter display, introduces a Section helper to group contracts, and updates the main screen to display contracts in active and inactive groups.

Changes

Contract display and grouping refactor

Layer / File(s)	Summary
Contract card and section components with supporting imports src/screens/Settings/Contracts.tsx	SDK imports now include encodeArkContract function; new utility imports support formatting, clipboard, haptics, and toast notifications. ContractCard is refactored with a bordered card layout, conditional label rendering, and parameter display. New Section helper groups contract lists and omits empty sections.
Contracts screen grouped layout src/screens/Settings/Contracts.tsx	Screen rendering changes from a single flat list to a grouped view: contracts are filtered into active and inactive arrays, each rendered via the Section component. Empty-state text is preserved when no contracts exist.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
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 (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the three main changes: encoding/copying params, empty state handling, and active/inactive sections organization.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/contracts-copy-params

---

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 and usage tips.}

[cloudflare-workers-and-pages]

Deploying tmp-boltz-upstream-mainnet-arkade-wallet with    Cloudflare Pages

Latest commit:	60ee259
Status:	✅  Deploy successful!
Preview URL:	https://2d7c9fb6.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev
Branch Preview URL:	https://feat-contracts-copy-params.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev

View logs

[cloudflare-workers-and-pages]

Deploying wallet-bitcoin with    Cloudflare Pages

Latest commit:	60ee259
Status:	✅  Deploy successful!
Preview URL:	https://c2847b44.wallet-bitcoin.pages.dev
Branch Preview URL:	https://feat-contracts-copy-params.wallet-bitcoin.pages.dev

View logs

[cloudflare-workers-and-pages]

Deploying wallet-mutinynet with    Cloudflare Pages

Latest commit:	60ee259
Status:	✅  Deploy successful!
Preview URL:	https://3c1d5e7b.arkade-wallet.pages.dev
Branch Preview URL:	https://feat-contracts-copy-params.arkade-wallet.pages.dev

View logs

[ghost]

Code Review — contracts view: copy encoded params, empty state, active/inactive sections

Scope: Single-file UI change (src/screens/Settings/Contracts.tsx). Not protocol-critical — no VTXO, signing, forfeit, or exit path logic touched.

Verdict: Clean, low-risk UI improvement. Two actionable items before merge.

---

🔴 Must fix

1. encodeArkContract called unguarded in render — can crash the screen
src/screens/Settings/Contracts.tsx:68

<CopyRow label='parameters' value={encodeArkContract(contract)} />

If encodeArkContract throws on a contract with unexpected/malformed data (e.g. a contract created by an older SDK version missing a field), the entire Contracts screen white-screens with an unhandled React error. This is the only screen where users can inspect their contracts — losing it is bad UX.

Wrap it:

const encoded = (() => {
  try { return encodeArkContract(contract) } catch { return '' }
})()
// ... then conditionally render:
{encoded && <CopyRow label='parameters' value={encoded} />}

Or compute it in a useMemo with a try-catch inside ContractCard.

2. Tests are stale — will break on this PR
src/test/screens/settings/contracts.test.tsx

The mock contracts lack createdAt (now rendered via prettyAgo) and label. More critically, encodeArkContract is imported as a real SDK function but will be called on incomplete mock data — this will either throw or produce garbage.

Needs:

• Add createdAt: 1717000000 (and optionally label) to mock contracts
• Mock or spy encodeArkContract from @arkade-os/sdk to avoid coupling test to SDK internals
• Add a test case for the empty state (contracts.length === 0 → renders "No contracts found.")
• Existing assertions for screen.getByText('default') / screen.getByText('vhtlc') should still pass since the text nodes remain, but verify after adding the missing fields

---

🟡 Nits (non-blocking)

3. encodeArkContract recomputed on every render
src/screens/Settings/Contracts.tsx:68

encodeArkContract likely involves string encoding work. On a settings screen with a handful of contracts this is fine, but wrapping it in useMemo would be the defensive play — especially if this component ever gets reused in a more render-heavy context.

4. key={c.script} as React key — pre-existing but worth noting
src/screens/Settings/Contracts.tsx:79

If two contracts can share the same script (same contract type, different addresses), React will silently drop one. c.address would be a safer unique key, or a composite ${c.address}:${c.script}.

---

✅ What's good

• import type { Contract } → import { Contract, encodeArkContract } — correct, need the runtime value now
• prettyAgo(contract.createdAt) — prettyAgo gracefully handles falsy (if (!timestamp) return '') and auto-detects ms vs s timestamps. Solid.
• contract.label ? ... : null — correctly handles the optional label field
• Active/inactive split logic is clean: === 'active' / !== 'active' is exhaustive
• Section returning null when empty prevents rendering empty headers
• Replacing inline cardStyle with <Shadow border> is a good consistency win

LGTM with the error handling fix and test updates. 🫡

—Arkana

[ghost]

Code Review — contracts view: copy encoded params, empty state, active/inactive sections

Scope: Single-file UI change (src/screens/Settings/Contracts.tsx). Not protocol-critical — no VTXO, signing, forfeit, or exit path logic touched.

Verdict: Clean, low-risk UI improvement. Two actionable items before merge.

---

🔴 Must fix

1. encodeArkContract called unguarded in render — can crash the screen
src/screens/Settings/Contracts.tsx:68

<CopyRow label='parameters' value={encodeArkContract(contract)} />

If encodeArkContract throws on a contract with unexpected/malformed data (e.g. a contract created by an older SDK version missing a field), the entire Contracts screen white-screens with an unhandled React error. This is the only screen where users can inspect their contracts — losing it is bad UX.

Wrap it:

const encoded = (() => {
  try { return encodeArkContract(contract) } catch { return '' }
})()
// ... then conditionally render:
{encoded && <CopyRow label='parameters' value={encoded} />}

Or compute it in a useMemo with a try-catch inside ContractCard.

2. Tests are stale — will break on this PR
src/test/screens/settings/contracts.test.tsx

The mock contracts lack createdAt (now rendered via prettyAgo) and label. More critically, encodeArkContract is imported as a real SDK function but will be called on incomplete mock data — this will either throw or produce garbage.

Needs:

• Add createdAt: 1717000000 (and optionally label) to mock contracts
• Mock or spy encodeArkContract from @arkade-os/sdk to avoid coupling test to SDK internals
• Add a test case for the empty state (contracts.length === 0 → renders "No contracts found.")
• Existing assertions for screen.getByText('default') / screen.getByText('vhtlc') should still pass since the text nodes remain, but verify after adding the missing fields

---

🟡 Nits (non-blocking)

3. encodeArkContract recomputed on every render
src/screens/Settings/Contracts.tsx:68

encodeArkContract likely involves string encoding work. On a settings screen with a handful of contracts this is fine, but wrapping it in useMemo would be the defensive play — especially if this component ever gets reused in a more render-heavy context.

4. key={c.script} as React key — pre-existing but worth noting
src/screens/Settings/Contracts.tsx:79

If two contracts can share the same script (same contract type, different addresses), React will silently drop one. c.address would be a safer unique key, or a composite ${c.address}:${c.script}.

---

✅ What's good

• import type { Contract } → import { Contract, encodeArkContract } — correct, need the runtime value now
• prettyAgo(contract.createdAt) — prettyAgo gracefully handles falsy (if (!timestamp) return '') and auto-detects ms vs s timestamps. Solid.
• contract.label ? ... : null — correctly handles the optional label field
• Active/inactive split logic is clean: === 'active' / !== 'active' is exhaustive
• Section returning null when empty prevents rendering empty headers
• Replacing inline cardStyle with <Shadow border> is a good consistency win

LGTM with the error handling fix and test updates. 🫡

—Arkana

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/screens/Settings/Contracts.tsx (1)

113-114: 💤 Low value

Consider using a single pass to partition contracts (optional micro-optimization).

The current implementation filters the contracts array twice (once for active, once for inactive). While this is clear and readable, you could partition in a single pass if preferred:

♻️ Single-pass partition approach

-  const active = contracts.filter((c) => c.state === 'active')
-  const inactive = contracts.filter((c) => c.state !== 'active')
+  const [active, inactive] = contracts.reduce<[Contract[], Contract[]]>(
+    ([act, inact], c) => (c.state === 'active' ? [[...act, c], inact] : [act, [...inact, c]]),
+    [[], []]
+  )

However, given that contract lists are typically small, the current approach is clearer and the performance difference is negligible.

🤖 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/screens/Settings/Contracts.tsx` around lines 113 - 114, The current code
computes active and inactive by filtering contracts twice (const active =
contracts.filter((c) => c.state === 'active') and const inactive =
contracts.filter((c) => c.state !== 'active')), so replace that with a
single-pass partition over contracts (e.g., a simple for loop or
Array.prototype.reduce) that builds active and inactive arrays in one traversal;
update any references to active and inactive to use the new variables and keep
the same semantics (state === 'active' goes to active, everything else goes to
inactive).

🤖 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/screens/Settings/Contracts.tsx`:
- Line 63: The UI currently calls prettyAgo(contract.createdAt) and will render
an empty string when contract.createdAt is missing; update the rendering in the
Contracts component (where contract.createdAt is used) to show a fallback such
as contract.createdAt ? prettyAgo(contract.createdAt) : 'Unknown' (i.e., guard
the call to prettyAgo with a truthy check on Contract.createdAt and render
'Unknown' when absent) so tests and missing dates display a clear value.

---

Nitpick comments:
In `@src/screens/Settings/Contracts.tsx`:
- Around line 113-114: The current code computes active and inactive by
filtering contracts twice (const active = contracts.filter((c) => c.state ===
'active') and const inactive = contracts.filter((c) => c.state !== 'active')),
so replace that with a single-pass partition over contracts (e.g., a simple for
loop or Array.prototype.reduce) that builds active and inactive arrays in one
traversal; update any references to active and inactive to use the new variables
and keep the same semantics (state === 'active' goes to active, everything else
goes to inactive).

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3880bc8e-b0ec-46e2-8671-911e8e31865a

📥 Commits

Reviewing files that changed from the base of the PR and between 0098371 and b0fe0a7.

📒 Files selected for processing (1)

• src/screens/Settings/Contracts.tsx

[tiero]

Is this a dev gated feature? Curious if not if it aligns with new redesign of @sahilc0

[pietro909]

@tiero it's dev-gated. The main issue was to expose the contract params but since I was there I decided to revamp a bit the UI. Not a blocker from my side.

[ghost]

Re-review after 60ee259f — all three issues from my previous review are resolved:

1. ✅ encodeArkContract wrapped in useMemo + try/catch — malformed contracts no longer white-screen
2. ✅ createdAt fallback to 'Unknown' when absent
3. ✅ React key changed to c.address (unique per contract)
4. ✅ Tests updated with params/createdAt fields + empty-state test added

Clean UI-only change. No protocol-critical paths touched. LGTM.

—Arkana

[bordalix]

@sahilc0 we need to have this on the new design.
And I think Boltz swaps are also useful in the new design.
Maybe add new options to Settings / Advanced?