[controller] add erc20 resources to controller #2485
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces new JSON files that map various resource types to their identifiers and addresses, specifically for different environments (local, mainnet, and sepolia). Additionally, it modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
You are out of MentatBot reviews. Your usage will refresh December 16 at 08:00 AM. |
|
Failed to generate code suggestions for PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
client/src/hooks/context/starknet-provider.tsx (1)
38-38: Verify token configuration consistency across environmentsThe token configuration is duplicated between mainnet and non-mainnet environments. Consider extracting this into a shared configuration.
const getControllerConfig = (isMainnet: boolean) => ({ rpc: env.VITE_PUBLIC_NODE_URL, namespace, slot, preset, policies: isMainnet ? [] : [...signingPolicy, ...policies, vrfPolicy], theme, tokens: { erc20: [LORDS, ...otherResources], }, colorMode, });Also applies to: 50-50
landing/src/components/ui/utils/utils.ts (1)
445-445: Enhance error handling and performance.Consider the following improvements:
- Add response validation to ensure the JSON structure matches
ResourceAddresses- Implement a retry mechanism for network failures
- Consider caching the results to avoid repeated fetches
- Add specific error types instead of returning an empty object
Here's a suggested implementation:
+const CACHE_DURATION = 5 * 60 * 1000; // 5 minutes +let cachedAddresses: ResourceAddresses | null = null; +let lastFetchTime = 0; export const getSeasonAddresses = async (): Promise<ResourceAddresses> => { + // Return cached data if valid + if (cachedAddresses && Date.now() - lastFetchTime < CACHE_DURATION) { + return cachedAddresses; + } + try { const path = getSeasonAddressesPath(); - const data = await getJSONFile(path); - return data; + const data = await retry(() => getJSONFile(path), 3); + + // Validate response structure + if (!isValidResourceAddresses(data)) { + throw new Error('Invalid resource addresses format'); + } + + // Update cache + cachedAddresses = data; + lastFetchTime = Date.now(); + return data; } catch (error) { console.error("Error loading season addresses:", error); - return {}; + throw new Error(`Failed to load resource addresses: ${error.message}`); } }; +function isValidResourceAddresses(data: any): data is ResourceAddresses { + return ( + typeof data === 'object' && + Object.entries(data).every( + ([key, value]) => + typeof key === 'string' && + Array.isArray(value) && + value.length === 2 && + typeof value[0] === 'number' && + typeof value[1] === 'string' + ) + ); +} +async function retry<T>(fn: () => Promise<T>, attempts: number): Promise<T> { + for (let i = 0; i < attempts; i++) { + try { + return await fn(); + } catch (error) { + if (i === attempts - 1) throw error; + await new Promise(resolve => setTimeout(resolve, 1000 * Math.pow(2, i))); + } + } + throw new Error('All retry attempts failed'); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
client/public/resource_addresses/local/resource_addresses.json(1 hunks)client/public/resource_addresses/mainnet/resource_addresses.json(1 hunks)client/public/resource_addresses/sepolia/resource_addresses.json(1 hunks)client/src/hooks/context/starknet-provider.tsx(3 hunks)client/src/ui/utils/utils.tsx(2 hunks)landing/src/components/providers/starknet-provider.tsx(2 hunks)landing/src/components/ui/utils/utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- client/public/resource_addresses/local/resource_addresses.json
- client/public/resource_addresses/mainnet/resource_addresses.json
🔇 Additional comments (2)
client/public/resource_addresses/sepolia/resource_addresses.json (1)
1-33: Verify contract addresses across environments
Please ensure that these contract addresses are correctly synchronized across all environments (local, sepolia, mainnet).
landing/src/components/ui/utils/utils.ts (1)
441-443: LGTM: Interface definition is clear and well-typed.
The ResourceAddresses interface provides good type safety for the resource mapping.
Comment on lines
+13
to
+15
| const otherResources = Object.entries(resourceAddresses) | ||
| .filter(([key]) => key !== "LORDS") | ||
| .map(([_, [__, address]]) => address); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety and error handling for resource mapping
The current implementation assumes the presence of "LORDS" key and correct array structure. Consider adding type checking and error handling.
-const otherResources = Object.entries(resourceAddresses)
- .filter(([key]) => key !== "LORDS")
- .map(([_, [__, address]]) => address);
+const otherResources = Object.entries(resourceAddresses)
+ .filter(([key]) => key !== "LORDS")
+ .map(([_, value]) => {
+ if (!Array.isArray(value) || value.length < 2) {
+ console.warn(`Invalid resource format for ${key}`);
+ return null;
+ }
+ return value[1];
+ })
+ .filter((address): address is string => address !== null);📝 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.
Suggested change
| const otherResources = Object.entries(resourceAddresses) | |
| .filter(([key]) => key !== "LORDS") | |
| .map(([_, [__, address]]) => address); | |
| const otherResources = Object.entries(resourceAddresses) | |
| .filter(([key]) => key !== "LORDS") | |
| .map(([_, value]) => { | |
| if (!Array.isArray(value) || value.length < 2) { | |
| console.warn(`Invalid resource format for ${key}`); | |
| return null; | |
| } | |
| return value[1]; | |
| }) | |
| .filter((address): address is string => address !== null); |
| //import { cartridgeController } from "./cartridge-controller"; | ||
|
|
||
| const resourceAddresses = await getSeasonAddresses(); |
There was a problem hiding this comment.
Avoid top-level await - move initialization into component
Using top-level await can cause issues in environments that don't support it. Consider moving the resource address initialization into the component.
-const resourceAddresses = await getSeasonAddresses();
+export function StarknetProvider({ children, onlyCartridge }: { children: React.ReactNode; onlyCartridge?: boolean }) {
+ const [resourceAddresses, setResourceAddresses] = useState<ResourceAddresses | null>(null);
+
+ useEffect(() => {
+ async function initializeAddresses() {
+ try {
+ const addresses = await getSeasonAddresses();
+ setResourceAddresses(addresses);
+ } catch (error) {
+ console.error('Failed to load resource addresses:', error);
+ }
+ }
+ initializeAddresses();
+ }, []);
+
+ if (!resourceAddresses) {
+ return <div>Loading resources...</div>;
+ }Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+11
to
+16
| const resourceAddresses = await getSeasonAddresses(); | ||
|
|
||
| const LORDS = resourceAddresses["LORDS"][1]; | ||
| const otherResources = Object.entries(resourceAddresses) | ||
| .filter(([key]) => key !== "LORDS") | ||
| .map(([_, [__, address]]) => address); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate provider implementations to avoid duplication
This provider implementation duplicates logic from the landing provider. Consider extracting common functionality into a shared utility.
// src/shared/providers/resource-loader.ts
export function useResourceAddresses() {
const [addresses, setAddresses] = useState<ResourceAddresses | null>(null);
const [error, setError] = useState<Error | null>(null);
useEffect(() => {
async function load() {
try {
const data = await getSeasonAddresses();
setAddresses(data);
} catch (err) {
setError(err as Error);
}
}
load();
}, []);
return { addresses, error };
}
Comment on lines
+1
to
+33
| { | ||
| "STONE": [1, "0x6158648e6c53488e316a9f681623b9e0359240329a00e5d5f5e82da21453b4c"], | ||
| "COAL": [2, "0xeff1980a4f28877118e06aff65d8c972ccc69c7d699975194ae8e83978b116"], | ||
| "WOOD": [3, "0x1605f515516e0ea60df3ae92a8204037866c3b27199865133f9f9f35208d298"], | ||
| "COPPER": [4, "0x76c0ee15cd5e588f0fa6ffb7c9ce301f96d33cd9f6bf0760ed2e55407130e4"], | ||
| "IRONWOOD": [5, "0x9f9cb4a6cb392f2c8526ae6c1f48ddb1eb67598030afdcabe06a48c752b3c6"], | ||
| "OBSIDIAN": [6, "0x53d73cddd205f590bf93290fcce62cb1079bd440e7253bef50ce2d890583c09"], | ||
| "GOLD": [7, "0x45ee17873d009cb7f8ad88bdd01218b07ceb055a8b22d6a0795028b2aafd8d8"], | ||
| "SILVER": [8, "0x56defcf99c674db261f508b584537c760b9001df9a8cb13275afa564f600dbe"], | ||
| "MITHRAL": [9, "0x7613ec3c16e4cefc9c33bea2b7e16614a0c4b5237e26cf75f335e0f028da988"], | ||
| "ALCHEMICALSILVER": [10, "0x705f42f2a89bf32c504c19c618ca6738747871016bba0b3fc32f3cd73782160"], | ||
| "COLDIRON": [11, "0x22bc712f59ceefc7c7521c607eb5f48759ffd906ee6342ee80dad2973f58d8"], | ||
| "DEEPCRYSTAL": [12, "0x242f3bcca7f72b5ca034dfe9903bb73024353cec795dca1fd8e2f500d17a733"], | ||
| "RUBY": [13, "0x9c473b26f7df7897b517f3c9e1ce8f1f7236b67666983f5ddaf039ca4c46a9"], | ||
| "DIAMONDS": [14, "0x78d34a4af3742f6f3377e97dd69f95680bb0f08521ad935428e3b2c3c5ed01e"], | ||
| "HARTWOOD": [15, "0x3d3f23f4829b8fe343b00c44c63828f5a81e468fb30eb9504c7c44455cead7b"], | ||
| "IGNIUM": [16, "0x204f9596fa6f05c2de88f3a7fe42c0d11fb04cc96128f18fbe112e44f360696"], | ||
| "TWILIGHTQUARTZ": [17, "0x1a0f1b62ef495956532db0306d13c92c02604995178d3fc98b84c8b8fb8a18e"], | ||
| "TRUEICE": [18, "0x3e089cd9be6c251dae20c64649a770507750f4b4c9e65ebf30bdb63b541605d"], | ||
| "ADAMANTINE": [19, "0x55399ca187391f4cc03abb00a04e3b6974bc0d7757ad959ac2804e6c1c468d5"], | ||
| "SAPPHIRE": [20, "0x62feb496f301b27bbda2389463d1ca06ef9f22d4208aecef2e85da1e4c90c4f"], | ||
| "ETHEREALSILICA": [21, "0x25eb12bb5298a2b14ceb9109086d330810d55c5e9a8c17e60fc956453b30d14"], | ||
| "DRAGONHIDE": [22, "0x463ebcc1b9ac911930a471fb9b608e9e7ca07ed81aab66f8889dd45b41c28b2"], | ||
| "DEMONHIDE": [28, "0x9613c7e4c04929c616a81f150006bc1a96bb77426851d300ef3dfaf2d0c318"], | ||
| "ANCIENTFRAGMENT": [29, "0x403c764fd76f926eec43218f46f79cfb5fe86dfeb1ae76010027697f65733f1"], | ||
| "DONKEY": [249, "0x6b6c4437543e46a8a80835fa50643fbd32275d3836396d8cd9c1d7c987f4ea9"], | ||
| "KNIGHT": [250, "0x298cba6b152d04fd54195dbae0e74512ff740d14946d2d65f26d0c039feffe2"], | ||
| "CROSSBOWMAN": [251, "0x116ce3b8c4a70f61e0131cc8681bc50a5eb35276e7985c07e3a508c2eb25f8f"], | ||
| "PALADIN": [252, "0x63f2da6f023bb0f281daa4011beb62ef501207569654e01d9315673471b8839"], | ||
| "WHEAT": [254, "0x2cbcad77a7b2bdf87c128f035beef05e3a007c15b2e332b1f1fb04d19ffd1b4"], | ||
| "FISH": [255, "0x3da1d29652bf12f64366ca4e9b36ec046cc92f6dc3b4675d956db1c3d742f0f"], | ||
| "LORDS": [253, "0x342ad5cc14002c005a5cedcfce2bd3af98d5e7fb79e9bf949b3a91cf145d72e"] | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add schema validation for resource addresses
The JSON configuration contains critical contract addresses. Consider adding schema validation to ensure data integrity.
Example schema (create as resource-addresses.schema.json):
{
"$schema": "http://json-schema.org/draft-07/schema#",
"type": "object",
"additionalProperties": false,
"patternProperties": {
"^[A-Z]+$": {
"type": "array",
"items": [
{ "type": "integer", "minimum": 1 },
{ "type": "string", "pattern": "^0x[a-f0-9]+$" }
],
"minItems": 2,
"maxItems": 2
}
},
"required": ["LORDS"]
}Also, add validation to the getSeasonAddresses function:
import Ajv from 'ajv';
import schema from './resource-addresses.schema.json';
export async function getSeasonAddresses() {
const ajv = new Ajv();
const validate = ajv.compile(schema);
const data = await fetchAddresses();
if (!validate(data)) {
throw new Error('Invalid resource addresses format');
}
return data;
}
Comment on lines
+505
to
+526
| export const getSeasonAddressesPath = () => { | ||
| return `/resource_addresses/${env.VITE_PUBLIC_CHAIN}/resource_addresses.json`; | ||
| }; | ||
| export const getJSONFile = async (filePath: string) => { | ||
| const response = await fetch(filePath); | ||
| const data = await response.json(); | ||
| return data; | ||
| }; | ||
| interface ResourceAddresses { | ||
| [key: string]: [number, string]; | ||
| } | ||
|
|
||
| export const getSeasonAddresses = async (): Promise<ResourceAddresses> => { | ||
| try { | ||
| const path = getSeasonAddressesPath(); | ||
| const data = await getJSONFile(path); | ||
| return data; | ||
| } catch (error) { | ||
| console.error("Error loading season addresses:", error); | ||
| return {}; | ||
| } | ||
| } |
There was a problem hiding this comment.
Consolidate duplicated code into a shared utility.
This code is duplicated from landing/src/components/ui/utils/utils.ts. Consider moving these functions to a shared utility module to avoid maintenance issues and ensure consistent behavior.
- Create a new shared module (e.g.,
shared/utils/resource-addresses.ts) - Move the following to the shared module:
ResourceAddressesinterfacegetSeasonAddressesPathgetJSONFilegetSeasonAddresses
- Import from the shared module in both locations
Would you like me to help create this shared module?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2468
we should wait for images to be uploaded in controller
Summary by CodeRabbit
New Features
Bug Fixes
Documentation