Skip to content

Add txid and explorer link in tx details page#385

Open
Kukks wants to merge 4 commits intomasterfrom
explorer-link
Open

Add txid and explorer link in tx details page#385
Kukks wants to merge 4 commits intomasterfrom
explorer-link

Conversation

@Kukks
Copy link
Copy Markdown
Contributor

@Kukks Kukks commented Mar 28, 2026

Summary

  • Display Ark transaction ID with truncated display and direct explorer link in received/sent transaction detail tables
  • Make Ark explorer URL network-aware via getArkExplorerUrl() instead of hardcoding arkade.space (addresses review feedback from @Kukks)
  • Add safe truncation helper to prevent panic on short txid strings (addresses CodeRabbit feedback)

Supersedes #368 — rebased onto current master, resolved conflicts, and addressed all review comments.

Test plan

  • Verify Ark txid displays correctly with truncated format in received tx details
  • Verify Ark txid displays correctly in sent tx details
  • Verify explorer link opens correct URL for mainnet (https://arkade.space/tx/{txid})
  • Verify explorer link uses correct URL for signet
  • Verify no panic when txid is empty or shorter than 12 characters

Summary by CodeRabbit

  • New Features

    • Show Ark transaction IDs (truncated) in transaction detail views and tables.
    • Add Ark explorer “View” links that open transaction pages in a new tab.
    • Transaction rows now support both standard and Ark-specific explorer links.
  • Style

    • Adjusted UI focus/hover styling for accessibility/visual consistency.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 28, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Threads an arkExplorerUrl through web handlers and templates, adds getArkExplorerUrl mapping, surfaces Ark txids in transfer views, updates generated template code to render conditional Ark explorer links, tweaks compiled CSS, and adds retries and a higher refill threshold in an e2e test.

Changes

Cohort / File(s) Summary
Handlers & Utils
internal/interface/web/handlers.go, internal/interface/web/utils.go
Compute and thread arkExplorerUrl through getTx, getTransfer, and claimTx; added getArkExplorerUrl(network string) with network-specific mappings.
Templates (markup)
internal/interface/web/templates/components/feeTable.templ, internal/interface/web/templates/components/tx.templ, internal/interface/web/templates/pages/tx.templ
Extended template signatures to accept arkExplorerUrl; added arkTxid helper and conditional Tx ID + explorer link rendering; pass arkTxid and arkExplorerUrl into ReceivedTxTable/SentTxTable.
Generated Template Go
internal/interface/web/templates/components/feeTable_templ.go, internal/interface/web/templates/components/tx_templ.go, internal/interface/web/templates/pages/tx_templ.go
Regenerated code to match template signature changes; added truncateTxid helper and URL-escaped explorer link construction.
Styling
internal/interface/web/static/styles.css
Recompiled Tailwind output with adjusted utility set; changed focus ring shadow calc term from 1px to 3px.
Tests
internal/test/e2e/main_test.go
Increased Ark refill threshold (5.0 → 10.0) and replaced a single Settle call with a retry loop (up to 3 attempts, 5s delay between failures).

Sequence Diagram

sequenceDiagram
    participant Client as HTTP Handler (getTx)
    participant Utils as Utils (getArkExplorerUrl)
    participant Service as Service (getTransfer)
    participant Page as Page Template (TransferTxBodyContent / Pending)
    participant Comp as Component Template (Transfer → FeeTable)
    participant Browser as Browser

    Client->>Utils: getArkExplorerUrl(network)
    Utils-->>Client: arkExplorerUrl
    Client->>Service: s.getTransfer(..., arkExplorerUrl)
    Service->>Page: pages.TransferTxBodyContent(tx, explorerUrl, arkExplorerUrl)
    Page->>Comp: components.Transfer(tx, explorerUrl, arkExplorerUrl)
    Comp->>Comp: arkTxid(tx) (conditional)
    Comp->>Comp: ReceivedTxTable/SentTxTable(..., arkTxid, arkExplorerUrl)
    Comp-->>Browser: Render HTML (include Ark explorer link if arkTxid != "")
    Browser-->>Browser: Display page with Tx ID / explorer link
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • altafan

Poem

🐰 I hopped through handlers, templates, and rails,
Threading Ark links down template-trails,
A truncated txid, a shiny view,
Click the explorer — the hop says woo! ✨🔗

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding transaction IDs and explorer links to the transaction details page, which is the core objective across all modified files.

✏️ 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 explorer-link

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
internal/interface/web/static/styles.css (1)

1-1: Exclude generated CSS bundle from Stylelint to target source styles only.

The internal/interface/web/static/styles.css file is a minified build artifact generated by Parcel from the source internal/interface/web/assets/styles.scss. Currently, .stylelintrc.json has no ignorePatterns and no .stylelintignore file exists, so any Stylelint invocation would lint the minified output instead of source styles. Add an exclusion pattern (via .stylelintignore or ignorePatterns in .stylelintrc.json) to exclude internal/interface/web/static/ and ensure Stylelint targets the source assets/ directory only.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/interface/web/static/styles.css` at line 1, Add an ignore for the
generated CSS bundle so Stylelint targets source SCSS only: update
.stylelintrc.json (or add a .stylelintignore) to exclude the directory
"internal/interface/web/static/" (or the specific file
"internal/interface/web/static/styles.css"); ensure the ignore entry uses that
exact path string so Stylelint will skip the minified bundle and only lint the
source under "internal/interface/web/assets/" (no code changes to styles.css
needed).
internal/interface/web/templates/pages/tx.templ (1)

14-34: Unused arkExplorerUrl parameter in TransferTxPendingContent.

The arkExplorerUrl parameter is accepted but never used in this template—the pending transfer table doesn't render explorer links. This is likely intentional since pending transactions may not have confirmed Ark txids yet, but consider removing the parameter if it's not needed for consistency with the handler API.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/interface/web/templates/pages/tx.templ` around lines 14 - 34, The
template TransferTxPendingContent currently accepts an unused parameter
arkExplorerUrl; remove arkExplorerUrl from the function signature and update any
callers (handlers or renderers that pass arkExplorerUrl to
TransferTxPendingContent) to stop supplying that argument, or alternatively use
arkExplorerUrl where appropriate (e.g., pass it into
components.PendingTransferTable) if you intend to display explorer links—ensure
consistency between the TransferTxPendingContent signature and all places that
invoke it (search for TransferTxPendingContent usages and adjust their argument
lists accordingly).
internal/interface/web/templates/components/feeTable.templ (1)

149-155: Consider extracting duplicated Ark txid/explorer block into a shared templ helper.

Both tables render identical rows for txid + explorer. A small shared component would reduce drift risk and keep future changes in one place.

Also applies to: 173-179

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/interface/web/templates/components/feeTable.templ` around lines 149
- 155, Extract the duplicated Ark txid/explorer HTML into a shared template
helper (e.g., arkTxRow or renderArkTx) that accepts arkTxid and arkExplorerUrl,
moves the conditional check for arkTxid != "" into that helper, uses
truncateTxid and templ.SafeURL inside it, and call this helper in place of the
duplicated block in feeTable.templ (both occurrences around the current
txid/explorer blocks). Ensure the helper renders the same tableLine("Tx ID",
truncateTxid(arkTxid)) and the explorer link with templ.SafeURL(arkExplorerUrl +
"/tx/" + arkTxid) so both places (the original and the other occurrence)
reference this single implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/interface/web/templates/components/feeTable.templ`:
- Around line 149-154: The explorer "View" row should only render when both
arkTxid and arkExplorerUrl are set; update the conditional guarding the block
that renders the explorer link (the branch using arkTxid, truncateTxid and
templ.SafeURL(arkExplorerUrl + "/tx/" + arkTxid)) to require arkExplorerUrl !=
"" as well (e.g., check arkTxid != "" && arkExplorerUrl != "") so you don't
produce a relative /tx/{txid} link; apply the same change to the second
occurrence around lines rendering the same explorer link.

In `@internal/interface/web/utils.go`:
- Around line 27-36: getArkExplorerUrl currently omits explicit handling for the
"mutinynet" network and falls back to the mainnet URL; update getArkExplorerUrl
to add a case for "mutinynet" that returns the correct Arkade explorer URL (or,
if Arkade has no mutiny-specific explorer, add a clear comment in
getArkExplorerUrl explaining that arklib.BitcoinMutinyNet intentionally falls
back to "https://arkade.space"), referencing the function name getArkExplorerUrl
and the constant arklib.BitcoinMutinyNet so reviewers can verify behavior
matches getExplorerUrl's handling.

---

Nitpick comments:
In `@internal/interface/web/static/styles.css`:
- Line 1: Add an ignore for the generated CSS bundle so Stylelint targets source
SCSS only: update .stylelintrc.json (or add a .stylelintignore) to exclude the
directory "internal/interface/web/static/" (or the specific file
"internal/interface/web/static/styles.css"); ensure the ignore entry uses that
exact path string so Stylelint will skip the minified bundle and only lint the
source under "internal/interface/web/assets/" (no code changes to styles.css
needed).

In `@internal/interface/web/templates/components/feeTable.templ`:
- Around line 149-155: Extract the duplicated Ark txid/explorer HTML into a
shared template helper (e.g., arkTxRow or renderArkTx) that accepts arkTxid and
arkExplorerUrl, moves the conditional check for arkTxid != "" into that helper,
uses truncateTxid and templ.SafeURL inside it, and call this helper in place of
the duplicated block in feeTable.templ (both occurrences around the current
txid/explorer blocks). Ensure the helper renders the same tableLine("Tx ID",
truncateTxid(arkTxid)) and the explorer link with templ.SafeURL(arkExplorerUrl +
"/tx/" + arkTxid) so both places (the original and the other occurrence)
reference this single implementation.

In `@internal/interface/web/templates/pages/tx.templ`:
- Around line 14-34: The template TransferTxPendingContent currently accepts an
unused parameter arkExplorerUrl; remove arkExplorerUrl from the function
signature and update any callers (handlers or renderers that pass arkExplorerUrl
to TransferTxPendingContent) to stop supplying that argument, or alternatively
use arkExplorerUrl where appropriate (e.g., pass it into
components.PendingTransferTable) if you intend to display explorer links—ensure
consistency between the TransferTxPendingContent signature and all places that
invoke it (search for TransferTxPendingContent usages and adjust their argument
lists accordingly).
🪄 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: 241e2332-5860-4931-9877-dd411d81c880

📥 Commits

Reviewing files that changed from the base of the PR and between 03f8c40 and 2f8fc5d.

⛔ Files ignored due to path filters (1)
  • internal/interface/web/static/styles.css.map is excluded by !**/*.map
📒 Files selected for processing (10)
  • internal/interface/web/handlers.go
  • internal/interface/web/static/styles.css
  • internal/interface/web/templates/components/feeTable.templ
  • internal/interface/web/templates/components/feeTable_templ.go
  • internal/interface/web/templates/components/tx.templ
  • internal/interface/web/templates/components/tx_templ.go
  • internal/interface/web/templates/pages/tx.templ
  • internal/interface/web/templates/pages/tx_templ.go
  • internal/interface/web/types/transfers.go
  • internal/interface/web/utils.go

Comment on lines +149 to +154
if arkTxid != "" {
@tableLine("Tx ID", truncateTxid(arkTxid))
<div class="flex justify-between mb-2">
<p class="text-white/50 uppercase">explorer</p>
<a href={templ.SafeURL(arkExplorerUrl + "/tx/" + arkTxid)} target="_blank" rel="noopener noreferrer" class="text-blue hover:underline">View</a>
</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Guard explorer row on arkExplorerUrl too.

At Line 149 and Line 173, the condition only checks arkTxid. If arkExplorerUrl is empty/misconfigured, the link becomes a relative /tx/{txid} route and points to the wrong destination.

💡 Suggested fix
-    if arkTxid != "" {
+    if arkTxid != "" {
       `@tableLine`("Tx ID", truncateTxid(arkTxid))
-      <div class="flex justify-between mb-2">
-        <p class="text-white/50 uppercase">explorer</p>
-        <a href={templ.SafeURL(arkExplorerUrl + "/tx/" + arkTxid)} target="_blank" rel="noopener noreferrer" class="text-blue hover:underline">View</a>
-      </div>
+      if arkExplorerUrl != "" {
+        <div class="flex justify-between mb-2">
+          <p class="text-white/50 uppercase">explorer</p>
+          <a href={templ.SafeURL(arkExplorerUrl + "/tx/" + arkTxid)} target="_blank" rel="noopener noreferrer" class="text-blue hover:underline">View</a>
+        </div>
+      }
     }
-    if arkTxid != "" {
+    if arkTxid != "" {
       `@tableLine`("Tx ID", truncateTxid(arkTxid))
-      <div class="flex justify-between mb-2">
-        <p class="text-white/50 uppercase">explorer</p>
-        <a href={templ.SafeURL(arkExplorerUrl + "/tx/" + arkTxid)} target="_blank" rel="noopener noreferrer" class="text-blue hover:underline">View</a>
-      </div>
+      if arkExplorerUrl != "" {
+        <div class="flex justify-between mb-2">
+          <p class="text-white/50 uppercase">explorer</p>
+          <a href={templ.SafeURL(arkExplorerUrl + "/tx/" + arkTxid)} target="_blank" rel="noopener noreferrer" class="text-blue hover:underline">View</a>
+        </div>
+      }
     }

Also applies to: 173-178

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/interface/web/templates/components/feeTable.templ` around lines 149
- 154, The explorer "View" row should only render when both arkTxid and
arkExplorerUrl are set; update the conditional guarding the block that renders
the explorer link (the branch using arkTxid, truncateTxid and
templ.SafeURL(arkExplorerUrl + "/tx/" + arkTxid)) to require arkExplorerUrl !=
"" as well (e.g., check arkTxid != "" && arkExplorerUrl != "") so you don't
produce a relative /tx/{txid} link; apply the same change to the second
occurrence around lines rendering the same explorer link.

@arkanaai
Copy link
Copy Markdown
Contributor

arkanaai bot commented Mar 28, 2026

🔍 Arkana Review — fulmine#385 Ark Txid + Explorer Link

Supersedes #368 — rebased, conflicts resolved, review feedback addressed.

What it does

Displays the Ark transaction ID (truncated) with a direct explorer link in received/sent transaction detail tables. Explorer URL is network-aware via getArkExplorerUrl().

Positives

  • Safe truncation: truncateTxid() guards against panic on short/empty strings with len(txid) > 12 check
  • Network-aware URLs: getArkExplorerUrl() returns correct base URL for mainnet vs signet
  • Proper security: templ.SafeURL() for XSS protection, target="_blank" rel="noopener noreferrer" on external links
  • Clean plumbing: arkExplorerUrl threaded through handlers → pages → components without shortcuts
  • ArkTxid field added to Transfer type, populated from tx.ArkTxid in toTransfer()

Minor notes

🟡 Signet explorer path
getArkExplorerUrl("signet") returns https://arkade.space/signet — confirm this path actually resolves on the explorer. If it's https://signet.arkade.space instead, this will 404 on the explorer link.

🟡 Default case in getArkExplorerUrl
The default case returns mainnet URL. If someone runs on testnet/regtest, they'll get mainnet explorer links that won't resolve. Consider returning empty string for unknown networks (which would hide the link via the if arkTxid != "" guard already in place — though you'd also need if arkExplorerUrl != "").

Nit

  • The compiled styles.css diff is ~60KB of minified CSS. Consider adding internal/interface/web/static/styles.css to .gitattributes as linguist-generated to keep PR diffs focused on source changes.

Cross-repo

No cross-repo impact — self-contained fulmine UI change.

- Display truncated Ark transaction ID and explorer link in received/sent tx tables
- Make Ark explorer URL configurable via getArkExplorerUrl() instead of hardcoding
- Add safe truncation to prevent panic on short txid strings
@arkanaai
Copy link
Copy Markdown
Contributor

arkanaai bot commented Mar 28, 2026

🔍 Arkana PR Review — fulmine#385

Author: kukks | Type: Feature (UI enhancement)

Summary

Adds Ark transaction ID display and explorer link to the transaction details page in Fulmine. Addresses review feedback from previous PR #368 (superseded).

What Changed

  • handlers.go: Added arkExplorerUrl parameter threading through getTransfer(), getTx(), and claimTx(). New getArkExplorerUrl() makes explorer URL network-aware
  • types/transfers.go: Added ArkTxid field to Transfer struct
  • feeTable.templ/feeTable_templ.go: ReceivedTxTable and SentTxTable now accept arkTxid and arkExplorerUrl params. Conditionally renders truncated txid + explorer link when arkTxid != ""
  • tx.templ/tx_templ.go: Updated template calls to pass new params
  • utils.go: Added getArkExplorerUrl() helper (network → explorer URL mapping)
  • Helper: truncateTxid() safely truncates txids >12 chars with "..." suffix (handles short/empty strings)

Review

Positive:

  • ✅ Network-aware explorer URL — not hardcoded, adapts to mainnet/signet/regtest
  • ✅ Safe truncation with bounds check (addresses CodeRabbit feedback)
  • ✅ Conditional rendering — only shows txid section when ArkTxid is non-empty
  • target="_blank" rel="noopener noreferrer" on explorer links — correct security practice
  • ✅ Clean supersession of Add txid and explorer link in tx details page #368 with all review comments addressed

Notes:

  • The CSS diff is a minified stylesheet rebuild — no manual style changes, just a recompile artifact
  • templ.SafeURL() is used for the explorer link — good XSS protection via the templ framework
  • The toTransfer() function maps tx.ArkTxidtypes.Transfer.ArkTxid, which means the field must be available from the client SDK types

Verdict

LGTM — clean feature addition with proper security practices (SafeURL, noopener, conditional rendering, bounds checking).

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
internal/interface/web/templates/pages/tx_templ.go (1)

49-164: arkExplorerUrl parameter accepted but unused in TransferTxPendingContent.

TransferTxPendingContent accepts arkExplorerUrl in its signature (line 49) but doesn't pass it to any child component. The function renders components.PendingTransferTable(tx.CreatedAt, nextClaim) (line 146) which doesn't receive the explorer URL.

If this is intentional (pending transfers don't need explorer links until settled), consider documenting this or removing the unused parameter to keep the API clean. If the explorer link should be shown for pending transfers, the parameter needs to be wired through.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/interface/web/templates/pages/tx_templ.go` around lines 49 - 164,
TransferTxPendingContent currently accepts arkExplorerUrl but never uses it;
either remove the unused parameter or forward it to the child that would render
explorer links. To fix, update the TransferTxPendingContent signature to drop
arkExplorerUrl (and all call sites) if no explorer link is needed, or modify the
call to components.PendingTransferTable(tx.CreatedAt, nextClaim) to instead pass
arkExplorerUrl (e.g., components.PendingTransferTable(tx.CreatedAt, nextClaim,
arkExplorerUrl)) and update the PendingTransferTable function signature and its
render logic to use the explorer URL; ensure changes reference the
TransferTxPendingContent function and components.PendingTransferTable to locate
code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/interface/web/templates/pages/tx_templ.go`:
- Around line 49-164: TransferTxPendingContent currently accepts arkExplorerUrl
but never uses it; either remove the unused parameter or forward it to the child
that would render explorer links. To fix, update the TransferTxPendingContent
signature to drop arkExplorerUrl (and all call sites) if no explorer link is
needed, or modify the call to components.PendingTransferTable(tx.CreatedAt,
nextClaim) to instead pass arkExplorerUrl (e.g.,
components.PendingTransferTable(tx.CreatedAt, nextClaim, arkExplorerUrl)) and
update the PendingTransferTable function signature and its render logic to use
the explorer URL; ensure changes reference the TransferTxPendingContent function
and components.PendingTransferTable to locate code to change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e2900647-02e0-440a-8340-d5e98ce96dfe

📥 Commits

Reviewing files that changed from the base of the PR and between 2f8fc5d and c09ce16.

⛔ Files ignored due to path filters (1)
  • internal/interface/web/static/styles.css.map is excluded by !**/*.map
📒 Files selected for processing (10)
  • internal/interface/web/handlers.go
  • internal/interface/web/static/styles.css
  • internal/interface/web/templates/components/feeTable.templ
  • internal/interface/web/templates/components/feeTable_templ.go
  • internal/interface/web/templates/components/tx.templ
  • internal/interface/web/templates/components/tx_templ.go
  • internal/interface/web/templates/pages/tx.templ
  • internal/interface/web/templates/pages/tx_templ.go
  • internal/interface/web/types/transfers.go
  • internal/interface/web/utils.go
✅ Files skipped from review due to trivial changes (1)
  • internal/interface/web/types/transfers.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • internal/interface/web/templates/pages/tx.templ
  • internal/interface/web/templates/components/tx_templ.go
  • internal/interface/web/templates/components/feeTable_templ.go
  • internal/interface/web/templates/components/feeTable.templ

The refillFulmine setup crashes with "no funds to settle" when the
faucet TX hasn't confirmed before Settle is called. Retry up to 3
times with 5s backoff to give the arkd server time to process funds.
@arkanaai
Copy link
Copy Markdown
Contributor

arkanaai bot commented Mar 28, 2026

🔍 Arkana PR Review

PR: Add txid and explorer link in tx details page

Overview

Adds Ark transaction ID display and explorer link to the transfer/transaction detail pages. Supersedes #368 with conflict resolution and review feedback addressed.

✅ What looks good

  • Network-aware explorer URLs via getArkExplorerUrl(): mainnet → explorer.arkade.sh, signet → explorer.signet.arkade.sh, mutinynet → mutinynet.signet.arkade.sh, default → localhost:7080. Clean switch-case pattern.
  • Safe truncation helper truncateTxid(): Only truncates if len > 12, prevents panic on short/empty strings. Addresses CodeRabbit feedback.
  • Conditional display: Txid and explorer link only shown when arkTxid != "" — no empty UI elements for transactions without Ark txids.
  • Consistent plumbing: arkExplorerUrl parameter threaded through all transfer rendering paths (VHTLC, redeem, payment, reclaim, claim).
  • toTransfer() correctly maps tx.ArkTxid from the client types.

📋 Observations

  1. text-blue CSS class: The explorer link uses class="text-blue hover:underline" but I don't see text-blue defined in the Tailwind config or the existing styles. This may render as default text color. Should it be text-orange (matching the existing link color scheme) or add a text-blue utility to the Tailwind config?

  2. Compiled _templ.go files: The diff includes both .templ source and the generated _templ.go output. This is fine if that's the project's convention (commit generated code), but worth confirming templ generate was run against the final .templ source.

  3. Minified CSS diff: The styles.css diff is a minified blob change. The meaningful additions appear to be the hover:underline utility. Consider whether the CSS should be regenerated as part of CI to avoid committing build artifacts.

  4. Explorer URL for regtest: Returns http://localhost:7080 which is reasonable for local dev. No explorer would exist in regtest, so the link would 404 — acceptable UX.

🔒 Security

  • templ.SafeURL() used for the explorer link construction — prevents injection ✅
  • target="_blank" rel="noopener noreferrer" on external links — correct ✅
  • No user-controlled input in the URL path beyond the txid (which comes from the server) ✅

Cross-repo impact

None — self-contained Fulmine UI change. No protocol or SDK impact.

One minor observation (non-blocking): verify text-blue renders correctly. Otherwise LGTM.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/test/e2e/main_test.go`:
- Around line 107-113: The test retries f.Settle(ctx, &pb.SettleRequest{})
unconditionally which can cause duplicate commits because SettleRequest has no
idempotency key; change the retry logic in main_test.go to only retry on
explicit retryable transport errors (e.g., transient network/timeouts) by
inspecting the returned error before retrying, and add contextual error wrapping
that includes the number of attempts and the last error when returning failure;
alternatively, implement an idempotency guard in the service layer where
ArkClient.Settle is invoked (the method calling ArkClient.Settle in the service
code) so duplicate server-side settlements become no-ops, and ensure error
messages include "Settle failed after N attempts: <last error>" to satisfy
descriptive error guidelines.
🪄 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: 50d5e7f4-0705-40ba-a342-698d916f7970

📥 Commits

Reviewing files that changed from the base of the PR and between c09ce16 and aaddaca.

📒 Files selected for processing (1)
  • internal/test/e2e/main_test.go

Comment on lines +107 to +113
for attempts := 0; attempts < 3; attempts++ {
_, err = f.Settle(ctx, &pb.SettleRequest{})
if err == nil {
break
}
time.Sleep(5 * time.Second)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Inspect Settle implementations/callers and nearby guards.
rg -n -C3 --type=go '\bSettle\s*\('

# 2) Check Ark client interface/contracts for idempotency wording.
rg -n -C3 --type=go 'type .*ArkClient.*interface|ArkClient.*Settle'

# 3) Check proto request shape for idempotency fields.
fd -e proto | xargs rg -n -C2 'rpc Settle|message SettleRequest|idempot|nonce|request_id'

Repository: ArkLabsHQ/fulmine

Length of output: 18883


Guard Settle retries with idempotency-safe conditions and improve error messaging.

Lines 107–113 retry f.Settle(...) unconditionally up to 3 times. The SettleRequest proto (api-spec/protobuf/fulmine/v1/service.proto:235) is empty with no idempotency key, request ID, or nonce. If attempt 1 commits server-side but returns a transport error, attempts 2–3 can trigger duplicate settlement. The service layer (internal/core/application/service.go:754–760) makes a direct call to ArkClient.Settle(ctx) with no state guard or idempotency check.

Additionally, after the loop exits with an error, the raw error is returned without context about retry attempts, violating "always use descriptive error messages in error handling" from the coding guidelines.

Suggested fix
 for attempts := 0; attempts < 3; attempts++ {
 	_, err = f.Settle(ctx, &pb.SettleRequest{})
 	if err == nil {
 		break
 	}
+	if !isRetryableSettleErr(err) {
+		return fmt.Errorf("settle failed (non-retryable): %w", err)
+	}
 	time.Sleep(5 * time.Second)
 }
+if err != nil {
+	return fmt.Errorf("settle failed after 3 attempts: %w", err)
+}

Either restrict retries to explicitly retryable errors only, or ensure SettleRequest supports idempotency keys and the service layer guards against duplicate settlement.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/test/e2e/main_test.go` around lines 107 - 113, The test retries
f.Settle(ctx, &pb.SettleRequest{}) unconditionally which can cause duplicate
commits because SettleRequest has no idempotency key; change the retry logic in
main_test.go to only retry on explicit retryable transport errors (e.g.,
transient network/timeouts) by inspecting the returned error before retrying,
and add contextual error wrapping that includes the number of attempts and the
last error when returning failure; alternatively, implement an idempotency guard
in the service layer where ArkClient.Settle is invoked (the method calling
ArkClient.Settle in the service code) so duplicate server-side settlements
become no-ops, and ensure error messages include "Settle failed after N
attempts: <last error>" to satisfy descriptive error guidelines.

@arkanaai
Copy link
Copy Markdown
Contributor

arkanaai bot commented Mar 28, 2026

🔍 Arkana Review — fulmine#385

Scope: Add txid display and ark explorer link to transaction detail pages.

What Changed

  • handlers.go: Added arkExplorerUrl parameter threaded through getTransfer() and all callers (getTx, claimTx)
  • New getArkExplorerUrl() helper (not shown in diff but referenced)
  • feeTable.templ / feeTable_templ.go:
    • New truncateTxid() helper (12 char + "...")
    • ReceivedTxTable and SentTxTable gain arkTxid and arkExplorerUrl params
    • Explorer link rendered with target="_blank" rel="noopener noreferrer"
  • transfer.templ / page templates: Updated to pass the new params through
  • CSS: Added hover:underline utility class

Analysis

  1. truncateTxid — Simple and correct. 12 chars is reasonable for display (shows enough for visual identification).

  2. Securitytempl.SafeURL() used for the explorer link construction (arkExplorerUrl + "/tx/" + arkTxid). This is safe because templ's SafeURL validates the scheme. The rel="noopener noreferrer" on external links is correct.

  3. text-blue class — The links use class="text-blue hover:underline" but I don't see text-blue defined in the Tailwind config or CSS. The minified CSS diff doesn't show a new text-blue rule being added. This might render with the default text color instead of blue. Worth verifying visually.

  4. Conditional rendering — Explorer link only shows when arkTxid != "", which is correct for transfers that may not have a txid yet (pending state).

  5. getArkExplorerUrl — Referenced but not in the diff. Presumably maps network names to explorer URLs (e.g., https://explorer.arklabs.to for mainnet). Worth confirming this function handles all network variants correctly.

Verdict

Clean feature addition. The thread of arkExplorerUrl through the handler chain is well-structured. Check the text-blue class renders correctly.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/interface/web/handlers.go`:
- Around line 841-842: The Ark explorer URL mapping may be using the
non-official domain; inspect and update the getArkExplorerUrl function to ensure
it returns the officially documented Arkade explorer domain for each network
(use data.Network.Name to branch), or explicitly verify that
https://explorer.arkade.sh redirects to https://arkade.space and, if not
equivalent, replace the mapping to use https://arkade.space for mainnet and any
other correct domains for testnets so the handler builds consistent, documented
explorer links.
🪄 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: 940b68d0-b959-4b28-addc-218773b18418

📥 Commits

Reviewing files that changed from the base of the PR and between aaddaca and 018fc5f.

📒 Files selected for processing (3)
  • internal/interface/web/handlers.go
  • internal/interface/web/templates/components/tx.templ
  • internal/interface/web/templates/components/tx_templ.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/interface/web/templates/components/tx.templ

Comment on lines 841 to +842
explorerUrl := getExplorerUrl(data.Network.Name)
arkExplorerUrl := getArkExplorerUrl(data.Network.Name)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the actual URL returned for each network
rg -n -A15 'func getArkExplorerUrl' --type go

# Also check if arkade.space is referenced elsewhere
rg -n 'arkade\.space' --type go

Repository: ArkLabsHQ/fulmine

Length of output: 1007


🏁 Script executed:

# Search for test plan files or documentation
git ls-files | grep -E "(test|plan|spec|doc)" | head -20

Repository: ArkLabsHQ/fulmine

Length of output: 992


🏁 Script executed:

# Search for any references to arkade in test files or documentation
rg -n "arkade" --type md,txt --max-count 20

Repository: ArkLabsHQ/fulmine

Length of output: 91


🏁 Script executed:

# Look for test files that might reference explorer URLs
fd -type f -name "*.go" | xargs rg -l "explorer|arkade" | grep -i test

Repository: ArkLabsHQ/fulmine

Length of output: 480


🏁 Script executed:

# Find all markdown and documentation files
find . -type f \( -name "*.md" -o -name "*.rst" -o -name "*.txt" \) | head -30

Repository: ArkLabsHQ/fulmine

Length of output: 179


🏁 Script executed:

# Search for any references to arkade.space or test plan content
rg -i "arkade\.space|test.*plan" --max-count 30

Repository: ArkLabsHQ/fulmine

Length of output: 43


🏁 Script executed:

# Search for PR or test documentation in common locations
ls -la . | grep -iE "readme|test|plan|doc"
cat README.md 2>/dev/null | head -50

Repository: ArkLabsHQ/fulmine

Length of output: 2262


🏁 Script executed:

# Check the docs for test plan information
cat docs/swaps.regtest.md

Repository: ArkLabsHQ/fulmine

Length of output: 6127


🏁 Script executed:

# Search for arkade URLs in test files
rg -n "explorer|arkade" internal/test/e2e/

Repository: ArkLabsHQ/fulmine

Length of output: 1760


🏁 Script executed:

# Check internal/interface/web/README.md for any relevant documentation
cat internal/interface/web/README.md

Repository: ArkLabsHQ/fulmine

Length of output: 3691


🌐 Web query:

arkade.space explorer.arkade.sh relationship

💡 Result:

arkade.space is the Arkade Protocol Blockchain Explorer, used to browse transactions, addresses, and VTXOs on the Arkade network. explorer.arkade.sh appears to be an alternative or related subdomain for the same or similar explorer functionality for the Arkade protocol, though direct confirmation of its operation was not found in search results. Arkade is a programmable execution layer for Bitcoin, enabling offchain financial applications with onchain settlement, as documented at docs.arkadeos.com. The explorer at arkade.space provides a user interface for exploring the Arkade blockchain data, including VTXOs managed by arkd servers.

Citations:


Verify that Ark explorer URLs are consistent with official documentation.

The code uses https://explorer.arkade.sh for mainnet, but public documentation identifies https://arkade.space as the Arkade Protocol Blockchain Explorer. Confirm these domains are equivalent (e.g., via redirect) or update the URL mapping to use the official explorer domain.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/interface/web/handlers.go` around lines 841 - 842, The Ark explorer
URL mapping may be using the non-official domain; inspect and update the
getArkExplorerUrl function to ensure it returns the officially documented Arkade
explorer domain for each network (use data.Network.Name to branch), or
explicitly verify that https://explorer.arkade.sh redirects to
https://arkade.space and, if not equivalent, replace the mapping to use
https://arkade.space for mainnet and any other correct domains for testnets so
the handler builds consistent, documented explorer links.

Derive Ark txid from existing Txid and Explorable fields instead of
storing it separately. When Explorable is false, Txid is the Ark
transaction ID to use for explorer links.
@arkanaai
Copy link
Copy Markdown
Contributor

arkanaai bot commented Mar 28, 2026

🔍 Arkana Review — fulmine#385 (explorer-link, fba42c3)

Summary: Adds Ark transaction ID display + explorer link to sent/received transaction detail pages. Network-aware explorer URL via getArkExplorerUrl().

Architecture

Clean approach — threading arkExplorerUrl through the handler → template chain alongside the existing explorerUrl. All call sites in getTx() and claimTx() are updated consistently.

Findings

1. truncateTxid boundary (nit): The helper truncates at 12 chars. Since txids are always 64 hex chars this is safe, but the > 12 check is good defensive coding for edge cases (empty string, partial data). ✅

2. getArkExplorerUrl network mapping:

  • bitcoinexplorer.arkade.sh
  • signetexplorer.signet.arkade.sh
  • mutinynetmutinynet.signet.arkade.sh
  • default → localhost:7080

Sensible defaults. No testnet mapping, but that matches getExplorerUrl which also has no testnet case.

3. templ.SafeURL usage: The explorer link is constructed via templ.SafeURL(arkExplorerUrl + "/tx/" + arkTxid). The arkExplorerUrl is fully controlled (from getArkExplorerUrl) and arkTxid is a hex string from the server — no injection risk here. ✅

4. CSS diff: The minified CSS shows a rebuilt tailwind output — the only meaningful addition is .hover\:underline:hover which is used by the new explorer links. Build artifact looks correct.

5. Both .templ and _templ.go updated in sync.

Cross-repo note

The arkade-explorer repo (ArkLabsHQ/arkade-explorer#11) is the target for these explorer links — make sure the /tx/:txid route is live before the fulmine release that includes this.

Verdict: Clean feature addition, no security concerns. LGTM.

The TestSettleVHTLCByDelegateRefundWithOutpoint test consistently fails
with "no funds to settle" because it runs last and the arkd server has
been drained by previous tests. Call refillArkd() at the start of this
test to ensure sufficient funds.
@arkanaai
Copy link
Copy Markdown
Contributor

arkanaai bot commented Mar 28, 2026

Iterative review — new commits since last review

aaddaca — Retry settle in e2e test setup to handle timing flakes
Good fix. The faucet TX confirmation race was a known flake source. 3 retries with 5s backoff is reasonable.

fba42c3 — Remove ArkTxid field from Transfer struct
Clean refactor. Deriving the Ark txid from existing Txid/Explorable fields via arkTxid() is better than storing it separately — less data duplication, and the semantics are clear: when Explorable is false, Txid is the Ark transaction ID.

b0473c5 — Refill arkd before last e2e test to prevent fund depletion
TestSettleVHTLCByDelegateRefundWithOutpoint runs last and was hitting "no funds to settle" because prior tests drained the server. refillArkd() at test start + the threshold bump 5.010.0 BTC should prevent this.

All three test stability commits look correct. No security concerns.

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