-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Graceful Perps icon fallback #24340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
| <!-- EAS Update configuration --> | ||
| <meta-data android:name="expo.modules.updates.UPDATES_CONFIGURATION_REQUEST_HEADERS_KEY" android:value="{"expo-channel-name":"rc"}"/> | ||
| <meta-data android:name="expo.modules.updates.EXPO_RUNTIME_VERSION" android:value="7.62.0" /> | ||
| <meta-data android:name="expo.modules.updates.EXPO_UPDATE_URL" android:value="https://u.expo.dev/" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EAS Update URL missing Expo project ID
High Severity
The EAS Update URL https://u.expo.dev/ is incomplete - it's missing the required Expo project UUID. According to ota.config.js, the URL format is https://u.expo.dev/${PROJECT_ID} where PROJECT_ID comes from the EXPO_PROJECT_ID environment variable. This appears to be accidentally committed output from running scripts/update-expo-channel.js without the environment variable set. Without the project ID, OTA updates won't be able to locate the correct Expo project, causing update checks to fail. These configuration changes also appear unrelated to the PR's stated purpose of implementing Perps icon fallback.
🔬 Verification Test
Test code:
// Verify the URL format expected by ota.config.js
const { UPDATE_URL, PROJECT_ID } = require('./ota.config.js');
console.log('PROJECT_ID:', PROJECT_ID || '(empty)');
console.log('UPDATE_URL:', UPDATE_URL);
console.log('URL is incomplete:', UPDATE_URL === 'https://u.expo.dev/');Command run:
node -e "const { UPDATE_URL, PROJECT_ID } = require('./ota.config.js'); console.log('PROJECT_ID:', PROJECT_ID || '(empty)'); console.log('UPDATE_URL:', UPDATE_URL); console.log('URL is incomplete:', UPDATE_URL === 'https://u.expo.dev/');"
Output:
PROJECT_ID: (empty)
UPDATE_URL: https://u.expo.dev/
URL is incomplete: true
Why this proves the bug: The output shows that without the EXPO_PROJECT_ID environment variable, the URL defaults to https://u.expo.dev/ (incomplete), which matches what was committed to the Android and iOS config files. The Expo Update URL requires a project UUID suffix to function correctly.
Additional Locations (1)
| ? useFallbackUrl | ||
| ? iconUrls.fallback | ||
| : iconUrls.primary | ||
| : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale fallback state causes wrong URL on symbol change
Low Severity
When the symbol changes after a primary URL failure, useFallbackUrl retains its true value from the previous symbol during the first render cycle because the reset happens in a useEffect (which runs after render). This causes imageUri to incorrectly use iconUrls.fallback instead of iconUrls.primary for the new symbol. The component then re-renders correctly after the effect runs, but this creates an unnecessary network request to the fallback URL and could cause a brief visual inconsistency if the fallback loads quickly. The state reset could be made synchronous by calculating the initial URL based on symbol change detection rather than relying on a post-render effect.
🔬 Verification Test
Test code:
// This test demonstrates the bug - checking intermediate state
it('should use primary URL immediately when symbol changes (BUG: uses fallback first)', () => {
// Simulate the render cycle manually
let useFallbackUrl = true; // State from previous symbol's primary failure
const newSymbol = 'ETH';
const iconUrls = {
primary: 'https://metamask/.../ETH.svg',
fallback: 'https://hyperliquid/.../ETH.svg'
};
// First render with new symbol (before useEffect runs)
const imageUri = iconUrls
? useFallbackUrl
? iconUrls.fallback // <-- BUG: This branch is taken
: iconUrls.primary
: null;
console.log('First render imageUri:', imageUri);
console.log('Expected primary URL:', iconUrls.primary);
console.log('Bug present:', imageUri === iconUrls.fallback);
});Command run:
node -e "
let useFallbackUrl = true;
const iconUrls = { primary: 'https://metamask/ETH.svg', fallback: 'https://hyperliquid/ETH.svg' };
const imageUri = iconUrls ? (useFallbackUrl ? iconUrls.fallback : iconUrls.primary) : null;
console.log('First render uses:', imageUri.includes('hyperliquid') ? 'FALLBACK (bug)' : 'PRIMARY (correct)');
console.log('Expected: PRIMARY');
"
Output:
First render uses: FALLBACK (bug)
Expected: PRIMARY
Why this proves the bug: The output shows that when useFallbackUrl is true from a previous symbol's failure, the first render with a new symbol incorrectly selects the fallback URL instead of the primary URL. The useEffect that resets useFallbackUrl to false only runs after this initial render, causing one render cycle with the wrong URL.
| <meta-data android:name="expo.modules.updates.CODE_SIGNING_CERTIFICATE" android:value="-----BEGIN CERTIFICATE-----
MIIC0zCCAbugAwIBAgIJGH+Ulb6OVEeJMA0GCSqGSIb3DQEBCwUAMBMxETAPBgNV
BAMTCE1ldGFNYXNrMB4XDTI1MTIxOTIzMTY1NFoXDTM1MTIxOTIzMTY1NFowEzER
MA8GA1UEAxMITWV0YU1hc2swggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB
AQDWvpseYyys+mFZrxGXMDD4VRGIs0u5mXgjwDf7fEOJWI7uldvVcVDPCO1v+/Ig
0nlz/NgDdVvYNgKFJJq4JMLDJNzxNIcaqSCKlO9IU5qWvnnfEeyWx6Pv0NQssHOD
Sc+WnvfCXub3akM9CE6Noy/KbIHpyUNwypux1eU5KGnZ704kNVsWmU7PeRn5Olnl
6t5Q93sIe2fbESDvHYh0TKC9eQvgkvrCCKlMgyqnZb4fdylXGbWRjivp+AKh/DHz
37bz9KtQO7YF8oZ1QvTiSVOb0hS1rzAE/YMatTbC214FQu5/w3vHCe7ejKcNCjzM
xFm6nSd41ho/SpW4r92dt0xvAgMBAAGjKjAoMA4GA1UdDwEB/wQEAwIHgDAWBgNV
HSUBAf8EDDAKBggrBgEFBQcDAzANBgkqhkiG9w0BAQsFAAOCAQEAn8OKneCOuUcI
wUGRVqWDcej4yYGugWWiIdEgy4as+vXUhMCk47uPwWuKNSILDuPeSxt2lo+AND8U
4N3I87+oYbLktOyph5FtpwUEMC8R/YE8Q5bNmi0LHzzGteenfUhSc+MVhpVVwZ1I
SbHGrj6/oet39FFFqJhWAU+RbMwSnYZKrZTfYELuEppP3WO03P9I8T3XR0d+CmMb
YdeLUvYilZI+3VxKL6tg/UWlZOX8MH6JbtNTm+2YMi1fp/hE8mrd+rR2Re5d87ub
srOY8HZvM0JX4RPddITpfSEwLwrPhdSFRwxyAVmBALYWVHeuiFn2pI3jfNeHF2Lt
SS2hIbf10Q==
-----END CERTIFICATE-----
" /> | ||
| <meta-data android:name="expo.modules.updates.CODE_SIGNING_METADATA" android:value="{"keyid":"rc","alg":"rsa-v1_5-sha256"}" /> | ||
| <meta-data android:name="expo.modules.updates.ENABLED" android:value="true" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may need to be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sigh sorry about this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsThe changes are entirely contained within the Perps (Perpetuals trading) feature module at
The PerpsTokenLogo component is used by 13 other components, all within the Perps module (PerpsCard, PerpsMarketHeader, PerpsOrderDetailsView, etc.). No changes affect core wallet functionality, controllers, Engine, or any other features outside of Perps. This is a UI enhancement that improves resilience by adding fallback icon sources. The changes are well-tested with comprehensive unit tests. Only the SmokePerps tag is needed to verify this functionality. |
|



Description
This PR implements a graceful degradation pattern for Perps asset icons, allowing the app to first attempt loading icons from MetaMask's contract-metadata repository and automatically fall back to HyperLiquid's CDN if the primary source fails.
We want to host curated Perps icons in our own contract-metadata repo for quality control. However, not all icons may be uploaded yet, and we need a reliable fallback. This approach allows gradual migration while maintaining full icon coverage
Solution:
METAMASK_PERPS_ICONS_BASE_URLconstant pointing to contract-metadata repogetAssetIconUrls()function that returns both primary and fallback URLsIn cases where both primary and fallback urls fail, we fallback to the default two letter icon.
Changelog
CHANGELOG entry: Perps icon fallback url mechanism
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2329
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Introduces dual-source Perps icon loading with automatic fallback and comprehensive tests.
METAMASK_PERPS_ICONS_BASE_URLand implementsgetAssetIconUrls()to return{ primary, fallback }for both regular and HIP-3 assets (MetaMask useship3:dex_SYMBOL.svg, HyperLiquid usesdex:SYMBOL.svg)PerpsTokenLogoto prefericonUrls.primary, switch toiconUrls.fallbackononError, and revert to primary when thesymbolchanges; preserves 2-letter text fallback if both failimageKeyincluding fallback state for proper re-rendering; retains loading and caching behaviorsPerpsTokenLogo.test.tsxandmarketUtils.test.tsto cover primary→fallback switching, HIP-3 formatting, k-prefix handling, uppercase normalization, and symbol-change resetWritten by Cursor Bugbot for commit 63075a8. This will update automatically on new commits. Configure here.