(Deposit/Withdraw) Fix Penumbra Issue #3966
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
WalkthroughThe pull request introduces the Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint (1.23.1)
packages/web/utils/bridge.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/packages/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)packages/web/utils/bridge.ts (2)
The Penumbra logo URL uses a different path pattern ("/networks/") compared to other bridges ("/bridges/"). Consider maintaining consistency with other bridge logos. Let's verify the asset existence and current bridge logo patterns:
Ensure that the Let's verify the Bridge type definition: ✅ Verification successfulBridge type correctly includes Penumbra The verification confirms that Penumbra is properly included in the Bridge type. The type is derived from 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check Bridge type definition
# Search for Bridge type definition
ast-grep --pattern 'type Bridge = $$$'
# Alternative: Search for Bridge enum definition
ast-grep --pattern 'enum Bridge {
$$$
}'
Length of output: 183 Script: #!/bin/bash
# Let's check the BridgeProviders type to see if Penumbra is included
ast-grep --pattern 'interface BridgeProviders {
$$$
}'
# Also search for any Penumbra-related bridge definitions
rg -A 5 "Penumbra" packages/bridge/src/
Length of output: 3637 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (10)
packages/utils/src/penumbra.ts (2)
1-6: Add TypeScript interface for chain informationConsider adding a type definition to ensure consistency with other chain information objects and improve type safety.
+interface ChainInfo { + prettyName: string; + chainId: string; + chainName: string; + color: string; +} + -export const PenumbraChainInfo = { +export const PenumbraChainInfo: ChainInfo = { prettyName: "Penumbra", chainId: "penumbra", chainName: "Penumbra", color: "#ff902f", };
5-5: Document color usage and add color validationThe color property should include a comment explaining its usage in the UI (e.g., for branding, buttons, or backgrounds). Also, consider adding runtime validation for the hex color format.
+// Color used for Penumbra chain branding in UI elements color: "#ff902f",packages/bridge/src/penumbra/index.ts (2)
1-16: Add class documentation.Consider adding JSDoc documentation to explain the purpose of this provider, its limitations, and its role in handling Penumbra bridge operations.
+/** + * Bridge provider for Penumbra blockchain. + * Currently supports asset discovery but other operations (quotes, transactions, external URLs) + * are not yet implemented. + */ export class PenumbraBridgeProvider implements BridgeProvider {
27-28: Remove or update misleading comment.The comment "just supports SOL via Penumbra" appears incorrect as the code doesn't enforce SOL-specific logic.
packages/bridge/src/bridge-providers.ts (2)
45-45: Consider graceful degradation for unsupported features.The provider is correctly instantiated with commonContext, matching other providers' initialization pattern. However, given that some functionalities throw errors (per AI summary), consider:
- Adding runtime capability checks
- Implementing feature detection
- Providing meaningful error messages to users when features are unavailable
Would you like me to propose a pattern for graceful feature degradation?
Penumbra bridge provider lacks proper test coverage and error handling validation
The codebase shows that while other bridge providers (Squid, IBC, Axelar) have comprehensive test coverage including error scenarios, the Penumbra bridge provider only has basic error throwing without tests. This could impact the reliability of stalled deposit fixes.
Required changes:
- Add unit tests for the Penumbra bridge provider following the patterns established in other providers (e.g.
squid-bridge-provider.spec.ts)- Include test cases for:
- Quote retrieval
- Transaction creation
- Error handling scenarios
- Status tracking
- External URL generation
🔗 Analysis chain
Line range hint
1-47: Ensure comprehensive testing of bridge provider integration.Given that this PR aims to fix stalled UM deposits, please ensure:
- Integration tests cover the full deposit/withdrawal flow
- Error scenarios are properly handled and communicated to users
- Stalled transaction detection and recovery mechanisms are in place
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for test coverage and error handling patterns # Look for test files related to Penumbra integration fd "penumbra.*test" --type f # Check for error handling patterns in the bridge implementation rg "catch|throw|Error" --type ts packages/bridge/src/penumbra/Length of output: 429
Script:
#!/bin/bash # Let's check if there are any test files in the bridge package and its structure fd "test" packages/bridge/ # Check the test patterns for other bridge providers to understand the testing standards fd ".*test" packages/bridge/src/ --type f --exec head -n 20 {}Length of output: 540
Script:
#!/bin/bash # Let's check the test implementations for other bridge providers rg -A 10 "describe|it\(" packages/bridge/src/{axelar,ibc,skip,squid}/__tests__/ # Check if there are any common test utilities or shared test patterns rg -A 5 "export|class|interface|type" packages/bridge/src/__tests__/Length of output: 115554
packages/web/server/api/routers/local-bridge-transfer.ts (1)
Line range hint
48-254: Consider architectural improvements for better maintainability.The current implementation could benefit from the following improvements:
- Extract chain-specific logic into separate handlers to reduce complexity and improve maintainability
- Implement a proper error handling strategy for unsupported chains instead of silently returning zero balances
- Consider using a strategy pattern to handle different chain types, making it easier to add new chains like Penumbra
Example structure:
interface ChainHandler { getBalance(asset: Asset): Promise<Balance>; } class EvmChainHandler implements ChainHandler { // Implementation for EVM chains } class CosmosChainHandler implements ChainHandler { // Implementation for Cosmos chains } class PenumbraChainHandler implements ChainHandler { // Implementation for Penumbra }Would you like me to help create a detailed proposal for this refactoring?
packages/web/components/bridge/use-bridges-supported-assets.ts (1)
17-24: Consider documenting supported asset types for PenumbraWhile the comment mentions BTC, SOL, and TRX assets for the listed providers, it would be helpful to explicitly document which asset types are supported specifically for Penumbra transfers.
Consider updating the comment to clarify:
- // include nomic, nitro, wormhole, and penumbra for suggesting BTC + SOL + TRX assets and chains + // include nomic, nitro, wormhole, and penumbra for suggesting BTC + SOL + TRX assets and chains + // Penumbra specifically supports: [list supported asset types]packages/bridge/src/interface.ts (2)
188-192: Align schema structure with existing chain schemasThe
penumbraChainSchemastructure differs from other chain schemas in two ways:
chainNameis required while it's optional in all other chain schemasnetworkNamefield is missing while it's present in some chain schemas (cosmos, evm)Consider this modification to maintain consistency:
const penumbraChainSchema = z.object({ chainId: z.string(), - chainName: z.string(), + chainName: z.string().optional(), + networkName: z.string().optional(), chainType: z.literal("penumbra"), });
188-192: Add JSDoc comments for the Penumbra schemaTo maintain consistency with other schemas in the file and improve code documentation, consider adding JSDoc comments explaining the Penumbra chain schema.
Add documentation above the schema:
+/** + * Schema for Penumbra chain configuration. + * + * @property chainId - The unique identifier for the Penumbra chain + * @property chainName - The human-readable name of the chain + * @property chainType - Literal "penumbra" to identify Penumbra chains + */ const penumbraChainSchema = z.object({ chainId: z.string(), chainName: z.string(), chainType: z.literal("penumbra"), });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
packages/web/public/networks/penumbra.svgis excluded by!**/*.svg,!**/*.svg
📒 Files selected for processing (8)
packages/bridge/src/bridge-providers.ts(3 hunks)packages/bridge/src/interface.ts(1 hunks)packages/bridge/src/penumbra/index.ts(1 hunks)packages/utils/src/index.ts(1 hunks)packages/utils/src/penumbra.ts(1 hunks)packages/web/components/bridge/use-bridges-supported-assets.ts(1 hunks)packages/web/server/api/routers/bridge-transfer.ts(2 hunks)packages/web/server/api/routers/local-bridge-transfer.ts(2 hunks)
🔇 Additional comments (13)
packages/utils/src/penumbra.ts (2)
3-3: Verify chainId format consistency
Let's verify that the chainId format follows the same convention as other chains in the system.
✅ Verification successful
Chain ID format is consistent with other non-numeric chain identifiers
Looking at the search results, we can see that the chainId format "penumbra" follows the same convention as other non-numeric chain identifiers in the system like "bitcoin", "solana", "celestia", etc. The format is consistent with other simple, lowercase chain identifiers used in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other chainId definitions to verify format consistency
rg -g '*.ts' -g '*.tsx' 'chainId:.*"[^"]*"' --no-filename | sort -u
Length of output: 13486
2-4: Consider consolidating name properties
The prettyName and chainName properties have the same value. Consider either:
- Documenting the different use cases for each property, or
- Consolidating them if they serve the same purpose
✅ Verification successful
Based on the search results, I can see that chainName and prettyName serve different purposes in the codebase:
-
chainNameis used extensively as an identifier in asset and counterparty definitions, appearing in asset type interfaces and test data. It's used for chain identification in IBC transfers and asset tracking. -
prettyNameappears much less frequently and is mainly used in wallet/chain info objects likePenumbraChainInfo,TronChainInfo, etc. for display purposes.
Keep both properties as they serve different purposes
The chainName property is used as a technical identifier throughout the codebase for asset and IBC routing, while prettyName appears to be used for UI display purposes. This separation of concerns is a good practice - one for system identification and one for user-facing display.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of prettyName and chainName to understand their purposes
rg -g '*.ts' -g '*.tsx' 'prettyName|chainName' -B2 -A2
Length of output: 144830
packages/utils/src/index.ts (2)
18-18: LGTM! Export follows established patterns
The new penumbra module export follows the same pattern as other chain-specific exports and maintains alphabetical ordering.
18-18: Verify penumbra module existence and usage
Let's confirm that the module exists and is being used by the bridge components.
✅ Verification successful
Penumbra module is properly integrated and actively used
The verification confirms that:
- The
penumbra.tsmodule exists inpackages/utils/src/and exports chain information - The module is actively used in the bridge implementation:
PenumbraBridgeProvideris implemented inpackages/bridge/src/penumbra/index.tsPenumbraChainInfois used in bridge transfer router atpackages/web/server/api/routers/bridge-transfer.ts- The bridge provider is properly registered in
packages/bridge/src/bridge-providers.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify penumbra module existence and its usage in bridge components
# Check if penumbra module exists
echo "Checking for penumbra module..."
fd --type f "penumbra.(ts|tsx)" packages/utils/src
# Check usage in bridge components
echo "Checking usage in bridge components..."
rg -l "from ['\"]@osmosis-labs/utils['\"].*penumbra" packages/
Length of output: 312
Script:
#!/bin/bash
# Let's check the content of penumbra.ts and its usage with more detailed patterns
# Check the content of penumbra.ts
echo "Content of penumbra.ts:"
cat packages/utils/src/penumbra.ts
# Search for any imports of penumbra, including relative paths
echo -e "\nChecking for penumbra imports:"
rg "from ['\"].*penumbra['\"]" -A 2
# Search for any usage of penumbra-related functions or types
echo -e "\nChecking for penumbra usage:"
rg "penumbra" --type ts --type tsx -i
Length of output: 917
Script:
#!/bin/bash
# Let's check the Penumbra bridge provider implementation and its usage
# Check the content of the Penumbra bridge provider
echo "Content of Penumbra bridge provider:"
cat packages/bridge/src/penumbra.ts
# Search for any usage of PenumbraBridgeProvider
echo -e "\nChecking PenumbraBridgeProvider usage:"
rg "PenumbraBridgeProvider" -A 2
# Search for any usage of PenumbraChainInfo
echo -e "\nChecking PenumbraChainInfo usage:"
rg "PenumbraChainInfo" -A 2
Length of output: 2148
packages/bridge/src/penumbra/index.ts (3)
16-16: LGTM!
The constructor correctly initializes the provider with the required context.
18-20: Verify if quote support is required for deposits.
Given that this PR aims to fix stalled UM deposits, please confirm:
- Is throwing "not supported" for quotes intentional?
- How does this affect the deposit flow?
Let's check if quotes are used in the deposit flow:
✅ Verification successful
Based on the search results, I can now provide a final response:
Throwing "not supported" for Penumbra quotes is intentional and won't affect deposits
The codebase shows that bridge providers can support different transfer types: "quote", "deposit-address", or "external-url". Looking at the implementation of other bridge providers (like Nitro and Wormhole) that also throw "not supported" for quotes, this appears to be a valid pattern for bridges that don't require quote support.
The search results show no direct dependency between deposits and quotes in the Penumbra bridge implementation, suggesting these are separate concerns. Bridge providers can function with deposit-address support alone, without requiring quote support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for quote usage in deposit-related code
rg -l 'getQuote.*deposit'
Length of output: 27
Script:
#!/bin/bash
# Let's try a broader search for getQuote usage
rg "getQuote" -A 3
# Also search for any deposit-related code that might use quotes
rg "deposit.*quote|quote.*deposit" -i
Length of output: 29908
58-60: Verify if transaction support is required for deposits.
Please confirm if throwing "not supported" for transactions is intentional and how this affects the deposit flow.
Let's check if transactions are used in the deposit flow:
✅ Verification successful
Based on the search results, I can now generate a final response:
Transaction support is not required for deposits in Penumbra bridge
The codebase shows that bridges can support different transfer variants: direct transactions or deposit addresses. Looking at the implementation:
- Some bridges like Skip, Squid, IBC, and Axelar implement
getTransactionData()for direct transaction flows - Others like Nomic use deposit address flows via
getDepositAddress() - Penumbra throwing "not supported" for transactions is intentional as it likely uses deposit address flow instead
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for transaction usage in deposit-related code
rg -l 'getTransactionData.*deposit'
Length of output: 37
Script:
#!/bin/bash
# Let's search for any usage of getTransactionData method
rg "getTransactionData" -A 3
# Also search for deposit-related code in bridge implementations
rg "deposit.*\{" -A 5
# Check for abstract class or interface definition
ast-grep --pattern 'interface $_ {
$$$
getTransactionData(): $_
$$$
}'
Length of output: 52725
packages/bridge/src/bridge-providers.ts (1)
6-6: LGTM! Import follows established pattern.
The import statement maintains alphabetical ordering and follows the same relative import pattern as other bridge providers.
packages/web/server/api/routers/local-bridge-transfer.ts (1)
66-66: LGTM: Schema addition follows established patterns.
The addition of Penumbra to the discriminated union is implemented consistently with other chain types.
packages/web/components/bridge/use-bridges-supported-assets.ts (2)
24-24: LGTM: Penumbra addition aligns with implementation pattern
The addition of "Penumbra" to the supportedAssetsBridges array follows the established pattern for external URL transfer options. This change is consistent with the PR's objective of implementing Penumbra bridge provider support.
17-23: Verify comment accuracy regarding external URL transfers
The comment clearly explains that these providers (nomic, nitro, wormhole, and penumbra) are included for suggesting assets and external URL transfers, not for native bridge support. However, let's verify that this behavior is consistently implemented.
✅ Verification successful
Based on the code analysis, I can now provide the final response:
Comment is accurate regarding external URL transfers
The code confirms that Penumbra is correctly implemented as an external URL provider:
-
The
PenumbraBridgeProviderimplementation explicitly throws errors for native operations:getQuote()throws "Penumbra quotes are currently not supported"getTransactionData()throws "Penumbra transactions are currently not supported"getExternalUrl()throws "Penumbra external urls are currently not supported"
-
The provider only implements
getSupportedAssets()to suggest SOL via Penumbra, which aligns with the comment's purpose of "suggesting BTC + SOL + TRX assets and chains as external URL transfer options" -
The provider is not included in the native bridge quote providers list in
use-bridge-quotes.ts, which matches the comment's statement "not supported by the bridge providers natively yet"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that Penumbra is properly handled as an external URL provider
# and doesn't interfere with native bridge functionality
# Check for any direct bridge implementation
ast-grep --pattern 'class PenumbraBridgeProvider {
$$$
transfer($$$) {
$$$
}
$$$
}'
# Check for URL handling
ast-grep --pattern 'getTransferUrl($$$) {
$$$
}'
Length of output: 150
Script:
#!/bin/bash
# Let's try a broader search to understand the bridge provider implementation
# Search for Penumbra-related files and implementations
rg -l "Penumbra" --type ts
# Search for bridge provider patterns
rg "class.*BridgeProvider" -A 5
# Look for URL transfer handling patterns
rg "getTransferUrl|transferUrl" -A 3
# Check the bridge configuration or setup files
fd "bridge" --type f --extension ts --extension tsx --exec rg -l "Penumbra|provider"
Length of output: 6858
Script:
#!/bin/bash
# Let's examine the Penumbra bridge provider implementation to understand its role
# Check the Penumbra bridge provider implementation
cat packages/bridge/src/penumbra/index.ts
# Check how it's used in bridge providers
cat packages/bridge/src/bridge-providers.ts
# Check the bridge quotes usage
cat packages/web/components/bridge/use-bridge-quotes.ts
Length of output: 30233
packages/web/server/api/routers/bridge-transfer.ts (2)
29-29: LGTM: Import follows established pattern
The addition of PenumbraChainInfo import follows the same pattern as other chain info types.
423-428: Verify Penumbra logo asset exists
The implementation follows the same pattern as other chain types and looks correct. However, please ensure that the logo file exists at the specified path.
✅ Verification successful
Logo asset verification successful
The Penumbra logo file exists at the correct location: packages/web/public/networks/penumbra.svg. The implementation is consistent with the asset's location.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the Penumbra logo file exists in the public directory
fd -t f "penumbra.svg" public/networks/
Length of output: 143
Script:
#!/bin/bash
# First, let's find the correct location of network logos
fd -t d "networks"
# Then, let's search for the penumbra logo file in the entire repository
fd -t f "penumbra.svg"
Length of output: 113
| const penumbraCounterparty = assetListAsset.counterparty.find( | ||
| (c) => c.chainName === "penumbra" | ||
| ); | ||
|
|
||
| if (penumbraCounterparty) { | ||
| return [ | ||
| { | ||
| transferTypes: ["external-url"], | ||
| chainId: "penumbra", | ||
| chainName: "Penumbra", | ||
| chainType: "penumbra", | ||
| denom: penumbraCounterparty.symbol, | ||
| address: penumbraCounterparty.sourceDenom, | ||
| decimals: penumbraCounterparty.decimals, | ||
| }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation and improve robustness.
Several improvements could make this code more robust:
- Validate required counterparty fields before using them
- Use case-insensitive chain name comparison
- Extract "penumbra" string constant
+const CHAIN_NAME = "penumbra";
+
const penumbraCounterparty = assetListAsset.counterparty.find(
- (c) => c.chainName === "penumbra"
+ (c) => c.chainName.toLowerCase() === CHAIN_NAME
);
-if (penumbraCounterparty) {
+if (
+ penumbraCounterparty &&
+ penumbraCounterparty.symbol &&
+ penumbraCounterparty.sourceDenom &&
+ typeof penumbraCounterparty.decimals === 'number'
+) {
return [{
transferTypes: ["external-url"],
- chainId: "penumbra",
+ chainId: CHAIN_NAME,
chainName: "Penumbra",
- chainType: "penumbra",
+ chainType: CHAIN_NAME,
denom: penumbraCounterparty.symbol,
address: penumbraCounterparty.sourceDenom,
decimals: penumbraCounterparty.decimals,
}];
}📝 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.
| const penumbraCounterparty = assetListAsset.counterparty.find( | |
| (c) => c.chainName === "penumbra" | |
| ); | |
| if (penumbraCounterparty) { | |
| return [ | |
| { | |
| transferTypes: ["external-url"], | |
| chainId: "penumbra", | |
| chainName: "Penumbra", | |
| chainType: "penumbra", | |
| denom: penumbraCounterparty.symbol, | |
| address: penumbraCounterparty.sourceDenom, | |
| decimals: penumbraCounterparty.decimals, | |
| }, | |
| ]; | |
| } | |
| const CHAIN_NAME = "penumbra"; | |
| const penumbraCounterparty = assetListAsset.counterparty.find( | |
| (c) => c.chainName.toLowerCase() === CHAIN_NAME | |
| ); | |
| if ( | |
| penumbraCounterparty && | |
| penumbraCounterparty.symbol && | |
| penumbraCounterparty.sourceDenom && | |
| typeof penumbraCounterparty.decimals === 'number' | |
| ) { | |
| return [ | |
| { | |
| transferTypes: ["external-url"], | |
| chainId: CHAIN_NAME, | |
| chainName: "Penumbra", | |
| chainType: CHAIN_NAME, | |
| denom: penumbraCounterparty.symbol, | |
| address: penumbraCounterparty.sourceDenom, | |
| decimals: penumbraCounterparty.decimals, | |
| }, | |
| ]; | |
| } |
| @@ -21,6 +22,7 @@ export class BridgeProviders { | |||
| [WormholeBridgeProvider.ID]: WormholeBridgeProvider; | |||
| [NitroBridgeProvider.ID]: NitroBridgeProvider; | |||
| [PicassoBridgeProvider.ID]: PicassoBridgeProvider; | |||
| [PenumbraBridgeProvider.ID]: PenumbraBridgeProvider; | |||
There was a problem hiding this comment.
💡 Codebase verification
Most methods in PenumbraBridgeProvider are not implemented and throw errors
The implementation shows that PenumbraBridgeProvider implements BridgeProvider interface but most of its methods are stubs that throw "not supported" errors:
getQuote(): throws "Penumbra quotes are currently not supported"getTransactionData(): throws "Penumbra transactions are currently not supported"getExternalUrl(): throws "Penumbra external urls are currently not supported"
Only getSupportedAssets() is implemented, which returns SOL assets that have Penumbra counterparts in the asset list.
🔗 Analysis chain
Verify PenumbraBridgeProvider interface implementation.
The type definition suggests PenumbraBridgeProvider implements the bridge provider interface. Given the AI summary mentions that some functionalities throw errors, we should verify the implementation details.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify PenumbraBridgeProvider implementation and error handling
# Check the implementation details of PenumbraBridgeProvider
ast-grep --pattern 'class PenumbraBridgeProvider implements $_ {
$$$
}'
# Look for throw statements in the implementation
rg "throw" --type ts packages/bridge/src/penumbra/
Length of output: 4241
| export const bridgeChainSchema = z.discriminatedUnion("chainType", [ | ||
| cosmosChainSchema, | ||
| evmChainSchema, | ||
| solanaChainSchema, | ||
| bitcoinChainSchema, | ||
| tronChainSchema, | ||
| penumbraChainSchema, |
There was a problem hiding this comment.
💡 Codebase verification
Chain type "penumbra" needs additional handling in switch statements
Based on the analysis of the codebase, while the "penumbra" chain type is properly defined in the schema and has basic infrastructure in place, there are several locations where switch statements and conditional checks need to be updated to handle the new chain type:
packages/web/components/bridge/amount-screen.tsx: Switch statement for wallet connection handling doesn't include "penumbra" casepackages/web/components/bridge/bridge-wallet-select-modal.tsx: Address validation switch statement needs "penumbra" casepackages/web/components/bridge/use-bridge-quotes.ts: Wallet connection checks need to account for "penumbra"packages/web/components/transactions/transaction-details/transaction-transfer-details.tsx: Explorer URL generation switch statement needs "penumbra" case
🔗 Analysis chain
Verify handling of "penumbra" chain type across the codebase
The addition of penumbraChainSchema to the discriminated union means that all code handling different chain types needs to account for the new "penumbra" type.
Let's verify the exhaustive handling of chain types:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find switch statements and type checks that need to handle the new "penumbra" chain type
# Search for switch statements on chainType
echo "Checking switch statements on chainType..."
rg "switch.*chainType" -A 10
# Search for conditional checks on chainType
echo "Checking conditional checks on chainType..."
rg "chainType\s*(===|==|!=|!==)\s*['\"]" -A 2
# Search for type guards using chainType
echo "Checking type guard usage..."
rg "chainType\s+in\s+" -A 2
Length of output: 24425
Script:
#!/bin/bash
# Let's check for exhaustive pattern matching in switch statements and type guards
# by looking for the "penumbra" chain type handling
# Search for switch statements that might need penumbra handling
echo "Checking switch/case statements for penumbra handling..."
ast-grep --pattern 'switch ($expr) {
$$$
case "penumbra":
$$$
}'
# Search for type definitions and interfaces that might need penumbra
echo "Checking type definitions for penumbra chain type..."
ast-grep --pattern 'type $_ = $$$"penumbra"$$$'
# Search for any other penumbra-related code patterns
echo "Checking other penumbra-related patterns..."
rg "penumbra" -B 2 -A 2
Length of output: 5193
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/web/components/bridge/amount-screen.tsx(1 hunks)packages/web/server/api/routers/bridge-transfer.ts(3 hunks)
🔇 Additional comments (5)
packages/web/server/api/routers/bridge-transfer.ts (3)
29-29: LGTM! Import follows established pattern.
The addition of PenumbraChainInfo follows the same pattern as other chain info imports.
45-45: LGTM! Type definition is consistent.
The addition of the Penumbra chain type to BridgeChainWithDisplayInfo follows the established pattern and maintains type safety.
424-429: LGTM! Chain handling implementation is consistent.
The Penumbra chain handling follows the same pattern as other chains.
Let's verify that the Penumbra logo file exists:
✅ Verification successful
Logo file exists and is correctly referenced
The Penumbra logo file exists at packages/web/public/networks/penumbra.svg, which matches the path referenced in the code (/networks/penumbra.svg). The implementation is correct and the asset is available.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the Penumbra logo file exists in the public/networks directory
fd "penumbra.svg" public/networks
Length of output: 136
Script:
#!/bin/bash
# Let's first find the correct location of the public/networks directory
fd -t d "networks" -d 3
Length of output: 23
Script:
#!/bin/bash
# Let's try to find any SVG files related to networks or chain logos
fd -e svg
Length of output: 6670
packages/web/components/bridge/amount-screen.tsx (2)
427-434: LGTM: Penumbra chain type support added correctly.
The implementation for the Penumbra chain type follows the established pattern and maintains type safety. The code correctly returns the required balance source structure with the chain type and assets.
427-434: Verify Penumbra chain type integration.
Please ensure that:
- The Penumbra chain type is properly defined in the bridge interface types.
- The balance fetching API endpoint supports the new chain type.
✅ Verification successful
Let me gather more information about the chain type definitions.
Let me check the implementation of the bridge supported assets hook to understand the type definitions better.
Let me check for chain type definitions in bridge-related files.
Let me check the bridge interface types and API endpoint implementation.
Based on the gathered information, I can now provide the final response:
Penumbra chain type integration is properly implemented.
The verification confirms that:
- Penumbra is properly defined as a chain type in the bridge interface types (
BridgeChainWithDisplayInfoinbridge-transfer.ts). - The balance fetching API endpoint supports Penumbra chain type through the
createAssetObject("penumbra", z.object({}))schema inlocal-bridge-transfer.ts. - The implementation correctly handles Penumbra assets in the amount screen component with proper type extraction.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Penumbra chain type integration
# Check if Penumbra chain type is defined in interface
rg -A 5 'type.*ChainType.*=.*{'
# Check if API endpoint handles Penumbra chain type
rg -A 10 'getSupportedAssetsBalances'
Length of output: 3660
Script:
#!/bin/bash
# Search for chain type definitions with proper escaping
rg -A 5 'type\s+ChainType'
# Search for Penumbra type definitions
rg -A 10 'penumbra.*type'
# Look for supported asset type definitions
ast-grep --pattern 'type SupportedAsset = $_'
Length of output: 425
Script:
#!/bin/bash
# Check the implementation of useBridgesSupportedAssets
cat packages/web/components/bridge/use-bridges-supported-assets.ts
# Look for chain type definitions in type files
fd -e ts -e tsx types.ts -x cat {}
fd -e ts -e tsx types.tsx -x cat {}
Length of output: 102080
Script:
#!/bin/bash
# Search for chain type definitions in bridge files
rg -A 5 'type.*Chain.*=' packages/web/components/bridge/
rg -A 5 'chainType.*=' packages/web/components/bridge/
# Look for Penumbra chain type usage in bridge files
rg -A 5 '"penumbra"' packages/web/components/bridge/
Length of output: 25900
Script:
#!/bin/bash
# Check bridge interface types
rg -A 10 'interface.*Bridge' packages/web/
rg -A 10 'type.*Bridge' packages/web/
# Check API endpoint implementation for Penumbra support
rg -A 10 'createAssetObject.*penumbra' packages/web/
Length of output: 16112
| } else if (chainType === "penumbra") { | ||
| return { | ||
| ...PenumbraChainInfo, | ||
| chainType, | ||
| logoUri: "/networks/penumbra.svg", | ||
| }; |
There was a problem hiding this comment.
💡 Codebase verification
Bridge provider implementation is incomplete and requires attention
The PenumbraBridgeProvider is properly registered but currently has non-functional implementations:
getQuote()throws "Penumbra quotes are currently not supported"getTransactionData()throws "Penumbra transactions are currently not supported"getExternalUrl()throws "Penumbra external urls are currently not supported"
Only getSupportedAssets() is implemented to handle SOL via Penumbra, while core bridging functionality is missing.
🔗 Analysis chain
Verify integration with PenumbraBridgeProvider.
While the router changes look good, let's verify the integration with the bridge provider.
Let's check for the bridge provider implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify PenumbraBridgeProvider implementation
# Look for the bridge provider class definition
ast-grep --pattern 'class PenumbraBridgeProvider implements BridgeProvider {
$$$
}'
# Look for the registration of the provider
rg -A 2 "PenumbraBridgeProvider" packages/bridge/src/bridge-providers.ts
Length of output: 4248
JoseRFelix
deleted the
jose/fe-1223-ui-issue-with-um-deposit-on-main-and-stage-environments
branch
What is the purpose of the change:
This PR resolves the issue of stalled UM deposits by introducing a bridge provider for Penumbra.
Linear Task
https://linear.app/osmosis/issue/FE-1223/ui-issue-with-um-deposit-on-main-and-stage-environments
Testing and Verifying