Skip to content

Conversation

dlipicar
Copy link
Contributor

@dlipicar dlipicar commented Aug 18, 2025

…callback

Fixes status-im/status-desktop#18568

The collectibles controller commands were hard to follow and the threading model was quite messy, causing issues like the one linked. This PR reworks the controller into 3 parts, which were moved to a separate package:

  • Loader (replaces loadOwnedCollectiblesCommand): in charge of refreshing the full list of owned collectibles for a given Account+ChainID, storing the results in the db and emitting events about the load progress.
  • PeriodicalLoader (replaces periodicRefreshOwnedCollectiblesCommand): in charge of running the Loaders at a given period for a given Account+ChainID
  • Controller: In charge of starting/restarting/stopping a PeriodicalLoader for each wallet Account+ChainID

WalletEvent sending was extracted to the Service, hopefully we'll get rid of those in the near future.

@status-im-auto
Copy link
Member

status-im-auto commented Aug 18, 2025

Jenkins Builds

Click to see older builds (66)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 8fac873 #1 2025-08-18 13:23:54 ~2 min android 📦aar
✖️ 8fac873 #1 2025-08-18 13:23:57 ~2 min tests 📄log
✔️ 8fac873 #1 2025-08-18 13:24:10 ~3 min linux 📦zip
✔️ 8fac873 #1 2025-08-18 13:24:41 ~3 min macos 📦zip
✔️ 8fac873 #1 2025-08-18 13:25:03 ~3 min macos 📦zip
✔️ 8fac873 #1 2025-08-18 13:25:20 ~4 min ios 📦zip
✔️ 8fac873 #1 2025-08-18 13:25:58 ~4 min windows 📦zip
✔️ 8fac873 #1 2025-08-18 13:28:53 ~7 min tests-rpc 📄log
✔️ 8fac873 #1 2025-08-18 13:31:54 ~10 min linux 📦zip
✔️ d70b958 #2 2025-08-18 13:32:33 ~2 min linux 📦zip
✔️ d70b958 #2 2025-08-18 13:32:34 ~2 min android 📦aar
✔️ d70b958 #2 2025-08-18 13:34:29 ~4 min windows 📦zip
✖️ d70b958 #2 2025-08-18 13:34:37 ~4 min tests 📄log
✔️ d70b958 #2 2025-08-18 13:35:51 ~6 min macos 📦zip
✔️ d70b958 #2 2025-08-18 13:37:49 ~8 min macos 📦zip
✔️ d70b958 #2 2025-08-18 13:38:30 ~8 min ios 📦zip
✔️ d70b958 #2 2025-08-18 13:40:11 ~10 min linux 📦zip
✔️ d70b958 #2 2025-08-18 13:40:42 ~10 min tests-rpc 📄log
✔️ 775c08d #3 2025-08-21 23:07:26 ~2 min linux/status-go 📦zip
✔️ 775c08d #3 2025-08-21 23:07:51 ~3 min android 📦aar
✔️ 775c08d #3 2025-08-21 23:07:57 ~3 min macos/status-go 📦zip
✖️ 775c08d #3 2025-08-21 23:08:06 ~3 min tests 📄log
✔️ 775c08d #3 2025-08-21 23:09:08 ~4 min ios 📦zip
✔️ 775c08d #3 2025-08-21 23:09:41 ~4 min windows/status-go 📦zip
✔️ 775c08d #3 2025-08-21 23:13:25 ~8 min tests-rpc 📄log
✔️ 775c08d #3 2025-08-21 23:15:50 ~10 min linux/nwaku 📦zip
✔️ 0a54ddf #4 2025-08-21 23:10:10 ~2 min linux/status-go 📦zip
✔️ 0a54ddf #4 2025-08-21 23:10:31 ~2 min android 📦aar
✔️ 0a54ddf #4 2025-08-21 23:10:50 ~2 min macos/status-go 📦zip
✖️ 0a54ddf #4 2025-08-21 23:11:18 ~3 min tests 📄log
✔️ 0a54ddf #4 2025-08-21 23:14:02 ~4 min ios 📦zip
✔️ 0a54ddf #4 2025-08-21 23:14:20 ~4 min windows/status-go 📦zip
✔️ 0a54ddf #4 2025-08-21 23:21:16 ~7 min tests-rpc 📄log
✔️ f45dc88 #5 2025-08-21 23:17:36 ~2 min android 📦aar
✔️ f45dc88 #5 2025-08-21 23:18:06 ~2 min macos/status-go 📦zip
✔️ f45dc88 #5 2025-08-21 23:18:25 ~3 min linux/status-go 📦zip
✖️ f45dc88 #5 2025-08-21 23:18:29 ~3 min tests 📄log
✔️ f45dc88 #5 2025-08-21 23:19:19 ~4 min ios 📦zip
✔️ f45dc88 #5 2025-08-21 23:20:10 ~4 min windows/status-go 📦zip
✔️ f45dc88 #4 2025-08-21 23:24:02 ~8 min linux/nwaku 📦zip
✔️ f45dc88 #5 2025-08-21 23:28:18 ~6 min tests-rpc 📄log
✔️ 16490da #6 2025-08-21 23:27:55 ~2 min android 📦aar
✔️ 16490da #6 2025-08-21 23:28:14 ~2 min macos/status-go 📦zip
✔️ 16490da #6 2025-08-21 23:28:29 ~3 min linux/status-go 📦zip
✖️ 16490da #6 2025-08-21 23:28:33 ~3 min tests 📄log
✔️ 16490da #6 2025-08-21 23:29:26 ~4 min ios 📦zip
✔️ 16490da #6 2025-08-21 23:30:09 ~4 min windows/status-go 📦zip
✔️ 16490da #5 2025-08-21 23:34:05 ~8 min linux/nwaku 📦zip
✔️ 16490da #6 2025-08-21 23:35:10 ~6 min tests-rpc 📄log
✔️ 4d2d7f4 #7 2025-08-21 23:33:25 ~2 min android 📦aar
✔️ 4d2d7f4 #7 2025-08-21 23:33:59 ~2 min macos/status-go 📦zip
✔️ 4d2d7f4 #7 2025-08-21 23:34:13 ~3 min linux/status-go 📦zip
✔️ 4d2d7f4 #7 2025-08-21 23:35:10 ~4 min ios 📦zip
✔️ 4d2d7f4 #7 2025-08-21 23:35:57 ~4 min windows/status-go 📦zip
✔️ 4d2d7f4 #6 2025-08-21 23:41:45 ~7 min linux/nwaku 📦zip
✔️ 4d2d7f4 #7 2025-08-21 23:42:03 ~6 min tests-rpc 📄log
✔️ 4d2d7f4 #7 2025-08-21 23:54:35 ~23 min tests 📄log
✔️ cc707a9 #8 2025-08-22 02:53:21 ~2 min linux/status-go 📦zip
✔️ cc707a9 #8 2025-08-22 02:53:43 ~2 min macos/status-go 📦zip
✔️ cc707a9 #8 2025-08-22 02:54:02 ~3 min android 📦aar
✔️ cc707a9 #8 2025-08-22 02:55:00 ~4 min ios 📦zip
✔️ cc707a9 #8 2025-08-22 02:55:37 ~4 min windows/status-go 📦zip
✔️ cc707a9 #8 2025-08-22 02:58:11 ~7 min tests-rpc 📄log
✔️ cc707a9 #7 2025-08-22 02:59:02 ~8 min linux/nwaku 📦zip
✖️ cc707a9 #8 2025-08-22 03:14:23 ~23 min tests 📄log
cc707a9 #4 2025-08-22 08:29:18 ~34 sec macos/status-go 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 6dc5474 #9 2025-08-22 12:14:49 ~2 min linux/status-go 📦zip
✔️ 6dc5474 #9 2025-08-22 12:14:58 ~2 min android 📦aar
✔️ 6dc5474 #9 2025-08-22 12:15:15 ~3 min macos/status-go 📦zip
✔️ 6dc5474 #9 2025-08-22 12:17:32 ~5 min windows/status-go 📦zip
✔️ 6dc5474 #9 2025-08-22 12:17:44 ~5 min ios 📦zip
✔️ 6dc5474 #9 2025-08-22 12:20:35 ~8 min tests-rpc 📄log
✔️ 6dc5474 #8 2025-08-22 12:22:40 ~10 min linux/nwaku 📦zip
✔️ 6dc5474 #9 2025-08-22 12:37:11 ~24 min tests 📄log
81f3e46 #10 2025-08-27 20:40:43 ~1 min android 📄log
✔️ 81f3e46 #10 2025-08-27 20:42:21 ~2 min linux/status-go 📦zip
✔️ 81f3e46 #10 2025-08-27 20:43:04 ~3 min macos/status-go 📦zip
81f3e46 #10 2025-08-27 20:44:20 ~4 min ios 📄log
✔️ 81f3e46 #10 2025-08-27 20:44:31 ~4 min windows/status-go 📦zip
✔️ 81f3e46 #10 2025-08-27 20:49:22 ~9 min tests-rpc 📄log
✔️ 81f3e46 #9 2025-08-27 20:49:29 ~9 min linux/nwaku 📦zip
✔️ 81f3e46 #10 2025-08-27 21:04:30 ~24 min tests 📄log

@dlipicar dlipicar force-pushed the fix/collectibles-controller-loaders branch 2 times, most recently from d70b958 to 775c08d Compare August 21, 2025 23:04
@dlipicar dlipicar changed the title (WIP) fix_: rework collectibles commands into loaders, fix accounts change … fix: rework collectibles commands into loaders, fix accounts change … Aug 21, 2025
@dlipicar dlipicar force-pushed the fix/collectibles-controller-loaders branch 4 times, most recently from 16490da to 4d2d7f4 Compare August 21, 2025 23:30
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

❌ Patch coverage is 79.35484% with 160 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.81%. Comparing base (2c69b91) to head (81f3e46).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
...rvices/wallet/collectibles/ownership/controller.go 78.28% 56 Missing and 15 partials ⚠️
services/wallet/collectibles/service.go 66.66% 48 Missing and 6 partials ⚠️
services/wallet/collectibles/ownership/loader.go 87.42% 17 Missing and 4 partials ⚠️
...wallet/collectibles/ownership/periodical_loader.go 87.93% 12 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6845      +/-   ##
===========================================
- Coverage    58.91%   58.81%   -0.10%     
===========================================
  Files          824      825       +1     
  Lines       121822   122029     +207     
===========================================
+ Hits         71769    71776       +7     
- Misses       42572    42773     +201     
+ Partials      7481     7480       -1     
Flag Coverage Δ
functional 30.35% <53.80%> (-0.23%) ⬇️
unit 55.31% <79.35%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
services/wallet/collectibles/manager.go 29.92% <100.00%> (-0.78%) ⬇️
...ices/wallet/collectibles/ownership/ownership_db.go 56.52% <ø> (ø)
...wallet/collectibles/ownership/periodical_loader.go 87.93% <87.93%> (ø)
services/wallet/collectibles/ownership/loader.go 87.42% <87.42%> (ø)
services/wallet/collectibles/service.go 33.89% <66.66%> (+16.95%) ⬆️
...rvices/wallet/collectibles/ownership/controller.go 78.28% <78.28%> (ø)

... and 47 files with indirect coverage changes

@dlipicar dlipicar force-pushed the fix/collectibles-controller-loaders branch from 4d2d7f4 to cc707a9 Compare August 22, 2025 02:50
@dlipicar dlipicar marked this pull request as ready for review August 22, 2025 02:54
@dlipicar dlipicar force-pushed the fix/collectibles-controller-loaders branch from cc707a9 to 6dc5474 Compare August 22, 2025 12:11
@dlipicar dlipicar force-pushed the fix/collectibles-controller-loaders branch from 6dc5474 to 81f3e46 Compare August 27, 2025 20:39
@dlipicar dlipicar merged commit bfa6eba into develop Aug 27, 2025
20 of 22 checks passed
@dlipicar dlipicar deleted the fix/collectibles-controller-loaders branch August 27, 2025 21:06
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.

Collectibles-related crash when multiple quick account addition signals are received
4 participants