Fix tron transaction status update#9388
Conversation
📝 WalkthroughWalkthroughThe PR updates the TronKit version reference and changes transaction record loading to replay reload requests received while a page load is in progress. ChangesTronKit version update
Transaction reload handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@walletkit/src/main/java/io/horizontalsystems/walletkit/modules/transactions/TransactionRecordRepository.kt`:
- Line 38: Update invalidateAdapters() to also reset the reloadRequested
AtomicBoolean when resetting loading and loadedPageNumber, preventing an
in-flight load from scheduling a spurious loadItems(0).
- Around line 235-237: Update the reload-request replay logic in
TransactionRecordRepository so it preserves the caller’s intended page:
handleUpdates must replay the page represented by loadedPageNumber, while
reload() and set() must replay from page 1 when their load is deferred by an
in-flight request. Track this intent with a distinct flag or equivalent
parameter, and have the replay consume it without changing normal pagination
behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ffc8b7c7-3477-4a27-aed8-adadc240eb14
📒 Files selected for processing (2)
gradle/libs.versions.tomlwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/transactions/TransactionRecordRepository.kt
| private var loadedPageNumber = 0 | ||
| private val items = CopyOnWriteArrayList<TransactionRecord>() | ||
| private val loading = AtomicBoolean(false) | ||
| private val reloadRequested = AtomicBoolean(false) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Clear reloadRequested in invalidateAdapters().
invalidateAdapters() (line 248) sets loading.set(false) but does not clear reloadRequested. If a load is in progress when invalidateAdapters() is called, the in-flight coroutine's finally block may see reloadRequested == true and trigger a spurious loadItems(0) (since invalidateAdapters() resets loadedPageNumber = 0).
🛡️ Proposed fix
override fun invalidateAdapters() {
unsubscribeFromUpdates()
adaptersMap.values.forEach(TransactionAdapterWrapper::clear)
adaptersMap.clear()
items.clear()
loadedPageNumber = 0
loading.set(false)
+ reloadRequested.set(false)
allLoaded.set(false)
selectedWallet = null
itemsSubject.onNext(emptyList())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private val reloadRequested = AtomicBoolean(false) | |
| override fun invalidateAdapters() { | |
| unsubscribeFromUpdates() | |
| adaptersMap.values.forEach(TransactionAdapterWrapper::clear) | |
| adaptersMap.clear() | |
| items.clear() | |
| loadedPageNumber = 0 | |
| loading.set(false) | |
| reloadRequested.set(false) | |
| allLoaded.set(false) | |
| selectedWallet = null | |
| itemsSubject.onNext(emptyList()) | |
| } |
🤖 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
`@walletkit/src/main/java/io/horizontalsystems/walletkit/modules/transactions/TransactionRecordRepository.kt`
at line 38, Update invalidateAdapters() to also reset the reloadRequested
AtomicBoolean when resetting loading and loadedPageNumber, preventing an
in-flight load from scheduling a spurious loadItems(0).
| if (reloadRequested.compareAndSet(true, false)) { | ||
| loadItems(loadedPageNumber) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Replay uses loadedPageNumber — wrong page for reload() and set() paths.
The replay calls loadItems(loadedPageNumber), which is correct for handleUpdates (refresh the page just loaded). However, reload() (line 187) and set() (line 170) call loadItems(1) with the intent of starting from page 1. If a load is already in progress when either is called, loadItems(1) returns early and sets reloadRequested = true. When the in-flight load finishes, loadedPageNumber has been set to the in-flight page (e.g., 2 or 3), so the replay loads that page instead of page 1. This causes incorrect pagination — the user sees items from the wrong page after a filter change or explicit reload.
The two call paths have different replay intents: handleUpdates wants the page just loaded (loadedPageNumber at replay time), while reload()/set() want page 1. Consider distinguishing them, e.g., with a fromStart parameter or a separate flag:
🔧 Proposed fix using a `fromStart` parameter
private val reloadRequested = AtomicBoolean(false)
+private val reloadFromStartRequested = AtomicBoolean(false)
// ...
override fun reload() {
adaptersMap.forEach { (_, transactionAdapterWrapper) ->
transactionAdapterWrapper.reload()
}
unsubscribeFromUpdates()
allLoaded.set(false)
- loadItems(1)
+ loadItems(1, fromStart = true)
subscribeForUpdates()
}
// In set():
if (reload) {
unsubscribeFromUpdates()
allLoaded.set(false)
- loadItems(1)
+ loadItems(1, fromStart = true)
subscribeForUpdates()
}
-private fun loadItems(page: Int) {
+private fun loadItems(page: Int, fromStart: Boolean = false) {
if (!loading.compareAndSet(false, true)) {
- reloadRequested.set(true)
+ if (fromStart) reloadFromStartRequested.set(true) else reloadRequested.set(true)
return
}
// ...
} finally {
loading.set(false)
+ if (reloadFromStartRequested.compareAndSet(true, false)) {
+ loadItems(1, fromStart = true)
+ } else if (reloadRequested.compareAndSet(true, false)) {
- if (reloadRequested.compareAndSet(true, false)) {
loadItems(loadedPageNumber)
}
}
}🤖 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
`@walletkit/src/main/java/io/horizontalsystems/walletkit/modules/transactions/TransactionRecordRepository.kt`
around lines 235 - 237, Update the reload-request replay logic in
TransactionRecordRepository so it preserves the caller’s intended page:
handleUpdates must replay the page represented by loadedPageNumber, while
reload() and set() must replay from page 1 when their load is deferred by an
in-flight request. Track this intent with a distinct flag or equivalent
parameter, and have the replay consume it without changing normal pagination
behavior.
2 participants
#9379
Summary by CodeRabbit