feat: add rwa data to tokens in tokens controller #7804
Conversation
oscarwroche
force-pushed
the
feat/sync-rwa-data-tokens-controller
branch
2 times, most recently
from
71c41ef to
b0f194e
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
| if (cachedToken?.rwaData) { | ||
| token.rwaData = cachedToken.rwaData; // Update the token RWA data | ||
| } |
There was a problem hiding this comment.
Cache unconditionally overwrites user-provided rwaData unlike name
Medium Severity
The cache propagation logic for rwaData unconditionally overwrites existing token data when cachedToken.rwaData exists, unlike the name handling which only updates when !token.name. This means user-provided rwaData (set via watchAsset or addToken) will be lost when the TokenListController:stateChange event fires with cached data. The condition if (cachedToken?.rwaData) is missing the !token.rwaData check that would preserve user-provided values.
oscarwroche
force-pushed
the
feat/sync-rwa-data-tokens-controller
branch
2 times, most recently
from
9a7d42f to
7cf3066
Compare
oscarwroche
changed the title
oscarwroche
force-pushed
the
feat/sync-rwa-data-tokens-controller
branch
from
bd29fa0 to
994090c
Compare
| } | ||
| if (cachedToken?.rwaData) { | ||
| token.rwaData = cachedToken.rwaData; // Update the token RWA data | ||
| } |
There was a problem hiding this comment.
Stale rwaData persists when cache no longer has it
Low Severity
The rwaData propagation logic only updates tokens when the cache has rwaData, but never clears it when the cache no longer has rwaData. If a token loses its RWA status in the API (cache updates without rwaData), the token retains stale rwaData. Since the code always overwrites rwaData when present (unlike name which checks !token.name), the intent appears to be keeping rwaData synchronized with the cache, but the clearing case is missing. This could display outdated RWA information (market hours, pause times) for tokens that are no longer RWA.
There was a problem hiding this comment.
This is on purpose
mcmire
left a comment
mcmire
left a comment
There was a problem hiding this comment.
One suggestion. Would be good to get input from @MetaMask/metamask-assets as well.
BTW, cp-XXX in PR titles for this repo doesn't have any effect — this pattern is only applicable to extension and mobile.
| // We rely on `window` to make requests | ||
| testEnvironment: '<rootDir>/jest.environment.js', | ||
|
|
||
| // Watchman isn't available in some environments (e.g. sandboxed CI/containers) |
There was a problem hiding this comment.
Nit: Should this be in its own PR? It doesn't seem to be related to the changes here and might get buried. It would also be good to understand what you were doing when you encountered the problem that this fixes.
There was a problem hiding this comment.
It's just codex that added this, good catch, will get rid of it :)
We would like to release a version of this package with these changes to cherry pick for last week's RC - do you know what the usual method for this would be ? @mcmire |
|
@oscarwroche Yup! So the process works like this:
|
|
@oscarwroche i'll publish a preview build , can you pls use it on the extension and check if everything is working as expected |
|
@metamaskbot publish-preview |
Akaryatrh
changed the title
| const tokens = updatedAllTokens[chainId as Hex][selectedAddress]; | ||
|
|
||
| for (const [, token] of Object.entries(tokens)) { | ||
| const cachedToken = chainData[token.address]; |
There was a problem hiding this comment.
For some reason, address from token is not lowercased (while those from chainData are), hence cachedToken returns undefined.
| const cachedToken = chainData[token.address.toLowerCase()]; |
This is maybe specific to RWA tokens. In the meantime we can apply this change but i guess there's an API where address is not forced to lowercase format.
4 participants
Explanation
rwaDataonto tokens when metadata is updated.rwaDatathroughaddTokenso suggested assets can carry RWA data.rwaDatafrom the supplied argument instead of token metadata (which doesn't containrwaData)References
Checklist
Note
Low Risk
Low risk: only threads an optional
rwaDatafield through token add flows and copies cachedrwaDataduring metadata refresh, without changing core balance/approval logic.Overview
Adds optional Real World Asset metadata (
rwaData) support toTokensControllertoken additions.addTokennow accepts an explicitrwaDataargument (used bywatchAsset/suggested-asset flows) and the TokenList cache sync now also propagates cachedrwaDataonto existing tokens when refreshing metadata. Changelog updated to note the new capability.Written by Cursor Bugbot for commit fca7da2. This will update automatically on new commits. Configure here.