You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
This reduces the number of reads to safe repos on every adapter initialization, by fetching and writing the contract deployment data directly into the project build. These artifacts are now a constant in the project.
PR Type
enhancement, configuration changes, tests
Description
Added new scripts to fetch and process Safe and module deployments, and to write deployment data to a file.
Refactored SafeContractSuite to use pre-fetched deployment data instead of fetching it dynamically.
Updated NearSafe class and tests to use the new SafeContractSuite initialization.
Added types for Safe deployments and deployment data.
Updated build scripts and dependencies in package.json.
Updated TypeScript configuration to include new script.
Changes walkthrough 📝
Relevant files
Enhancement
6 files
fetch-deployments.ts
Add script to fetch and process Safe and module deployments
scripts/fetch-deployments.ts
Added script to fetch and process Safe and module deployments.
Defined deployment versions and chain ID.
Implemented functions to get and fetch deployments.
Error Handling The function fetchDeployments lacks comprehensive error handling for asynchronous operations within the Promise.all. Consider adding more specific error handling for each individual promise to provide clearer debugging and fault isolation.
Hardcoded Values The chain ID '11155111' is hardcoded in multiple places. Consider defining it as a constant or making it configurable to enhance flexibility and maintainability.
Error Handling The error handling in fetchAndWriteDeployments function could be improved by providing more detailed error messages or handling specific types of errors differently to aid in troubleshooting.
const [singleton, proxyFactory, moduleSetup, m4337] = await Promise.all([
- safeDeployment(getSafeL2SingletonDeployment),- safeDeployment(getProxyFactoryDeployment),- m4337Deployment(getSafeModuleSetupDeployment),- m4337Deployment(getSafe4337ModuleDeployment),+ safeDeployment(getSafeL2SingletonDeployment).catch(e => { throw new Error(`Failed to fetch singleton: ${e}`); }),+ safeDeployment(getProxyFactoryDeployment).catch(e => { throw new Error(`Failed to fetch proxyFactory: ${e}`); }),+ m4337Deployment(getSafeModuleSetupDeployment).catch(e => { throw new Error(`Failed to fetch moduleSetup: ${e}`); }),+ m4337Deployment(getSafe4337ModuleDeployment).catch(e => { throw new Error(`Failed to fetch m4337: ${e}`); }),
]);
Suggestion importance[1-10]: 9
Why: Adding error handling for individual promises within Promise.all enhances robustness and provides more detailed error information, which is crucial for debugging and reliability.
9
Best practice
Enhance the immutability of ABI definitions by using the readonly modifier
To ensure that the ABI definitions are immutable and cannot be altered at runtime, consider marking the abi array as readonly. This change will enforce immutability at the type level.
Why: Marking the abi array as readonly enforces immutability, which is a good practice for ensuring that the ABI definitions cannot be altered at runtime. This is a significant improvement for maintaining the integrity of the ABI.
8
Improve type safety and readability by using a more specific type for event input names
Consider using a more specific type for the name field in the inputs array of the event definitions. Using a string type like "address" or "bytes32" directly can help avoid potential type mismatches and improve code readability.
-name: "owner",+name: "owner" as const,
type: "address",
Suggestion importance[1-10]: 7
Why: Using as const for the name field improves type safety and readability, but it is a minor improvement and not critical for functionality.
7
Initialize class properties directly in the constructor for safer usage
Consider initializing SAFE_DEPLOYMENTS directly in the constructor to ensure it is always defined before use, which can prevent potential runtime errors if SAFE_DEPLOYMENTS is accessed before the constructor runs.
Why: Initializing SAFE_DEPLOYMENTS directly in the constructor can prevent potential runtime errors, but the current code already initializes it immediately within the constructor, so the improvement is minor.
7
Maintainability
Use a constant or configurable option for chain ID to enhance code flexibility
Replace the hardcoded chain ID in getClient with a configurable option or constant to enhance flexibility and maintainability of the code.
Why: Replacing the hardcoded chain ID with a configurable option improves maintainability and flexibility, making the code easier to adapt to different environments.
8
Reduce redundancy and improve manageability by extracting common event input patterns into a separate type
For better code organization and potential reuse, consider extracting the repeated event input patterns into a separate type or interface. This approach can reduce redundancy and make the ABI easier to manage and update.
Why: Extracting repeated event input patterns into a separate type reduces redundancy and improves code maintainability. However, it is a minor refactor and does not address any critical issues.
6
Possible issue
Prevent runtime errors by adding explicit type assertions in the ABI definitions
To avoid potential runtime errors and ensure that the ABI matches expected types, consider adding explicit type assertions or checks where the ABI is consumed. This can prevent issues when interacting with the blockchain.
Why: Adding as const for the type field in the ABI definitions helps prevent potential runtime errors by ensuring type correctness. This is a useful improvement for type safety.
7
Readability
Improve variable naming for clarity and maintainability
Use a more descriptive variable name than tsContent for the TypeScript content string to improve code readability.
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.
User description
This reduces the number of reads to safe repos on every adapter initialization, by fetching and writing the contract deployment data directly into the project build. These artifacts are now a constant in the project.
PR Type
enhancement, configuration changes, tests
Description
SafeContractSuite
to use pre-fetched deployment data instead of fetching it dynamically.NearSafe
class and tests to use the newSafeContractSuite
initialization.package.json
.Changes walkthrough 📝
6 files
fetch-deployments.ts
Add script to fetch and process Safe and module deployments
scripts/fetch-deployments.ts
safe-deployments.ts
Add script to fetch and write deployment data
scripts/safe-deployments.ts
deployments.ts
Add auto-generated deployment data file
src/_gen/deployments.ts
constants.
safe.ts
Refactor SafeContractSuite to use pre-fetched deployment data
src/lib/safe.ts
SafeContractSuite
to use pre-fetched deployment data.near-safe.ts
Update NearSafe to use new SafeContractSuite initialization
src/near-safe.ts
NearSafe
class to use the newSafeContractSuite
initialization.
types.ts
Add types for Safe deployments and deployment data
src/types.ts
1 files
safe.spec.ts
Update tests for new ViemPack initialization
tests/lib/safe.spec.ts
ViemPack
initialization.2 files
package.json
Update build scripts and dependencies
package.json
devDependencies
.tsconfig.json
Update TypeScript configuration to include new script
tsconfig.json