-
Notifications
You must be signed in to change notification settings - Fork 91
refactor: update ERC20CommerceEscrowWrapper deployment script to support network-specific contract addresses #1665
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
base: master
Are you sure you want to change the base?
refactor: update ERC20CommerceEscrowWrapper deployment script to support network-specific contract addresses #1665
Conversation
…ort network-specific contract addresses - Introduced NETWORK_CONTRACTS for managing contract addresses by network. - Updated deployment logic to use existing ERC20FeeProxy if available, or deploy a new one. - Modified AuthCaptureEscrow address retrieval to be network-specific. - Updated Sepolia deployment address for AuthCaptureEscrow and ERC20CommerceEscrowWrapper.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded network-specific contract mappings and validation to the ERC20 deploy script (conditional reuse/deploy of ERC20FeeProxy); updated several on-chain artifact addresses/creation blocks; switched The Graph config/client to use The Graph Studio/Explorer endpoints; and adjusted tests for timers and optional subgraph client injection. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (24)📓 Common learnings📚 Learning: 2024-11-05T16:53:05.280ZApplied to files:
📚 Learning: 2024-10-28T16:03:33.215ZApplied to files:
📚 Learning: 2024-10-17T18:30:55.410ZApplied to files:
📚 Learning: 2024-10-29T08:02:02.600ZApplied to files:
📚 Learning: 2024-11-06T14:48:18.698ZApplied to files:
📚 Learning: 2024-12-18T03:53:54.370ZApplied to files:
📚 Learning: 2024-11-08T18:24:06.144ZApplied to files:
📚 Learning: 2024-12-06T10:32:50.647ZApplied to files:
📚 Learning: 2024-11-21T09:06:12.938ZApplied to files:
📚 Learning: 2024-11-22T13:30:25.703ZApplied to files:
📚 Learning: 2024-11-05T05:33:32.481ZApplied to files:
📚 Learning: 2024-11-22T13:13:26.166ZApplied to files:
📚 Learning: 2024-07-15T14:13:23.019ZApplied to files:
📚 Learning: 2024-11-05T05:04:01.710ZApplied to files:
📚 Learning: 2024-07-17T15:06:14.563ZApplied to files:
📚 Learning: 2024-11-05T16:58:18.471ZApplied to files:
📚 Learning: 2024-10-29T09:00:54.169ZApplied to files:
📚 Learning: 2024-12-09T18:59:04.613ZApplied to files:
📚 Learning: 2024-10-29T08:03:10.463ZApplied to files:
📚 Learning: 2024-12-06T11:27:46.988ZApplied to files:
📚 Learning: 2024-07-17T13:57:39.144ZApplied to files:
📚 Learning: 2024-11-05T05:33:36.189ZApplied to files:
📚 Learning: 2024-12-04T05:01:13.722ZApplied to files:
🧬 Code graph analysis (1)packages/request-client.js/test/index.test.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Comment |
Greptile OverviewGreptile SummaryRefactored the deployment script to support network-specific contract addresses through a Changes:
Issues:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Script as Deployment Script
participant Config as NETWORK_CONTRACTS
participant Network as Network Check
participant ERC20FeeProxy as ERC20FeeProxy
participant AuthCaptureEscrow as AuthCaptureEscrow
participant Wrapper as ERC20CommerceEscrowWrapper
Script->>Network: Get network name
Script->>Config: Lookup network in NETWORK_CONTRACTS
alt Network not configured
Config-->>Script: throw Error
end
Config-->>Script: Return network config
alt ERC20FeeProxy exists in config
Script->>Script: Use existing ERC20FeeProxy address
else ERC20FeeProxy not in config
Script->>ERC20FeeProxy: Deploy new ERC20FeeProxy
ERC20FeeProxy-->>Script: Return deployed address
end
Script->>Config: Get AuthCaptureEscrow address
Config-->>Script: Return network-specific address
Script->>Wrapper: Deploy with (authCaptureEscrowAddress, erc20FeeProxyAddress)
Wrapper-->>Script: Return deployed address
Script->>Script: Log deployment summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, 1 comment
packages/smart-contracts/src/lib/artifacts/AuthCaptureEscrow/index.ts
Outdated
Show resolved
Hide resolved
…ndex.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
- Changed TheGraph API schema URL to the latest version. - Removed unused Alchemy URL and related logic from the client. - Updated test to reflect the new URL structure for TheGraph Studio.
…/github.com/RequestNetwork/requestNetwork into feat/commerce-escrow-eth-sepolia-deployment
…tions - Added jest.useRealTimers() in afterEach to ensure real timers are used in tests. - Updated RequestNetwork instantiation in multiple tests to include paymentOptions with a getSubgraphClient function returning undefined.
- Modified jest.useFakeTimers() to use the advanceTimers option for better control in balance retrieval tests. - Ensured consistency in timer usage across multiple test cases.
…EtherscanProvider methods - Updated tests to utilize jest.spyOn for better readability and maintainability when mocking EtherscanProvider's getHistory and getNetwork methods. - Ensured consistent mocking approach across multiple test cases.
…er management - Added mockServer.resetHandlers() in beforeEach and afterAll to ensure a clean state for tests. - Incorporated jest.useRealTimers() and jest.useFakeTimers() for better control over timer behavior in tests. - Enabled and disabled payment detection in tests to validate balance retrieval functionality.
There was a problem hiding this 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: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/request-client.js/test/index.test.ts(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: - Only comment on issues that would block merging — ignore minor or stylistic concerns.
- Restrict feedback to errors, security risks, or functionality-breaking problems.
- Do not post comments on code style, formatting, or non-critical improvements.
- Keep reviews short: flag only issues that make the PR unsafe to merge.
- Limit review comments to 3–5 items maximum, unless additional blockers exist.
- Group similar issues into a single comment instead of posting multiple notes.
- Skip repetition — if a pattern repeats, mention it once at a summary level only.
- Do not add general suggestions; focus strictly on merge-blocking concerns.
- If there are no critical problems, respond with minimal approval (e.g., 'Looks good'). Do not add additional review.
- Avoid line-by-line commentary unless it highlights a critical bug or security hole.
- Highlight only issues that could cause runtime errors, data loss, or severe maintainability issues.
- Ignore minor optimization opportunities — focus solely on correctness and safety.
- Provide a top-level summary of critical blockers rather than detailed per-line notes.
- Comment only when the issue must be resolved before merge — otherwise, remain silent.
- When in doubt, err on the side of fewer comments — brevity and blocking issues only.
- Avoid posting any refactoring issues
Files:
packages/request-client.js/test/index.test.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1486
File: packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts:1-24
Timestamp: 2024-11-11T16:10:26.692Z
Learning: The team prefers to maintain individual deployment scripts for each network in `packages/smart-contracts/deploy`, rather than consolidating them into a unified script.
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 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: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1633
File: packages/smart-contracts/scripts/test-deploy-erc20-recurring-payment-proxy.ts:16-16
Timestamp: 2025-06-23T09:32:16.214Z
Learning: Test deployment scripts in the RequestNetwork repository (files with `test-deploy-` prefix) use the same deployer address for multiple roles for simplicity in local testing environments. Production deployments use separate addresses configured through environment variables.
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/integration-test/test/scheduled/erc20-proxy.test.ts:0-0
Timestamp: 2024-11-05T16:53:05.280Z
Learning: In `packages/integration-test/test/scheduled/erc20-proxy.test.ts`, when upgrading dependencies like `ethers`, additional error handling test cases for contract interactions and provider errors may not be necessary.
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1478
File: packages/smart-contracts/scripts-create2/contract-setup/update-fee-proxies.ts:29-31
Timestamp: 2024-10-30T17:54:34.513Z
Learning: When logging missing contract deployments in `updateFeeProxies`, prefer using `console.warn` over `console.info` to highlight potential issues.
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:237-246
Timestamp: 2024-11-08T18:24:06.144Z
Learning: In `packages/payment-processor/test/payment/single-request-proxy.test.ts`, when asserting the `feeProxyUsed` in emitted events from `SingleRequestProxyFactory`, retrieve the `erc20FeeProxy` (or `ethereumFeeProxy`) public variable from the `SingleRequestProxyFactory` contract instead of using `wallet.address`.
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:198-206
Timestamp: 2024-11-08T18:24:19.095Z
Learning: In tests for `SingleRequestProxyFactory`, the `feeProxyUsed` in events should be verified by retrieving the `ethereumFeeProxy` public variable from the `SingleRequestProxyFactory` contract, not `wallet.address`.
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts:5-5
Timestamp: 2024-11-12T16:52:41.557Z
Learning: When the smart contracts are not being modified, types like `SingleRequestProxyFactory` in `packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts` should remain unchanged, even if terminology elsewhere in the code is updated.
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/smart-contracts/scripts/test-deploy-single-request-proxy.ts:14-32
Timestamp: 2024-10-28T20:00:25.780Z
Learning: In test files, such as `packages/smart-contracts/scripts/test-deploy-single-request-proxy.ts`, extensive error handling and input validation are considered unnecessary.
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 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.
📚 Learning: 2024-11-05T16:53:05.280Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/integration-test/test/scheduled/erc20-proxy.test.ts:0-0
Timestamp: 2024-11-05T16:53:05.280Z
Learning: In `packages/integration-test/test/scheduled/erc20-proxy.test.ts`, when upgrading dependencies like `ethers`, additional error handling test cases for contract interactions and provider errors may not be necessary.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-10-28T16:03:33.215Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:251-270
Timestamp: 2024-10-28T16:03:33.215Z
Learning: When testing the payment-processor module, specifically in `packages/payment-processor/test/payment/single-request-proxy.test.ts`, it's acceptable to omit tests for partial payments if they have already been covered at the smart-contract level.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-10-17T18:30:55.410Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1453
File: packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts:150-152
Timestamp: 2024-10-17T18:30:55.410Z
Learning: In `packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts`, the skipped test `'should process a partial payment correctly'` exists intentionally to show that partial payments are supported without duplicating previous happy-path tests.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-10-29T08:02:02.600Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:138-139
Timestamp: 2024-10-29T08:02:02.600Z
Learning: When testing invalid requests in `packages/payment-processor/test/payment/single-request-proxy.test.ts`, it's acceptable to use `ts-expect-error` to suppress TypeScript errors when the request intentionally lacks required properties.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-11-06T14:48:18.698Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1484
File: packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts:0-0
Timestamp: 2024-11-06T14:48:18.698Z
Learning: In `packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts`, when the existing happy path tests are deemed sufficient, additional test cases may not be necessary.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-12-18T03:53:54.370Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/test/payment/erc-20-private-payment-hinkal.test.ts:38-38
Timestamp: 2024-12-18T03:53:54.370Z
Learning: In `packages/payment-processor/test/payment/erc-20-private-payment-hinkal.test.ts`, mainnet RPC is intentionally used for real on-chain tests as confirmed by MantisClone.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-11-08T18:24:06.144Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:237-246
Timestamp: 2024-11-08T18:24:06.144Z
Learning: In `packages/payment-processor/test/payment/single-request-proxy.test.ts`, when asserting the `feeProxyUsed` in emitted events from `SingleRequestProxyFactory`, retrieve the `erc20FeeProxy` (or `ethereumFeeProxy`) public variable from the `SingleRequestProxyFactory` contract instead of using `wallet.address`.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-12-06T10:32:50.647Z
Learnt from: rodrigopavezi
Repo: RequestNetwork/requestNetwork PR: 1510
File: .circleci/config.yml:0-0
Timestamp: 2024-12-06T10:32:50.647Z
Learning: It's acceptable to exclude the following packages from unit tests in CI: `requestnetwork/smart-contracts`, `requestnetwork/payment-detection`, `requestnetwork/payment-processor`, and `requestnetwork/integration-test`.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-11-21T09:06:12.938Z
Learnt from: rodrigopavezi
Repo: RequestNetwork/requestNetwork PR: 1475
File: packages/transaction-manager/test/unit/utils/test-data.ts:87-119
Timestamp: 2024-11-21T09:06:12.938Z
Learning: In `packages/transaction-manager/test/unit/utils/test-data.ts`, mocks like `fakeEpkCipherProvider` do not require extensive test coverage for input validation and error handling.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-11-22T13:30:25.703Z
Learnt from: rodrigopavezi
Repo: RequestNetwork/requestNetwork PR: 1475
File: packages/transaction-manager/test/unit/utils/test-data.ts:165-205
Timestamp: 2024-11-22T13:30:25.703Z
Learning: In `packages/transaction-manager/test/unit/utils/test-data.ts`, modifying `storedRawData` in the `FakeLitProtocolProvider` class may break existing functionality, so it should be left as is.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-07-17T15:06:14.563Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1386
File: packages/request-client.js/src/api/request-network.ts:25-25
Timestamp: 2024-07-17T15:06:14.563Z
Learning: The `persistTransaction()` function in the `no-persist-http-data-access.ts` file is tested in the `in-memory-request.test.ts` file, which is separate from the typical `index.test.ts` file to avoid test interference.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-10-29T08:03:10.463Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:225-228
Timestamp: 2024-10-29T08:03:10.463Z
Learning: In 'packages/payment-processor/test/payment/single-request-proxy.test.ts' (TypeScript), when testing with an invalid proxy address, we should not use the zero address.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-10-29T09:00:54.169Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:202-209
Timestamp: 2024-10-29T09:00:54.169Z
Learning: In the `packages/payment-processor/src/payment/single-request-proxy.ts` file, within the `payWithEthereumSingleRequestProxy` function, the current error handling is acceptable as per the project's expectations.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-11-05T16:58:18.471Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/payment-detection/test/provider.test.ts:25-25
Timestamp: 2024-11-05T16:58:18.471Z
Learning: In `provider.test.ts`, when testing `getDefaultProvider`, we use a chain that Infura supports but is not in our own RPC list (such as `maticmum`) to ensure that the function correctly falls back to `InfuraProvider`.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-12-04T05:05:19.610Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/test/payment/erc-20-private-payment-hinkal.test.ts:0-0
Timestamp: 2024-12-04T05:05:19.610Z
Learning: The function `createRequestForHinkal` in `erc-20-private-payment-hinkal.test.ts` is intended for testing purposes only and should remain in the test directory.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-12-06T11:27:46.988Z
Learnt from: rodrigopavezi
Repo: RequestNetwork/requestNetwork PR: 1512
File: packages/integration-test/test/lit-protocol.test.ts:9-31
Timestamp: 2024-12-06T11:27:46.988Z
Learning: In the `waitForConfirmation` function within `packages/integration-test/test/lit-protocol.test.ts`, checking for the request state being `CREATED` or `PENDING` is correct as per design.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-07-17T13:57:39.144Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1386
File: packages/request-client.js/src/api/request-network.ts:74-145
Timestamp: 2024-07-17T13:57:39.144Z
Learning: The data passed to the `preparePaymentRequest` method in the `RequestNetwork` class is reliably formatted by the transaction manager, negating the need for additional error handling for data formatting within this method.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-12-09T18:59:04.613Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/src/payment/erc-20-private-payment-hinkal.ts:80-99
Timestamp: 2024-12-09T18:59:04.613Z
Learning: In the `RequestNetwork` codebase, payment processor functions such as `payErc20ProxyRequestFromHinkalShieldedAddress` in `packages/payment-processor/src/payment/erc-20-private-payment-hinkal.ts` should not include explicit request validation or try/catch blocks, and should rely on the underlying components (like `hinkalObject`) to handle error reporting.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-11-05T05:33:36.189Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 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.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-07-15T14:13:23.019Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1428
File: packages/payment-processor/src/payment/near-conversion.ts:20-21
Timestamp: 2024-07-15T14:13:23.019Z
Learning: Ensure that `mockedNearWalletConnection` in test files uses the `NearProvider` type instead of `any`.
Applied to files:
packages/request-client.js/test/index.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
- Added jest.restoreAllMocks() in afterEach to guarantee that all mocks are reset after each test, improving test isolation and reliability.
- Removed the advanceTimers option from jest.useFakeTimers() to streamline timer behavior in balance retrieval tests. - Ensured consistent timer usage across multiple test cases for improved test clarity.
…ests - Updated tests to use setTimeout instead of jest.advanceTimersByTime for better clarity and reliability in timing behavior. - Removed jest.useFakeTimers() calls to streamline timer management across multiple test cases.
Description of the changes
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.