fix: get confirmedTimeStamp

[jvsena42]

Closes #521

ROADMAP

Description

• Activity List: best time estimates in transfer items subtitle text (e.g.: (± 10d))
• Activity Details: transfer_pending status text with correct duration for 'confirms in X' text
• Check what other places are requiring our logic for displaying time estimates and fix as needed

Preview

transfer-from-spending.webm

transfer-from-savings.webm

QA Notes

Tested send on-chain with different speeds, but all have the same fee, so it is always displayed with same time

[Copilot]

Pull request overview

This PR fixes the display of time estimates for pending transfers in the Activity List and Activity Details screens. The changes address issue #521 by properly calculating confirmation time estimates based on fee rates instead of using placeholder values.

Key Changes:

• Added a new timestamp() extension function to properly handle confirmed timestamps for on-chain transactions
• Implemented getFeeDescription() method to calculate time estimates based on fee rates
• Updated Activity List and Activity Details to display proper duration estimates for pending transfers

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
ActivityRow.kt	Updated to use fee rate information from blocktank for calculating and displaying transfer duration estimates in activity list items
ActivityDetailScreen.kt	Modified to pass fee rates and display accurate pending transfer duration in status section
FeeRate.kt	Added new getFeeDescription() composable function to determine time estimates based on fee rate tiers
Activities.kt	Added timestamp() extension function to properly retrieve confirmed or creation timestamps for activities

[claude]

Code Review

Overall Assessment

Good implementation that addresses the issue requirements for dynamic time estimates. The code follows most project conventions and patterns. There are a few areas that need attention.

---

✅ Strengths

1. Clean extension function: The new Activity.timestamp() extension in Activities.kt:74-80 properly handles both Lightning and Onchain activities with correct fallback logic for confirmTimestamp.

2. Consistent pattern usage: The code follows existing patterns for accessing ViewModels via Compose locals (blocktankViewModel) and StateFlow collection.

3. Code reuse: Good use of the new Activity.timestamp() extension to replace duplicated timestamp logic in both ActivityRow.kt and ActivityDetailScreen.kt.

4. Composable function placement: FeeRate.getFeeDescription() is appropriately marked as @Composable since it uses stringResource().

---

⚠️ Issues & Recommendations

1. Null Safety Concern (Medium Priority)

Location: ActivityRow.kt:66-69, ActivityDetailScreen.kt:195-198

val blocktankInfo by blocktankViewModel?.info?.collectAsStateWithLifecycle() ?: remember {
    mutableStateOf(null)
}
val feeRates = blocktankInfo?.onchain?.feeRates

Issue: When feeRates is null, FeeRate.getFeeDescription() falls back to NORMAL, which returns "±20m". This might not reflect the actual transaction confirmation time if fee rate data is unavailable.

Recommendation: Consider handling the null case more explicitly:

• Add a loading state while fee rates are being fetched
• Or display a generic message like "Pending" when fee rates aren't available
• Log a warning when falling back to NORMAL

2. TODO Comments Should Be Removed or Tracked (Low Priority)

Location: ActivityRow.kt:326, ActivityDetailScreen.kt:964

// TODO remove this method after transifex update
private fun String.removeEstimationSymbol() = this.replace("±", "")

Issue: These TODO comments indicate temporary workarounds but lack:

• A timeline or issue reference
• Explanation of what "transifex update" means
• Why the ± symbol needs to be removed

Recommendation:

• Create a follow-up issue to track the transifex update and reference it in the comment
• Or if the strings are already updated in Transifex, remove the workaround now
• Add more context: // TODO(#ISSUE_NUMBER): Remove after transifex updates string resources to not include ± symbol

3. Missing Unit Tests (Medium Priority)

Issue: No unit tests were added for:

• Activity.timestamp() extension function
• FeeRate.getFeeDescription() and FeeRate.fromSatsPerVByte() logic
• The new time estimation logic

Recommendation: Add unit tests to verify:

// Test cases for Activities.kt
- timestamp() returns v1.timestamp for Lightning activities
- timestamp() returns v1.confirmTimestamp when confirmed = true and confirmTimestamp != null
- timestamp() falls back to v1.timestamp when confirmed = true but confirmTimestamp = null
- timestamp() returns v1.timestamp when confirmed = false

// Test cases for FeeRate.kt
- fromSatsPerVByte() correctly maps fee rates to enum values
- getFeeDescription() returns correct string for each fee rate tier
- getFeeDescription() handles null feeEstimates correctly

4. Potential String Resource Issue (Low Priority)

Location: String replacements in ActivityRow.kt:132,140 and ActivityDetailScreen.kt:745

.replace("{duration}", duration.removeEstimationSymbol())

Issue: Using .replace() for string formatting is fragile and doesn't support localization placeholders properly.

Recommendation: Consider using Android's string format arguments:

<string name="wallet__activity_transfer_spending_pending">From Savings (%1$s)</string>

stringResource(R.string.wallet__activity_transfer_spending_pending, duration)

This would be more robust and align with Android best practices.

5. Code Comment Missing (Low Priority)

Location: Activities.kt:74

The new Activity.timestamp() function would benefit from a KDoc comment explaining:

• When confirmTimestamp vs timestamp is used
• Why we fall back to timestamp when confirmTimestamp is null

---

🔒 Security

No security concerns identified.

---

⚡ Performance

No performance concerns. The changes are minimal and don't introduce any expensive operations.

---

📋 Code Style & Conventions

Follows CLAUDE.md guidelines:

• ✅ Uses extension functions appropriately
• ✅ Preserves string resources
• ✅ Uses @Composable annotation correctly
• ✅ Places modifiers in proper position
• ✅ Uses StateFlow with collectAsStateWithLifecycle()

Minor style note: The new extension function Activity.timestamp() is placed in a good location in Activities.kt, maintaining alphabetical/logical ordering with other Activity extensions.

---

🎯 Suggested Next Steps

1. Add unit tests for the new timestamp() extension and FeeRate methods
2. Clarify or resolve the TODO comments about transifex
3. Consider more explicit handling of null feeRates case
4. (Optional) Refactor string replacements to use proper format arguments

---

✅ Approval Recommendation

Approved with minor suggestions. The code correctly implements the required functionality and follows project patterns. The concerns raised are mostly about test coverage and code maintenance, not blocking issues. Consider addressing the recommendations in a follow-up PR if not in this one.

[ovitrif]

tAck