add monad testnet support #69
Conversation
WalkthroughThis pull request introduces updates across multiple packages, primarily focusing on adding support for the Monad Testnet. The changes include version bumps for several packages ( Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Core as Core Package
participant Orderbook as Orderbook Package
participant Network as Monad Testnet
Dev->>Core: Define Monad Testnet Chain
Core->>Orderbook: Update Chain Mappings
Orderbook->>Network: Configure Testnet Support
Network-->>Orderbook: Confirm Configuration
Poem
Finishing Touches
🪧 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 comments (1)
packages/core/src/lib/garden/garden.spec.ts (1)
Line range hint
46-51: Expand wallet configuration for Monad testnet.The
walletsobject needs to be updated to include Monad testnet support.wallets = { [Chains.arbitrum_sepolia]: arbitrumWalletClient, [Chains.ethereum_sepolia]: ethereumWalletClient, + [Chains.monad_testnet]: createWalletClient({ + account, + chain: monadTestnet, + transport: http(), + }), // [Chains.bitcoin_regtest]: btcWallet, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/core/package.json(1 hunks)packages/core/src/lib/garden/garden.spec.ts(2 hunks)packages/core/src/lib/quote/quote.spec.ts(4 hunks)packages/core/src/lib/switchOrAddNetwork.ts(2 hunks)packages/orderbook/package.json(1 hunks)packages/orderbook/src/lib/asset.ts(4 hunks)packages/react-hooks/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/core/package.json
- packages/react-hooks/package.json
- packages/orderbook/package.json
🔇 Additional comments (7)
packages/orderbook/src/lib/asset.ts (2)
115-115: Verify the timelock duration for monad_testnetThe timelock value of 172800 (48 hours) for monad_testnet is significantly higher than other testnet chains which typically use 28800 (8 hours) or 7200 (2 hours). Please verify if this extended duration is intentional.
52-52: LGTM! Chain integration follows consistent patternsThe monad_testnet integration follows the established patterns for chain definitions, testnet identification, and EVM chain classification.
Also applies to: 72-73, 96-97
packages/core/src/lib/switchOrAddNetwork.ts (2)
66-68: Verify block explorer URLsThe block explorer URLs use
monad-devnet.devnet101.comwhich seems inconsistent with a testnet environment. Please verify if these are the correct URLs for the Monad testnet.
85-85: LGTM! Chain mapping is correctThe monad_testnet mapping to monadTestnet follows the established pattern in evmToViemChainMap.
packages/core/src/lib/quote/quote.spec.ts (2)
Line range hint
25-92: Address skipped tests before merging.Multiple test cases have been skipped (
exact_out,should get attested quote,test,should get strategies). These tests need to be either:
- Updated to support the new testnet environment
- Removed if no longer relevant
- Documented with a TODO explaining why they're skipped
This is particularly important as we're adding support for a new network.
13-23: Review the increased timeout and amount values.The test timeout has been increased to 30 seconds and the amount to 10000000. Please verify:
- If these increases are necessary for the Monad testnet
- If they indicate potential performance issues that need addressing
✅ Verification successful
The timeout and amount values appear justified for the Arbitrum Sepolia testnet
The 30-second timeout is reasonable considering:
- The test makes external API calls to price.garden.finance
- It involves cross-chain operations which typically need longer timeouts
- Another test in the same file also uses a 30s timeout
The increased amount (10M) is specific to the Arbitrum Sepolia test case, while other test cases use smaller amounts (~100K). This suggests it's an intentional adjustment for this particular testnet.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if similar timeouts exist in other test files rg "it\(.*,.*30000" --type tsLength of output: 31
Script:
#!/bin/bash # Search for quote-related test files fd "quote.*\.spec" --type f # Search for jest timeout configurations rg "jest.*timeout" --type ts # Search for similar amount patterns in test files rg "amount.*10000000|10000000.*amount" --type ts # Search for other quote tests rg "quote\.getQuote" --type tsLength of output: 448
Script:
#!/bin/bash # Get the complete quote test file cat packages/core/src/lib/quote/quote.spec.tsLength of output: 3440
packages/core/src/lib/garden/garden.spec.ts (1)
39-44:⚠️ Potential issueAdd Monad testnet configuration.
The PR objective is to add Monad testnet support, but the configuration only includes Arbitrum Sepolia and Ethereum Sepolia. Please add the necessary Monad testnet configuration.
| http: [ | ||
| 'https://rpc.monad-testnet.category.xyz/rpc/SPa4TbSgMJxz5ywmzYvvc9O5nkAVGTgOcsi7yErG', | ||
| ], |
There was a problem hiding this comment.
Security: Avoid hardcoding API keys in RPC URLs
The RPC URL contains what appears to be an API key (SPa4TbSgMJxz5ywmzYvvc9O5nkAVGTgOcsi7yErG). API keys should be configured through environment variables to prevent exposure and enable key rotation.
Consider:
- Moving the API key to environment variables
- Using a configuration service to manage such sensitive values
| @@ -7,20 +7,22 @@ import { Quote } from './quote'; | |||
| import { describe, expect, it } from 'vitest'; | |||
|
|
|||
| describe('quote', () => { | |||
| const quoteUrl = 'http://localhost:6969'; | |||
| const quoteUrl = 'https://price.garden.finance'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding production URLs in tests.
Using production URLs in tests can lead to flaky tests and unnecessary production load. Consider using environment variables or test configurations.
- const quoteUrl = 'https://price.garden.finance';
+ const quoteUrl = process.env.QUOTE_URL || 'https://price.garden.finance';📝 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 quoteUrl = 'https://price.garden.finance'; | |
| const quoteUrl = process.env.QUOTE_URL || 'https://price.garden.finance'; |
| // import { Quote } from './../quote/quote'; | ||
| // import { Orderbook } from 'gardenfi/orderbook'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented out code.
Instead of commenting out code, either:
- Remove it if it's no longer needed
- Add a TODO explaining why it's temporarily commented out
- Use feature flags if the functionality needs to be toggled
This helps maintain code clarity and prevents technical debt.
Also applies to: 36-41
Summary by CodeRabbit
Release Notes
Package Updates
@gardenfi/corefrom 2.0.3 to 2.0.4@gardenfi/orderbookfrom 2.0.0 to 2.0.1@gardenfi/react-hooksfrom 2.0.4 to 2.0.5New Features
Testing