Conversation
|
There was a problem hiding this comment.
Pull request overview
Adjusts the “unwire” config loading flow to tolerate missing unwire config files, and replaces the concrete mainnet unwire config with a template.
Changes:
loadUnwireConfignow returnsundefinedwhen the unwire YAML file doesn’t exist.- Mainnet graph builders short-circuit to an empty graph when no unwire config is present.
- Removes
unwire.asset.ymland adds atemplate-unwire.asset.ymlplaceholder.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/stg-evm-v2/devtools/config/utils/unwire.config.utils.ts | Makes unwire config loading optional when the YAML file is absent |
| packages/stg-evm-v2/devtools/config/mainnet/01/messaging-asset-removal.config.ts | Skips building the messaging-asset-removal graph when no unwire config exists |
| packages/stg-evm-v2/devtools/config/mainnet/01/chainsConfig/unwire/unwire.asset.yml | Removes the concrete mainnet unwire config file |
| packages/stg-evm-v2/devtools/config/mainnet/01/chainsConfig/unwire/template-unwire.asset.yml | Adds a template YAML to guide creation of an unwire config |
| packages/stg-evm-v2/devtools/config/mainnet/01/asset.unwire.config.ts | Skips building the asset unwire graph when no unwire config exists |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function loadUnwireConfig(baseDir: string): ResolvedUnwireConfig | undefined { | ||
| const configPath = resolveUnwireConfigPath(baseDir) | ||
| if (!fs.existsSync(configPath)) { | ||
| return undefined | ||
| } | ||
|
|
There was a problem hiding this comment.
loadUnwireConfig now returns undefined on a missing file, which is a silent/optional-load behavior that doesn’t match the API name. Consider renaming to something like tryLoadUnwireConfig, or alternatively keep loadUnwireConfig as “must exist” (throw with a clear message) and introduce a separate optional loader to avoid surprising callers.
| export function loadUnwireConfig(baseDir: string): ResolvedUnwireConfig | undefined { | |
| const configPath = resolveUnwireConfigPath(baseDir) | |
| if (!fs.existsSync(configPath)) { | |
| return undefined | |
| } | |
| export function tryLoadUnwireConfig(baseDir: string): ResolvedUnwireConfig | undefined { | |
| const configPath = resolveUnwireConfigPath(baseDir) | |
| if (!fs.existsSync(configPath)) { | |
| return undefined | |
| } | |
| return loadUnwireConfig(baseDir) | |
| } | |
| export function loadUnwireConfig(baseDir: string): ResolvedUnwireConfig { | |
| const configPath = resolveUnwireConfigPath(baseDir) | |
| if (!fs.existsSync(configPath)) { | |
| throw new Error(`Unwire config file not found at ${configPath}`) | |
| } |
| const { tokenName, disconnectChains, remainingChains } = loadUnwireConfig(__dirname) | ||
| const unwireConfig = loadUnwireConfig(__dirname) | ||
| if (!unwireConfig) { | ||
| return { contracts: [], connections: [] } |
There was a problem hiding this comment.
Returning an empty graph when the unwire config is missing can make the task appear to succeed while doing nothing, which is easy to miss. Prefer failing fast with a clear error (e.g., pointing at chainsConfig/unwire/template-unwire.asset.yml as the expected template) or at least emitting an explicit warning so operators know the run was a no-op.
| return { contracts: [], connections: [] } | |
| throw new Error( | |
| `Missing unwire config for directory "${__dirname}". ` + | |
| 'Please create an unwire config based on "chainsConfig/unwire/template-unwire.asset.yml".' | |
| ) |
|
|
||
| const unwireConfig = loadUnwireConfig(__dirname) | ||
| if (!unwireConfig) { | ||
| return { contracts: [], connections: [] } |
There was a problem hiding this comment.
Same as asset.unwire.config.ts: this silently produces a no-op graph when the config is missing. Consider throwing a descriptive error (or at minimum logging a warning) so CI/operators don’t mistakenly interpret an empty plan as a successful removal run.
| return { contracts: [], connections: [] } | |
| throw new Error( | |
| `Missing unwire config for messaging asset removal in directory: ${__dirname}. ` + | |
| 'This would produce an empty (no-op) graph, which is disallowed. ' + | |
| 'Ensure an unwire config is present or fix the config path.' | |
| ) |
| @@ -0,0 +1,10 @@ | |||
| # Asset mesh unwire config (mainnet) | |||
| # This file is used by asset.unwire.config.ts and messaging-asset-removal.config.ts | |||
| asset: eurc/usdc/usdt/eth | |||
There was a problem hiding this comment.
The template’s asset value looks like a literal slash-separated token (eurc/usdc/usdt/eth), which will be parsed as a single string and likely fail downstream if someone forgets to replace it. Consider changing it to a clearer placeholder (e.g., asset: <token> plus a comment listing allowed values) to reduce accidental misconfiguration.
| asset: eurc/usdc/usdt/eth | |
| # Replace <token> with a single asset symbol. Allowed values: eurc, usdc, usdt, eth. | |
| asset: <token> |
ravinagill15
left a comment
ravinagill15
left a comment
There was a problem hiding this comment.
Let's add changeset
| const { tokenName, disconnectChains, remainingChains } = loadUnwireConfig(__dirname) | ||
| const unwireConfig = loadUnwireConfig(__dirname) | ||
| if (!unwireConfig) { | ||
| return { contracts: [], connections: [] } |
|
|
||
| const unwireConfig = loadUnwireConfig(__dirname) | ||
| if (!unwireConfig) { | ||
| return { contracts: [], connections: [] } |
2 participants
No description provided.