Feature / Trending tokens

[sonytooo]

• Add trending tokens to the DappsController. Later the dapps ctrl is planned to be renamed to the more generic name ExploreController to match to FE functionality

[PetromirDev]

/review

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🔒 No security concerns identified

✅ No TODO sections

⚡ Recommended focus areas for review Mutable state exposure The trendingTokens getter returns the internal #trendingTokens array reference directly. External callers can mutate the array or its items, corrupting the controller's internal state and causing stale or inconsistent UI data. get trendingTokens(): TrendingToken[] { return this.#trendingTokens } Unhandled rejection from public method The public continuouslyUpdateTrendingTokens method is the RecurringTimeout callback. Its catch block mutates the interval timeout and then re-throws the error, so any direct caller receives an unhandled rejection instead of a safely emitted error. async continuouslyUpdateTrendingTokens() { if (this.#continuouslyUpdateTrendingTokensPromise) { await this.#continuouslyUpdateTrendingTokensPromise return } this.#continuouslyUpdateTrendingTokensPromise = this.#continuouslyUpdateTrendingTokens() .catch((err) => { this.#updateTrendingTokensInterval.updateTimeout({ timeout: TRENDING_TOKENS_FAILED_UPDATE_INTERVAL }) throw err }) .finally(() => { this.#continuouslyUpdateTrendingTokensPromise = undefined }) await this.#continuouslyUpdateTrendingTokensPromise } Parallel storage write risk The trending-tokens background loop awaits this.#storage.set('trending', ...). This can run concurrently with other storage writes in the same controller (e.g., the un-awaited void this.#storage.set('dappsV2', ...) in #syncDappsBlacklistedStatusWithPhishing). Per project invariants, parallel storage.set() calls risk corrupting persisted state. await this.#storage.set('trending', { updatedAt, tokens: this.#trendingTokens })

[PetromirDev]

Should we move it to continuous updates?

[PetromirDev]

Why do we rethrow the error here and not emit an error?

[PetromirDev]

Isn't this way too often? There are 30 tokens, yes, but we are updating the tokens regardless of whether the extension is active or not. Imo, we shoud run the interval while the extension is active and simply update on popup open if the last update is more than X minutes ago. If the interval runs less often then it will be okay to run it always (e.g., every 4+ hours)

[JIOjosBG]

i updated properties on the backend to reduce request size, you can merge this here #2529