Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add Admin scripts for Single Request Proxy Factory #1478

Conversation

aimensahnoun
Copy link
Member

@aimensahnoun aimensahnoun commented Oct 30, 2024

Resolves #1462

Description of the changes

  • Update the transferOwner task to include Single Request Proxy Factory
  • Add a new task to update the fee proxy addresses in the Single Request Proxy Factory.

Summary by CodeRabbit

  • New Features

    • Introduced the SingleRequestProxyFactory contract setup functionality, allowing for streamlined updates of fee proxy addresses.
    • Added new asynchronous functions to update ERC20 and Ethereum fee proxy addresses.
    • Implemented a new setup process for the SingleRequestProxyFactory contract across multiple networks.
  • Bug Fixes

    • Enhanced error handling during contract setup to log warnings for invalid addresses.
  • Documentation

    • Updated documentation to reflect new functionalities and methods related to the SingleRequestProxyFactory.

Copy link
Contributor

coderabbitai bot commented Oct 30, 2024

Walkthrough

The pull request introduces several changes primarily focused on the SingleRequestProxyFactory contract. A new case is added to the getArtifact function to handle this contract's artifact. Additionally, two asynchronous functions are implemented to update the ERC20 and Ethereum fee proxy addresses within the contract. A new TypeScript file is created for setting up the SingleRequestProxyFactory, and modifications are made to the setupContract function to incorporate this new setup functionality.

Changes

File Path Change Summary
packages/smart-contracts/scripts-create2/utils.ts Added case for SingleRequestProxyFactory in getArtifact function to return its artifact.
packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts Introduced updateSRPFERC20FeeProxyAddress and updateSRPFEthereumFeeProxyAddress functions for updating fee proxies.
packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts Created setupSRPF function for setting up the SingleRequestProxyFactory contract post-deployment.
packages/smart-contracts/scripts-create2/contract-setup/setups.ts Updated setupContract function to include handling for the SingleRequestProxyFactory contract.

Assessment against linked issues

Objective Addressed Explanation
Add Admins Scripts for the SingleRequestProxyFactory to the SDK (#1462)

Possibly related PRs

Suggested reviewers

  • rodrigopavezi
  • kevindavee
  • alexandre-abrioux
  • sstefdev
  • MantisClone
  • yomarion

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between e91492c and 69d7b4c.

📒 Files selected for processing (3)
  • packages/smart-contracts/hardhat.config.ts (2 hunks)
  • packages/smart-contracts/scripts-create2/contract-setup/update-fee-proxies.ts (1 hunks)
  • packages/smart-contracts/scripts-create2/utils.ts (1 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/smart-contracts/scripts-create2/utils.ts (1)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/scripts-create2/utils.ts:23-23
Timestamp: 2024-09-27T11:54:45.798Z
Learning: When a contract has not been deployed yet and thus has no artifact, the `getArtifact` function will be updated after deployment.
🔇 Additional comments (6)
packages/smart-contracts/scripts-create2/utils.ts (3)

Line range hint 10-24: LGTM: SingleRequestProxyFactory correctly added to deployment list

The inclusion of 'SingleRequestProxyFactory' in the CREATE2 deployment list aligns with the PR objectives and follows the existing pattern.


Line range hint 71-86: LGTM: isContractDeployed inherently supports SingleRequestProxyFactory

The existing implementation automatically handles the new contract through its use of getArtifact and robust error handling. No changes were needed here, which is a good example of modular design.


60-61: Verify SingleRequestProxyFactory artifact availability

The implementation follows the established pattern correctly. However, let's verify that the artifact is properly exported from the artifacts module.

✅ Verification successful

SingleRequestProxyFactory artifact is properly exported and available

The artifact is correctly exported from packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts with proper typing and version configuration, following the established pattern of other artifacts in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that singleRequestProxyFactoryArtifact is exported from artifacts
rg -l "export.*singleRequestProxyFactoryArtifact" "packages/smart-contracts/src/"

Length of output: 159


Script:

#!/bin/bash
# Let's verify the actual export in the found file
cat packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts

Length of output: 717

packages/smart-contracts/hardhat.config.ts (2)

25-25: LGTM!

The import statement follows the project's established patterns and is properly organized within the scripts-create2 directory.


408-417: LGTM! Task implementation follows established patterns.

The task is well-structured and includes necessary security checks:

  • Follows the project's task naming and implementation patterns
  • Includes DEPLOYER_KEY_GUARD for security
  • Properly handles EOA context

Let's verify the consistency of the implementation with other similar tasks:

✅ Verification successful

Task implementation is consistent with existing patterns

The verification confirms that the task follows the same patterns used in other similar tasks in the codebase:

  • EOA flag pattern is consistently used in tasks like 'update-contracts' and 'transfer-ownership'
  • DEPLOYER_KEY_GUARD is properly implemented as a security measure across all sensitive tasks
  • Implementation matches the established structure for task definitions
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if other tasks follow similar patterns for EOA handling and DEPLOYER_KEY_GUARD usage

# Test: Search for tasks with EOA flag and DEPLOYER_KEY_GUARD
rg "addFlag\('eoa'" -A 10
rg "DEPLOYER_KEY_GUARD" -A 2

Length of output: 3877

packages/smart-contracts/scripts-create2/contract-setup/update-fee-proxies.ts (1)

21-21: Verify Networks Configuration

Ensure that hre.config.xdeploy.networks is correctly configured and contains the expected network names. Incorrect or missing network configurations could lead to unexpected behavior.

Run the following script to list the configured networks:

✅ Verification successful

Networks Configuration is Properly Defined

The hre.config.xdeploy.networks is correctly configured in the hardhat config. It either uses a single network from the NETWORK environment variable or defaults to a comprehensive list of networks including mainnet, matic, bsc, celo, xdai, fuse, arbitrum-one, fantom, avalanche, optimism, and moonbeam. The configuration matches the expected type definition in types.ts.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that hre.config.xdeploy.networks is properly configured.

# Test: Output the networks configured in xdeploy. Expect: List of network names.
cat hardhat.config.ts | grep 'xdeploy' -A 10

Length of output: 94


Script:

#!/bin/bash
# Let's first find the hardhat config file as it might be in a different location
fd -t f "hardhat.config" --full-path

# Also search for any files that might contain xdeploy configuration
rg -l "xdeploy"

Length of output: 1487


Script:

#!/bin/bash
# Let's examine the hardhat config file for xdeploy configuration
cat packages/smart-contracts/hardhat.config.ts | grep -A 20 "xdeploy"

# Also check the types file to understand the expected structure
cat packages/smart-contracts/scripts-create2/types.ts | grep -A 10 "xdeploy"

Length of output: 842

Copy link
Member

@MantisClone MantisClone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Aimen. The logic looks good, but the structure should be updated to match the existing convention. 🙏

@aimensahnoun aimensahnoun enabled auto-merge (squash) November 4, 2024 11:31
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Reviewing files that changed from the base of the PR and between 69d7b4c and d354a0a.

📒 Files selected for processing (3)
  • packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts (2 hunks)
  • packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts (1 hunks)
  • packages/smart-contracts/scripts-create2/contract-setup/setups.ts (2 hunks)
🔇 Additional comments (7)
packages/smart-contracts/scripts-create2/contract-setup/setups.ts (2)

8-8: LGTM! Import follows established patterns.

The import statement for setupSingleRequestProxyFactory aligns with the existing import patterns and supports the PR's objective of adding admin scripts for the Single Request Proxy Factory.


54-57: LGTM! Verify setupSingleRequestProxyFactory implementation.

The switch case implementation follows the established pattern and correctly handles the SingleRequestProxyFactory setup.

Let's verify the implementation of the imported setup function:

✅ Verification successful

Let me gather more information about the implementation details of the setupSingleRequestProxyFactory function.


Implementation verified and follows expected patterns

The setupSingleRequestProxyFactory implementation is well-structured and consistent with the codebase patterns:

  • Takes the expected parameters: contractAddress, hre, and signWithEoa
  • Properly handles contract deployment address fallback
  • Updates both ERC20 and Ethereum fee proxy addresses
  • Includes appropriate error handling and logging
  • Uses the same utility functions as other setups (getSignerAndGasFees, updateERC20FeeProxyAddress, updateEthereumFeeProxyAddress)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence and implementation of setupSingleRequestProxyFactory

# Test 1: Check if the setup file exists
fd "setupSingleRequestProxyFactory.ts" --type f

# Test 2: Verify the implementation matches the expected pattern
rg -A 10 "export const setupSingleRequestProxyFactory = async" 

Length of output: 1496


Script:

#!/bin/bash
# Get the complete implementation of setupSingleRequestProxyFactory
cat packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts

Length of output: 2268

packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts (2)

1-9: LGTM! Well-organized imports with clear separation of concerns.

The imports are properly structured, bringing in necessary dependencies for currency handling, contract artifacts, types, and admin tasks.


17-25: LGTM! Well-structured function signature with proper TypeScript types.

The parameter object pattern with optional contractAddress provides good flexibility and maintainability.

packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts (3)

4-4: LGTM: Import changes are appropriate

The addition of ethers to the import statement is necessary for using ethers.utils.getAddress() in the new functions.


415-447: LGTM: Well-implemented ERC20 fee proxy update function

The implementation follows best practices with proper address normalization, clear logging, and avoids unnecessary transactions.


449-483: LGTM: Well-implemented Ethereum fee proxy update function

The implementation follows the same best practices as updateERC20FeeProxyAddress with proper address normalization and clear logging.

Copy link
Member

@MantisClone MantisClone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved 👍 pending comment resolution 🚧

@aimensahnoun aimensahnoun merged commit 4412aa2 into master Nov 5, 2024
24 of 25 checks passed
@aimensahnoun aimensahnoun deleted the 1462-add-admins-scripts-for-the-singlerequestproxyfactory-to-the-sdk branch November 5, 2024 11:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Reviewing files that changed from the base of the PR and between d354a0a and 3beb884.

📒 Files selected for processing (3)
  • packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts (2 hunks)
  • packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts (1 hunks)
  • packages/smart-contracts/scripts-create2/contract-setup/setups.ts (2 hunks)
🧰 Additional context used
📓 Learnings (3)
packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts (1)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts:423-483
Timestamp: 2024-11-05T05:33:53.140Z
Learning: When creating or naming functions related to the SingleRequestProxyFactory, include 'SingleRequestProxyFactory' or 'SRPF' in the function names for clarity.
packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts (5)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts:30-36
Timestamp: 2024-11-05T05:33:36.551Z
Learning: In the RequestNetwork project, admin scripts like `setupSingleRequestProxyFactory.ts` in `packages/smart-contracts/scripts-create2/contract-setup/` do not require extensive error checking.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts:26-28
Timestamp: 2024-11-05T05:33:36.189Z
Learning: In the `requestNetwork` project's TypeScript setup scripts located in `packages/smart-contracts/scripts-create2/contract-setup`, `Promise.all` is used for asynchronous network setup tasks to maintain consistency across scripts.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts:45-58
Timestamp: 2024-11-05T05:04:01.710Z
Learning: When executing blockchain transactions in scripts (e.g., in `packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts`), ensure that transactions are executed serially rather than in parallel to maintain correct execution order and prevent potential issues.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts:38-43
Timestamp: 2024-11-05T05:33:32.481Z
Learning: In the RequestNetwork codebase, setup scripts such as `setupSingleRequestProxyFactory.ts` do not include contract existence checks before interacting with contracts, even though scripts like `check-deployer.ts` do include such checks.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts:423-483
Timestamp: 2024-11-05T05:33:53.140Z
Learning: When creating or naming functions related to the SingleRequestProxyFactory, include 'SingleRequestProxyFactory' or 'SRPF' in the function names for clarity.
packages/smart-contracts/scripts-create2/contract-setup/setups.ts (3)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts:423-483
Timestamp: 2024-11-05T05:33:53.140Z
Learning: When creating or naming functions related to the SingleRequestProxyFactory, include 'SingleRequestProxyFactory' or 'SRPF' in the function names for clarity.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts:38-43
Timestamp: 2024-11-05T05:33:32.481Z
Learning: In the RequestNetwork codebase, setup scripts such as `setupSingleRequestProxyFactory.ts` do not include contract existence checks before interacting with contracts, even though scripts like `check-deployer.ts` do include such checks.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1478
File: packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts:30-36
Timestamp: 2024-11-05T05:33:36.551Z
Learning: In the RequestNetwork project, admin scripts like `setupSingleRequestProxyFactory.ts` in `packages/smart-contracts/scripts-create2/contract-setup/` do not require extensive error checking.
🔇 Additional comments (4)
packages/smart-contracts/scripts-create2/contract-setup/setups.ts (2)

8-8: LGTM! Import statement follows established patterns.

The import statement maintains consistency with other setup imports and follows the naming convention using the 'SRPF' abbreviation.


54-57: LGTM! Switch case implementation is consistent.

The new case for SingleRequestProxyFactory follows the established pattern:

  • Uses consistent parameter passing
  • Maintains the same structure as other cases
  • Uses the full contract name for clarity
packages/smart-contracts/scripts-create2/contract-setup/adminTasks.ts (1)

4-4: LGTM!

The import statement is properly organized and includes the necessary ethers import for address comparison.

packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts (1)

1-69: LGTM!

The setupSRPF function is well-implemented, adhering to project conventions and ensuring proper setup of the SingleRequestProxyFactory across the specified networks. The use of Promise.all aligns with existing patterns in the codebase, and error handling is appropriately managed.

Comment on lines +423 to +483
export const updateSRPFERC20FeeProxyAddress = async (
contract: Contract,
network: CurrencyTypes.EvmChainName,
txOverrides: Overrides,
signer: Wallet,
signWithEoa: boolean,
): Promise<void> => {
const erc20ProxyAddress = artifacts.erc20FeeProxyArtifact.getAddress(network);
const currentErc20Proxy = await contract.erc20FeeProxy();

if (ethers.utils.getAddress(currentErc20Proxy) !== ethers.utils.getAddress(erc20ProxyAddress)) {
await executeContractMethod({
network,
contract,
method: 'setERC20FeeProxy',
props: [erc20ProxyAddress],
txOverrides,
signer,
signWithEoa,
});
console.log(`Updated ERC20FeeProxy to ${erc20ProxyAddress} on ${network}`);
} else {
console.log(`ERC20FeeProxy is already set to ${erc20ProxyAddress} on ${network}`);
}
};

/**
* Updates the Ethereum fee proxy address in the SingleRequestProxyFactory contract
* @param contract SingleRequestProxyFactory contract
* @param network The network used
* @param txOverrides information related to gas fees
* @param signer Who is performing the updating
* @param signWithEoa Is the transaction to be signed by an EOA
*/
export const updateSRPFEthereumFeeProxyAddress = async (
contract: Contract,
network: CurrencyTypes.EvmChainName,
txOverrides: Overrides,
signer: Wallet,
signWithEoa: boolean,
): Promise<void> => {
const ethereumProxyAddress = artifacts.ethereumFeeProxyArtifact.getAddress(network);
const currentEthereumProxy = await contract.ethereumFeeProxy();

if (
ethers.utils.getAddress(currentEthereumProxy) !== ethers.utils.getAddress(ethereumProxyAddress)
) {
await executeContractMethod({
network,
contract,
method: 'setEthereumFeeProxy',
props: [ethereumProxyAddress],
txOverrides,
signer,
signWithEoa,
});
console.log(`Updated EthereumFeeProxy to ${ethereumProxyAddress} on ${network}`);
} else {
console.log(`EthereumFeeProxy is already set to ${ethereumProxyAddress} on ${network}`);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider refactoring to reduce code duplication between proxy update functions.

The two functions share almost identical structure and differ only in method names and variable names. Consider refactoring into a single function with a type parameter:

-export const updateSRPFERC20FeeProxyAddress = async (
-  contract: Contract,
-  network: CurrencyTypes.EvmChainName,
-  txOverrides: Overrides,
-  signer: Wallet,
-  signWithEoa: boolean,
-): Promise<void> => {
-  const erc20ProxyAddress = artifacts.erc20FeeProxyArtifact.getAddress(network);
-  const currentErc20Proxy = await contract.erc20FeeProxy();
-
-  if (ethers.utils.getAddress(currentErc20Proxy) !== ethers.utils.getAddress(erc20ProxyAddress)) {
-    await executeContractMethod({
-      network,
-      contract,
-      method: 'setERC20FeeProxy',
-      props: [erc20ProxyAddress],
-      txOverrides,
-      signer,
-      signWithEoa,
-    });
-    console.log(`Updated ERC20FeeProxy to ${erc20ProxyAddress} on ${network}`);
-  } else {
-    console.log(`ERC20FeeProxy is already set to ${erc20ProxyAddress} on ${network}`);
-  }
-};
-
-export const updateSRPFEthereumFeeProxyAddress = async (
-  contract: Contract,
-  network: CurrencyTypes.EvmChainName,
-  txOverrides: Overrides,
-  signer: Wallet,
-  signWithEoa: boolean,
-): Promise<void> => {
-  const ethereumProxyAddress = artifacts.ethereumFeeProxyArtifact.getAddress(network);
-  const currentEthereumProxy = await contract.ethereumFeeProxy();
-
-  if (
-    ethers.utils.getAddress(currentEthereumProxy) !== ethers.utils.getAddress(ethereumProxyAddress)
-  ) {
-    await executeContractMethod({
-      network,
-      contract,
-      method: 'setEthereumFeeProxy',
-      props: [ethereumProxyAddress],
-      txOverrides,
-      signer,
-      signWithEoa,
-    });
-    console.log(`Updated EthereumFeeProxy to ${ethereumProxyAddress} on ${network}`);
-  } else {
-    console.log(`EthereumFeeProxy is already set to ${ethereumProxyAddress} on ${network}`);
-  }
-};
+type ProxyType = 'ERC20' | 'Ethereum';
+
+const updateSRPFFeeProxyAddress = async (
+  proxyType: ProxyType,
+  contract: Contract,
+  network: CurrencyTypes.EvmChainName,
+  txOverrides: Overrides,
+  signer: Wallet,
+  signWithEoa: boolean,
+): Promise<void> => {
+  const config = {
+    ERC20: {
+      getAddress: () => artifacts.erc20FeeProxyArtifact.getAddress(network),
+      getCurrentProxy: () => contract.erc20FeeProxy(),
+      setMethod: 'setERC20FeeProxy',
+    },
+    Ethereum: {
+      getAddress: () => artifacts.ethereumFeeProxyArtifact.getAddress(network),
+      getCurrentProxy: () => contract.ethereumFeeProxy(),
+      setMethod: 'setEthereumFeeProxy',
+    },
+  }[proxyType];
+
+  const proxyAddress = config.getAddress();
+  const currentProxy = await config.getCurrentProxy();
+
+  if (ethers.utils.getAddress(currentProxy) !== ethers.utils.getAddress(proxyAddress)) {
+    await executeContractMethod({
+      network,
+      contract,
+      method: config.setMethod,
+      props: [proxyAddress],
+      txOverrides,
+      signer,
+      signWithEoa,
+    });
+    console.log(`Updated ${proxyType}FeeProxy to ${proxyAddress} on ${network}`);
+  } else {
+    console.log(`${proxyType}FeeProxy is already set to ${proxyAddress} on ${network}`);
+  }
+};
+
+export const updateSRPFERC20FeeProxyAddress = (
+  contract: Contract,
+  network: CurrencyTypes.EvmChainName,
+  txOverrides: Overrides,
+  signer: Wallet,
+  signWithEoa: boolean,
+): Promise<void> =>
+  updateSRPFFeeProxyAddress('ERC20', contract, network, txOverrides, signer, signWithEoa);
+
+export const updateSRPFEthereumFeeProxyAddress = (
+  contract: Contract,
+  network: CurrencyTypes.EvmChainName,
+  txOverrides: Overrides,
+  signer: Wallet,
+  signWithEoa: boolean,
+): Promise<void> =>
+  updateSRPFFeeProxyAddress('Ethereum', contract, network, txOverrides, signer, signWithEoa);

This refactoring:

  1. Reduces code duplication
  2. Makes it easier to maintain and update both proxy types
  3. Keeps the same public API
  4. Improves type safety with the ProxyType type
📝 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.

Suggested change
export const updateSRPFERC20FeeProxyAddress = async (
contract: Contract,
network: CurrencyTypes.EvmChainName,
txOverrides: Overrides,
signer: Wallet,
signWithEoa: boolean,
): Promise<void> => {
const erc20ProxyAddress = artifacts.erc20FeeProxyArtifact.getAddress(network);
const currentErc20Proxy = await contract.erc20FeeProxy();
if (ethers.utils.getAddress(currentErc20Proxy) !== ethers.utils.getAddress(erc20ProxyAddress)) {
await executeContractMethod({
network,
contract,
method: 'setERC20FeeProxy',
props: [erc20ProxyAddress],
txOverrides,
signer,
signWithEoa,
});
console.log(`Updated ERC20FeeProxy to ${erc20ProxyAddress} on ${network}`);
} else {
console.log(`ERC20FeeProxy is already set to ${erc20ProxyAddress} on ${network}`);
}
};
/**
* Updates the Ethereum fee proxy address in the SingleRequestProxyFactory contract
* @param contract SingleRequestProxyFactory contract
* @param network The network used
* @param txOverrides information related to gas fees
* @param signer Who is performing the updating
* @param signWithEoa Is the transaction to be signed by an EOA
*/
export const updateSRPFEthereumFeeProxyAddress = async (
contract: Contract,
network: CurrencyTypes.EvmChainName,
txOverrides: Overrides,
signer: Wallet,
signWithEoa: boolean,
): Promise<void> => {
const ethereumProxyAddress = artifacts.ethereumFeeProxyArtifact.getAddress(network);
const currentEthereumProxy = await contract.ethereumFeeProxy();
if (
ethers.utils.getAddress(currentEthereumProxy) !== ethers.utils.getAddress(ethereumProxyAddress)
) {
await executeContractMethod({
network,
contract,
method: 'setEthereumFeeProxy',
props: [ethereumProxyAddress],
txOverrides,
signer,
signWithEoa,
});
console.log(`Updated EthereumFeeProxy to ${ethereumProxyAddress} on ${network}`);
} else {
console.log(`EthereumFeeProxy is already set to ${ethereumProxyAddress} on ${network}`);
}
};
type ProxyType = 'ERC20' | 'Ethereum';
const updateSRPFFeeProxyAddress = async (
proxyType: ProxyType,
contract: Contract,
network: CurrencyTypes.EvmChainName,
txOverrides: Overrides,
signer: Wallet,
signWithEoa: boolean,
): Promise<void> => {
const config = {
ERC20: {
getAddress: () => artifacts.erc20FeeProxyArtifact.getAddress(network),
getCurrentProxy: () => contract.erc20FeeProxy(),
setMethod: 'setERC20FeeProxy',
},
Ethereum: {
getAddress: () => artifacts.ethereumFeeProxyArtifact.getAddress(network),
getCurrentProxy: () => contract.ethereumFeeProxy(),
setMethod: 'setEthereumFeeProxy',
},
}[proxyType];
const proxyAddress = config.getAddress();
const currentProxy = await config.getCurrentProxy();
if (ethers.utils.getAddress(currentProxy) !== ethers.utils.getAddress(proxyAddress)) {
await executeContractMethod({
network,
contract,
method: config.setMethod,
props: [proxyAddress],
txOverrides,
signer,
signWithEoa,
});
console.log(`Updated ${proxyType}FeeProxy to ${proxyAddress} on ${network}`);
} else {
console.log(`${proxyType}FeeProxy is already set to ${proxyAddress} on ${network}`);
}
};
export const updateSRPFERC20FeeProxyAddress = (
contract: Contract,
network: CurrencyTypes.EvmChainName,
txOverrides: Overrides,
signer: Wallet,
signWithEoa: boolean,
): Promise<void> =>
updateSRPFFeeProxyAddress('ERC20', contract, network, txOverrides, signer, signWithEoa);
export const updateSRPFEthereumFeeProxyAddress = (
contract: Contract,
network: CurrencyTypes.EvmChainName,
txOverrides: Overrides,
signer: Wallet,
signWithEoa: boolean,
): Promise<void> =>
updateSRPFFeeProxyAddress('Ethereum', contract, network, txOverrides, signer, signWithEoa);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Admins Scripts for the SingleRequestProxyFactory to the SDK
3 participants