Simplifying chain expansion #32
Conversation
WalkthroughThe pull request introduces significant changes to the repository, including a renaming from 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (3)
README.md (1)
73-124: Excellent work on streamlining the process for adding new chains!The instructions are clear, well-structured, and provide a comprehensive guide for setting up indexing for new chains. The use of a configuration file and environment variables allows for great flexibility and customization.
A few suggestions:
- Consider adding a note about the precedence order of environment variables and configuration values to clarify which takes priority when both are specified.
- It might be helpful to provide some example configuration files or environment variable setups for common chains to serve as a starting point for users.
Do you want me to generate some example configuration files or environment variable setups for popular EVM-based chains to include in the documentation?
Tools
LanguageTool
[uncategorized] ~96-~96: Loose punctuation mark.
Context: ...or the new chain: -PROCESSOR_NAME: Set this to your chosen processor name ...(UNLIKELY_OPENING_PUNCTUATION)
src/utils/ibc-processor.ts (2)
25-33: Clarify Error Message for Missing Processor NameThe error message when
processorNameis missing could be more informative by indicating both the missing function argument and the environment variable.Apply this diff to improve the error message:
if (!processorName) { throw new Error( - 'PROCESSOR_NAME environment variable is not set' + 'Processor name is not provided and PROCESSOR_NAME environment variable is not set' ) }
50-52: Ensure Complete Environment Variable InterpolationWhile transactions and contracts are interpolated with environment variables, consider applying interpolation to all string values within the config to maintain consistency and prevent potential misconfigurations.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- README.md (3 hunks)
- commands.json (1 hunks)
- squid.yaml (1 hunks)
- src/chains/backfill.ts (0 hunks)
- src/chains/optimism.ts (0 hunks)
- src/chains/processor.ts (1 hunks)
- src/chains/wallets/base.ts (0 hunks)
- src/chains/wallets/eth.ts (0 hunks)
- src/chains/wallets/optimism.ts (0 hunks)
- src/utils/ibc-processor.ts (4 hunks)
Files not reviewed due to no reviewable changes (5)
- src/chains/backfill.ts
- src/chains/optimism.ts
- src/chains/wallets/base.ts
- src/chains/wallets/eth.ts
- src/chains/wallets/optimism.ts
Additional context used
LanguageTool
README.md
[uncategorized] ~96-~96: Loose punctuation mark.
Context: ...or the new chain: -PROCESSOR_NAME: Set this to your chosen processor name ...(UNLIKELY_OPENING_PUNCTUATION)
Biome
src/utils/ibc-processor.ts
[error] 98-98: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 108-108: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
Additional comments not posted (10)
src/chains/processor.ts (1)
4-4: Verify that removing the'base'argument doesn't break existing functionality.The change to the
runProcessorfunction call aligns with the PR objective of simplifying the file processing workflow. However, please ensure that removing the'base'argument doesn't break any existing functionality or dependencies in the codebase.Run the following script to verify the impact of this change:
Verification successful
Removal of 'base' argument from runProcessor is verified and consistent
The change to
runProcessor(handler)in src/chains/processor.ts has been verified and found to be consistent with the rest of the codebase. Here are the key findings:
- The
runProcessorfunction is defined in src/utils/ibc-processor.ts and only expects a single argument (handler).- No other instances of
runProcessorbeing called with 'base' as the first argument were found in the codebase.- The change aligns with the function's definition and usage across the project.
Based on this analysis, the removal of the 'base' argument appears to be a deliberate API simplification that doesn't break existing functionality.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that removing the 'base' argument from runProcessor doesn't break existing functionality. # Test 1: Search for usage of the 'base' argument in the runProcessor function. Expect: No occurrences. rg --type typescript $'runProcessor\(\'base\'' # Test 2: Search for the runProcessor function usage. Expect: Only occurrences with a single argument. rg --type typescript -A 5 $'runProcessor\('Length of output: 166
Script:
#!/bin/bash # Description: Verify the usage of runProcessor function after the removal of 'base' argument # Find TypeScript and JavaScript files fd '\.(ts|js)$' | xargs grep -n -A 5 "runProcessor(" | grep -v "^--$" # Search for any remaining instances of 'base' as the first argument fd '\.(ts|js)$' | xargs grep -n "runProcessor('base'" fd '\.(ts|js)$' | xargs grep -n 'runProcessor("base"'Length of output: 708
squid.yaml (4)
13-13: LGTM!The simplification of the command argument for the
optimism-processoraligns with the PR objective of streamlining the processing workflow. This change allows for more flexible control over the processor's behavior through external configurations.
15-15: LGTM!The update to the environment variable for the
optimism-processoraligns with the PR objective of centralizing control over the processor's behavior through external configurations. SettingPROCESSOR_NAMEtooptimismclearly indicates the intended chain for this processor.
19-19: LGTM!The simplification of the command argument for the
base-processoraligns with the PR objective of streamlining the processing workflow. This change allows for more flexible control over the processor's behavior through external configurations.
21-21: LGTM!The update to the environment variable for the
base-processoraligns with the PR objective of centralizing control over the processor's behavior through external configurations. SettingPROCESSOR_NAMEtobaseclearly indicates the intended chain for this processor.commands.json (2)
47-50: Simplified command structure and improved configuration flexibility.The changes to the "process" command definition are a great improvement:
- Consolidating multiple processor commands into a single generic command simplifies the overall command structure and reduces duplication.
- Using the "--require=dotenv/config" flag to load environment variables from a .env file provides flexibility in configuring the processor without modifying the code.
- Keeping the "build" and "migration:apply" dependencies ensures that the project is properly set up before running the processor.
Well done!
52-55: Simplified production command and improved environment handling.The changes to the "process:prod" command definition are appropriate for a production environment:
- Consolidating multiple production processor commands into a single generic command simplifies the overall command structure and reduces duplication.
- Removing the "--require=dotenv/config" flag indicates that the production environment should be configured through other means, such as system environment variables. This is a good practice for production environments.
- Removing the "build" dependency suggests that the production command assumes the project is already built, which is a reasonable assumption for a production setup.
- Keeping the command hidden from the user is appropriate for a production command.
Just ensure that the production environment is properly configured with the necessary environment variables and that the project is pre-built before running this command.
README.md (2)
8-9: LGTM!The repository URL and name have been updated correctly to reflect the project's expanded scope.
30-31: LGTM!The commands to run the Optimism and Base processors separately are correct and consistent with the project's focus on EVM-based chains.
src/utils/ibc-processor.ts (1)
14-20: ConfigSchema Enhancement Looks GoodThe addition of optional configuration parameters enhances flexibility. The schema changes are appropriate and well-defined.
| let rpcRateLimit = config[processorName]?.rpcRateLimit ?? Number(process.env.RPC_RATE_LIMIT) | ||
| if (rpcRateLimit === undefined) { | ||
| throw new Error(`Missing RPC rate limit`) | ||
| } |
There was a problem hiding this comment.
Validate Numeric Configuration to Handle NaN Values
When parsing numeric environment variables, Number(...) can return NaN if the value is invalid or undefined. Checking rpcRateLimit === undefined won't catch NaN. Use Number.isNaN to ensure the value is a valid number.
Apply this diff to fix the issue:
let rpcRateLimit = config[processorName]?.rpcRateLimit ?? Number(process.env.RPC_RATE_LIMIT)
- if (rpcRateLimit === undefined) {
+ if (Number.isNaN(rpcRateLimit)) {
throw new Error(`Missing or invalid RPC rate limit`)
}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.
| let rpcRateLimit = config[processorName]?.rpcRateLimit ?? Number(process.env.RPC_RATE_LIMIT) | |
| if (rpcRateLimit === undefined) { | |
| throw new Error(`Missing RPC rate limit`) | |
| } | |
| let rpcRateLimit = config[processorName]?.rpcRateLimit ?? Number(process.env.RPC_RATE_LIMIT) | |
| if (Number.isNaN(rpcRateLimit)) { | |
| throw new Error(`Missing or invalid RPC rate limit`) | |
| } |
| let customMaxBatchCallSize = Number(process.env[`${capProcessorName}_MAX_BATCH_CALL_SIZE`]) | ||
| if (!isNaN(customMaxBatchCallSize)) { | ||
| maxBatchCallSize = customMaxBatchCallSize | ||
| } |
There was a problem hiding this comment.
Use Number.isNaN Instead of isNaN for Type-Safe Checks
Consistent with the previous recommendation, replace isNaN with Number.isNaN to avoid type coercion issues.
Apply this diff to fix the issue:
let customMaxBatchCallSize = Number(process.env[`${capProcessorName}_MAX_BATCH_CALL_SIZE`])
- if (!isNaN(customMaxBatchCallSize)) {
+ if (!Number.isNaN(customMaxBatchCallSize)) {
maxBatchCallSize = customMaxBatchCallSize
}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.
| let customMaxBatchCallSize = Number(process.env[`${capProcessorName}_MAX_BATCH_CALL_SIZE`]) | |
| if (!isNaN(customMaxBatchCallSize)) { | |
| maxBatchCallSize = customMaxBatchCallSize | |
| } | |
| let customMaxBatchCallSize = Number(process.env[`${capProcessorName}_MAX_BATCH_CALL_SIZE`]) | |
| if (!Number.isNaN(customMaxBatchCallSize)) { | |
| maxBatchCallSize = customMaxBatchCallSize | |
| } |
Tools
Biome
[error] 108-108: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
| let customRateLimit = Number(process.env[`${capProcessorName}_RPC_RATE_LIMIT`]) | ||
| if (!isNaN(customRateLimit)) { | ||
| rpcRateLimit = customRateLimit | ||
| } |
There was a problem hiding this comment.
Use Number.isNaN Instead of isNaN for Type-Safe Checks
The use of isNaN can lead to unexpected results due to type coercion. It's safer to use Number.isNaN, which doesn't coerce the argument.
Apply this diff to fix the issue:
let customRateLimit = Number(process.env[`${capProcessorName}_RPC_RATE_LIMIT`])
- if (!isNaN(customRateLimit)) {
+ if (!Number.isNaN(customRateLimit)) {
rpcRateLimit = customRateLimit
}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.
| let customRateLimit = Number(process.env[`${capProcessorName}_RPC_RATE_LIMIT`]) | |
| if (!isNaN(customRateLimit)) { | |
| rpcRateLimit = customRateLimit | |
| } | |
| let customRateLimit = Number(process.env[`${capProcessorName}_RPC_RATE_LIMIT`]) | |
| if (!Number.isNaN(customRateLimit)) { | |
| rpcRateLimit = customRateLimit | |
| } |
Tools
Biome
[error] 98-98: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
| let maxBatchCallSize = config[processorName]?.maxBatchCallSize ?? Number(process.env.MAX_BATCH_CALL_SIZE ?? "100") | ||
| if (maxBatchCallSize === undefined) { | ||
| throw new Error(`Missing max batch call size`) |
There was a problem hiding this comment.
Validate Numeric Configuration to Handle NaN Values
Similar to rpcRateLimit, ensure that maxBatchCallSize is a valid number and not NaN.
Apply this diff to fix the issue:
let maxBatchCallSize = config[processorName]?.maxBatchCallSize ?? Number(process.env.MAX_BATCH_CALL_SIZE ?? "100")
- if (maxBatchCallSize === undefined) {
+ if (Number.isNaN(maxBatchCallSize)) {
throw new Error(`Missing or invalid max batch call size`)
}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.
| let maxBatchCallSize = config[processorName]?.maxBatchCallSize ?? Number(process.env.MAX_BATCH_CALL_SIZE ?? "100") | |
| if (maxBatchCallSize === undefined) { | |
| throw new Error(`Missing max batch call size`) | |
| let maxBatchCallSize = config[processorName]?.maxBatchCallSize ?? Number(process.env.MAX_BATCH_CALL_SIZE ?? "100") | |
| if (Number.isNaN(maxBatchCallSize)) { | |
| throw new Error(`Missing or invalid max batch call size`) |
| let finalityConfirmation = config[processorName]?.finalityConfirmation ?? Number(process.env.FINALITY_CONFIRMATION); | ||
| if (finalityConfirmation) { | ||
| processor = processor.setFinalityConfirmation(Number(finalityConfirmation)) | ||
| processor = processor.setFinalityConfirmation(finalityConfirmation) |
There was a problem hiding this comment.
Validate Numeric Configuration for finalityConfirmation
Ensure that finalityConfirmation is validated against NaN to prevent unintended behavior.
Apply this diff to fix the issue:
let finalityConfirmation = config[processorName]?.finalityConfirmation ?? Number(process.env.FINALITY_CONFIRMATION);
- if (finalityConfirmation) {
+ if (!Number.isNaN(finalityConfirmation)) {
processor = processor.setFinalityConfirmation(finalityConfirmation)
}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.
| let finalityConfirmation = config[processorName]?.finalityConfirmation ?? Number(process.env.FINALITY_CONFIRMATION); | |
| if (finalityConfirmation) { | |
| processor = processor.setFinalityConfirmation(Number(finalityConfirmation)) | |
| processor = processor.setFinalityConfirmation(finalityConfirmation) | |
| let finalityConfirmation = config[processorName]?.finalityConfirmation ?? Number(process.env.FINALITY_CONFIRMATION); | |
| if (!Number.isNaN(finalityConfirmation)) { | |
| processor = processor.setFinalityConfirmation(finalityConfirmation) |
Inkvi
force-pushed
the
inkvi/chain-expansion
branch
from
12193f9 to
0a00314
Compare
Inkvi
force-pushed
the
inkvi/chain-expansion
branch
from
0a00314 to
939b7fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (11)
- README.md (3 hunks)
- commands.json (1 hunks)
- package.json (0 hunks)
- squid.yaml (1 hunks)
- src/chains/backfill.ts (0 hunks)
- src/chains/optimism.ts (0 hunks)
- src/chains/processor.ts (1 hunks)
- src/chains/wallets/base.ts (0 hunks)
- src/chains/wallets/eth.ts (0 hunks)
- src/chains/wallets/optimism.ts (0 hunks)
- src/utils/ibc-processor.ts (4 hunks)
Files not reviewed due to no reviewable changes (6)
- package.json
- src/chains/backfill.ts
- src/chains/optimism.ts
- src/chains/wallets/base.ts
- src/chains/wallets/eth.ts
- src/chains/wallets/optimism.ts
Files skipped from review as they are similar to previous changes (2)
- squid.yaml
- src/chains/processor.ts
Additional context used
LanguageTool
README.md
[uncategorized] ~96-~96: Loose punctuation mark.
Context: ...or the new chain: -PROCESSOR_NAME: Set this to your chosen processor name ...(UNLIKELY_OPENING_PUNCTUATION)
Biome
src/utils/ibc-processor.ts
[error] 89-89: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 99-99: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
Additional comments not posted (14)
commands.json (2)
47-50: LGTM!The updated
"process"command simplifies the processor execution by centralizing it to a single filelib/chains/processor.js. This change reduces redundancy and improves maintainability.Loading the
.envfile allows for flexible configuration through environment variables, while keeping the dependencies unchanged ensures the necessary setup steps are performed before running the processor.Overall, this change enhances the command structure and makes it easier to manage the processor execution.
52-55: Looks good!Updating the
"process:prod"command to use the same generic processor filelib/chains/processor.jsensures consistency between the development and production environments. This reduces the chances of errors caused by differences in the code.Removing the
.envfile loading is appropriate for production, as the configuration should be managed through other means, such as environment variables set in the production environment.Keeping the
"migration:apply"dependency ensures the database is properly set up before running the processor in production.Overall, this change simplifies the production command, aligns it with the development command, and promotes a consistent and reliable production setup.
src/utils/ibc-processor.ts (6)
13-19: LGTM!The addition of new optional configuration parameters to the
ConfigSchemaenhances the flexibility and configurability of theIbcProcessor. The changes look good.
24-32: LGTM!The updates to the
IbcProcessorfunction signature and the handling of theprocessorNameparameter improve usability and provide clear error handling. The changes look good.
44-47: LGTM!The introduction of environment variable interpolation in the config file is a useful feature that enhances flexibility. The fallback behavior of keeping the placeholder if the environment variable is not found is a good approach. The changes look good.
78-106: LGTM!The updates to the configuration value retrieval logic prioritize config values over environment variables, which provides a more consistent and manageable approach. The error handling for missing required values and the flexibility to set custom rate limits and max batch call sizes through processor-specific environment variables are valuable additions. Overall, the changes look good, with the exception of the
isNaNusage mentioned in the previous comments.Tools
Biome
[error] 89-89: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 99-99: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
109-123: LGTM!The updates to the configuration value retrieval logic for
gateway,fromBlock, andfinalityConfirmationmaintain consistency with the overall configuration approach. Setting the processor's properties based on the defined values allows for flexible configuration. The changes look good.
145-154: LGTM!The changes to the return value of
IbcProcessorand the corresponding updates inrunProcessorimprove code organization and readability. The version handling logic ensures that the configured version takes precedence and enforces the requirement for a valid version. Overall, the changes look good, with the exception of the version validation logic mentioned in the previous comment.README.md (6)
8-9: LGTM!The repository URL update aligns with the PR objective of renaming the repository to reflect expanded functionality.
30-31: LGTM!The addition of commands for running Optimism and Base processors separately aligns with the PR objective of simplifying the command structure for processing various blockchain transactions. This change improves the flexibility of running individual processors.
73-89: Configuration structure updates look good!The changes to the configuration file structure, including the addition of optional fields such as
rpc,rpcRateLimit,maxBatchCallSize,gateway,fromBlock,finalityConfirmation, andversion, simplify the process of adding a new chain for indexing. The ability to override these fields with environment variables provides flexibility in configuration.These updates align with the PR objective of improving the ease of configuration and setup for handling multichain transfers.
92-93: Clarification and note are helpful!The clarification about all fields being optional and the ability to override them with environment variables is helpful for users setting up the indexer. It provides flexibility in configuration and ensures that users are aware of the precedence of environment variables over configuration file values.
The note about the processor's behavior when
contractsandtransactionsare omitted is also valuable, as it prevents unexpected behavior and informs users about the conditions under which the processor won't perform any actual work.
94-110: Environment variable instructions are comprehensive!The instructions for setting environment variables for a new chain are clear and comprehensive. They cover both global environment variables and chain-specific overrides, providing flexibility in configuration.
The inclusion of variables such as
PROCESSOR_NAME,CONFIG_FILE,{PROCESSOR_NAME}_RPC,{PROCESSOR_NAME}_GATEWAY,DISPATCHER_ADDRESS_{PROCESSOR_NAME}_START_BLOCK,{PROCESSOR_NAME}_VERSION,RPC_RATE_LIMIT,MAX_BATCH_CALL_SIZE, andFINALITY_CONFIRMATIONensures that users have control over various aspects of the indexer's behavior.The chain-specific overrides for
RPC_RATE_LIMITandMAX_BATCH_CALL_SIZEallow for fine-grained configuration based on the requirements of each chain.Tools
LanguageTool
[uncategorized] ~96-~96: Loose punctuation mark.
Context: ...or the new chain: -PROCESSOR_NAME: Set this to your chosen processor name ...(UNLIKELY_OPENING_PUNCTUATION)
112-124: Instructions for running the processor are clear and emphasize benefits!The instructions for running the processor for a new chain using the
PROCESSOR_NAMEenvironment variable are clear and straightforward. By simply specifying the desired processor name, users can easily run the processor with the configuration specific to that chain.The emphasis on using the same processor code for multiple chains is a key benefit of this setup. It simplifies maintenance and reduces code duplication, as the same codebase can be used across different chains with minimal modifications. This approach promotes code reuse and makes it easier to manage the indexer for multiple chains.
The note at the end reinforces this benefit and highlights the flexibility of this setup in terms of running the processor for different chains with minimal changes.
| let version = process.env[`${processorName.toUpperCase()}_VERSION`] ?? config.version ?? 1; | ||
| if (!version) { | ||
| throw new Error('Version not set') | ||
| } |
There was a problem hiding this comment.
Correct Version Validation Logic
Checking !version may incorrectly throw an error if version is 0. Since version defaults to 1, this might not be an immediate issue, but for robustness, consider validating whether version is a valid number.
Apply this diff to improve the check:
let version = process.env[`${processorName.toUpperCase()}_VERSION`] ?? config.version ?? 1;
- if (!version) {
+ if (Number.isNaN(Number(version))) {
throw new Error('Invalid or missing version')
}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.
| let version = process.env[`${processorName.toUpperCase()}_VERSION`] ?? config.version ?? 1; | |
| if (!version) { | |
| throw new Error('Version not set') | |
| } | |
| let version = process.env[`${processorName.toUpperCase()}_VERSION`] ?? config.version ?? 1; | |
| if (Number.isNaN(Number(version))) { | |
| throw new Error('Invalid or missing version') | |
| } |
| if (!isNaN(customMaxBatchCallSize)) { | ||
| maxBatchCallSize = customMaxBatchCallSize |
There was a problem hiding this comment.
Use Number.isNaN Instead of isNaN for Type-Safe Checks
Consistent with the previous recommendation, replace isNaN with Number.isNaN to avoid type coercion issues.
Apply this diff to fix the issue:
let customMaxBatchCallSize = Number(process.env[`${capProcessorName}_MAX_BATCH_CALL_SIZE`])
- if (!isNaN(customMaxBatchCallSize)) {
+ if (!Number.isNaN(customMaxBatchCallSize)) {
maxBatchCallSize = customMaxBatchCallSize
}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.
| if (!isNaN(customMaxBatchCallSize)) { | |
| maxBatchCallSize = customMaxBatchCallSize | |
| if (!Number.isNaN(customMaxBatchCallSize)) { | |
| maxBatchCallSize = customMaxBatchCallSize |
Tools
Biome
[error] 99-99: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
| if (!isNaN(customRateLimit)) { | ||
| rpcRateLimit = customRateLimit |
There was a problem hiding this comment.
Use Number.isNaN Instead of isNaN for Type-Safe Checks
The use of isNaN can lead to unexpected results due to type coercion. It's safer to use Number.isNaN, which doesn't coerce the argument.
Apply this diff to fix the issue:
let customRateLimit = Number(process.env[`${capProcessorName}_RPC_RATE_LIMIT`])
- if (!isNaN(customRateLimit)) {
+ if (!Number.isNaN(customRateLimit)) {
rpcRateLimit = customRateLimit
}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.
| if (!isNaN(customRateLimit)) { | |
| rpcRateLimit = customRateLimit | |
| let customRateLimit = Number(process.env[`${capProcessorName}_RPC_RATE_LIMIT`]) | |
| if (!Number.isNaN(customRateLimit)) { | |
| rpcRateLimit = customRateLimit |
Tools
Biome
[error] 89-89: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
|
@Inkvi Looking at the README it's not totally clear to me how to add a new chain. Are developers supposed to add a new configuration file, update an environment variable, or both? Can we update the .env already being tracked with examples, or do we need to add the env variable for them? |
They can do either or both. Env vars take precedence over the config. |
Creating a new processor for each file is no longer required. The processor can be fully controlled via env vars and a config file.
Summary by CodeRabbit
New Features
evm-indexer, reflecting broader functionality.Bug Fixes
Refactor
Documentation