feat: enhance cancel method to support allowance revocation for recurring payments #1657
Conversation
This file contains hidden or 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
Contributor
WalkthroughAdds runtime dependency Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant RequestAPI as Request.cancel()
participant PaymentProcessor as getRecurringPaymentProxyAddress()
participant ERC20Factory as ERC20__factory
Caller->>RequestAPI: cancel(signerIdentity, refundInformation?, options?)
alt options.isRecurringPayment && options.isPayerCancel && recurringPaymentInfo present
RequestAPI->>PaymentProcessor: getRecurringPaymentProxyAddress(network)
PaymentProcessor-->>RequestAPI: proxyAddress
RequestAPI->>ERC20Factory: createInterface().encodeFunctionData("approve", [proxyAddress, 0])
ERC20Factory-->>RequestAPI: allowanceRevocationCalldata (0x...)
RequestAPI->>Caller: IRequestDataWithEvents + allowanceRevocationCalldata
else
RequestAPI->>Caller: IRequestDataWithEvents (allowanceRevocationCalldata = undefined)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
|
Hello @mayur1377, thank you for submitting your first pull request to the requestNetwork repository. We value your contribution and encourage you to review our contribution guidelines to ensure your submission meets our standards. Please note that every merged PR is automatically enrolled in our Best PR Initiative, offering a chance to win $500 each quarter. Our team is available via GitHub Discussions or Discord if you have any questions. Welcome aboard! |
Member
|
Thank you for your submission! As you prepare for the review process, please ensure that your PR title, description, and any linked issues fully comply with our contribution guidelines. A clear explanation of your changes and their context will help expedite the review process. Every merged PR is automatically entered into our Best PR Initiative, offering a chance to win $500 every quarter. We appreciate your attention to detail and look forward to reviewing your contribution! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/request-client.js/test/api/request.test.ts (1)
223-272: Ensure cancel still returns the enriched request objectPlease add an expectation that the cancel result still exposes the
IRequestDataWithEventsAPI (e.g.,expect(typeof result.on).toBe('function')). That would have caught regressions where we accidentally return a plain object without the event emitter. This complements the new calldata checks without changing behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/request-client.js/package.json(1 hunks)packages/request-client.js/src/api/request.ts(3 hunks)packages/request-client.js/test/api/request.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (21)
📚 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/api/request.test.tspackages/request-client.js/src/api/request.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/api/request.test.tspackages/request-client.js/src/api/request.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/api/request.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/api/request.test.tspackages/request-client.js/src/api/request.ts
📚 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/api/request.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/api/request.test.tspackages/request-client.js/package.json
📚 Learning: 2024-11-12T17:48:47.072Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:211-219
Timestamp: 2024-11-12T17:48:47.072Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, the error handling logic in `payWithEthereumSingleRequestForwarder` is correct and does not require changes to differentiate between ERC20 and Ethereum SingleRequestForwarder contracts.
Applied to files:
packages/request-client.js/test/api/request.test.tspackages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-12T16:54:02.702Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:104-104
Timestamp: 2024-11-12T16:54:02.702Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, when the smart contract has not changed, event argument names such as `proxyAddress` remain the same, even if variable names in the code are updated to use new terminology like `forwarderAddress`.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.tspackages/request-client.js/package.json
📚 Learning: 2024-10-28T16:10:09.506Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/request-client.js/src/api/request-network.ts:542-554
Timestamp: 2024-10-28T16:10:09.506Z
Learning: In `packages/request-client.js/src/api/request-network.ts`, within the `prepareRequestDataForPayment` function, it is acceptable to use parameter spreading (`...requestData.parameters`) to include all fields from `requestData.parameters` in the returned object.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.ts
📚 Learning: 2024-11-08T20:01:10.313Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/src/payment/single-request-proxy.ts:0-0
Timestamp: 2024-11-08T20:01:10.313Z
Learning: In `packages/payment-processor/src/payment/single-request-proxy.ts`, when decoding event data, the team prefers not to include documentation of the event data structure if maintaining it would be difficult.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.ts
📚 Learning: 2024-11-08T20:01:10.313Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/src/payment/single-request-proxy.ts:0-0
Timestamp: 2024-11-08T20:01:10.313Z
Learning: In `packages/payment-processor/src/payment/single-request-proxy.ts`, the team prefers not to store or validate decoded values that are not needed.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-12T16:52:41.557Z
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.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-05T16:57:30.406Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/payment-processor/overrides/jest/index.d.ts:0-0
Timestamp: 2024-11-05T16:57:30.406Z
Learning: The `index.d.ts` file at `packages/payment-processor/overrides/jest/index.d.ts` is copied from `node_modules/types/jest` and should not be modified manually.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-04T12:18:18.615Z
Learnt from: giorgi-kiknavelidze
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/usage-examples/package.json:42-42
Timestamp: 2024-11-04T12:18:18.615Z
Learning: In the RequestNetwork project, the `dotenv` package version is maintained at `10.0.0` across packages, including `packages/smart-contracts/package.json`, to ensure consistency.
Applied to files:
packages/request-client.js/package.json
📚 Learning: 2024-11-01T18:44:46.597Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/integration-test/package.json:60-60
Timestamp: 2024-11-01T18:44:46.597Z
Learning: In the RequestNetwork project, it's unnecessary to include dependency version information for ethers in the `README.md` of `packages/integration-test`, as the SDK user's package manager handles this.
Applied to files:
packages/request-client.js/package.json
📚 Learning: 2024-11-04T12:18:12.407Z
Learnt from: giorgi-kiknavelidze
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/package.json:42-42
Timestamp: 2024-11-04T12:18:12.407Z
Learning: In the RequestNetwork project, dependencies in `package.json` files are pinned to exact versions to prevent unexpected breakages with examples. Therefore, caret version ranges (e.g., `^0.0.167`) should not be suggested.
Applied to files:
packages/request-client.js/package.json
📚 Learning: 2024-11-18T12:33:47.986Z
Learnt from: rodrigopavezi
Repo: RequestNetwork/requestNetwork PR: 1475
File: packages/epk-cypher/package.json:49-67
Timestamp: 2024-11-18T12:33:47.986Z
Learning: In the `packages/epk-cypher/package.json` file, avoid suggesting updates to development dependencies unless essential, as the maintainer prefers to prevent potential instability.
Applied to files:
packages/request-client.js/package.json
📚 Learning: 2024-07-17T13:50:20.447Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1386
File: packages/request-client.js/src/api/request-network.ts:74-145
Timestamp: 2024-07-17T13:50:20.447Z
Learning: The transaction manager in the Request Network SDK formats the data ensuring its integrity before it reaches methods like `preparePaymentRequest`.
Applied to files:
packages/request-client.js/package.json
🧬 Code graph analysis (2)
packages/request-client.js/test/api/request.test.ts (2)
packages/request-client.js/src/api/request.ts (1)
Request(30-846)packages/payment-processor/test/payment/shared.ts (1)
currencyManager(4-21)
packages/request-client.js/src/api/request.ts (4)
packages/types/src/identity-types.ts (1)
IIdentity(2-7)packages/types/src/currency-types.ts (1)
EvmChainName(6-35)packages/types/src/client-types.ts (1)
IRequestDataWithEvents(22-28)packages/payment-processor/src/payment/erc20-recurring-payment-proxy.ts (1)
getRecurringPaymentProxyAddress(220-228)
🪛 Gitleaks (8.29.0)
packages/request-client.js/test/api/request.test.ts
[high] 229-229: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 245-245: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 266-266: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (1)
packages/request-client.js/package.json (1)
52-52: Dependency pin aligns with project policyThanks for keeping
@requestnetwork/payment-processorpinned to0.57.0; that's consistent with the Request Network practice of exact versions. Based on learnings
mayur1377
changed the title
…ring payments - Updated the cancel method to accept options for recurring payment cancellations. - Improved flexibility for payer/payee allowance revocation during cancellation.
mayur1377
force-pushed
the
feature/recurring-payments-two-way-cancel
branch
from
7caf556 to
fd2a103
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/request-client.js/test/api/request.test.ts (1)
223-272: Add regression for missing recurring payment metadataOnce the cancellation path enforces
recurringPaymentInfo, please add a test that asserts we throw ifisRecurringPaymentandisPayerCancelare true butrecurringPaymentInfois omitted, so future refactors can’t silently drop the calldata contract.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/request-client.js/package.json(1 hunks)packages/request-client.js/src/api/request.ts(3 hunks)packages/request-client.js/test/api/request.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/request-client.js/package.json
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
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.
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.
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.
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`.
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.
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.
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.
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:211-219
Timestamp: 2024-11-12T17:48:47.072Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, the error handling logic in `payWithEthereumSingleRequestForwarder` is correct and does not require changes to differentiate between ERC20 and Ethereum SingleRequestForwarder contracts.
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.
📚 Learning: 2024-11-12T16:54:02.702Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:104-104
Timestamp: 2024-11-12T16:54:02.702Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, when the smart contract has not changed, event argument names such as `proxyAddress` remain the same, even if variable names in the code are updated to use new terminology like `forwarderAddress`.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.ts
📚 Learning: 2024-11-12T17:48:47.072Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:211-219
Timestamp: 2024-11-12T17:48:47.072Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, the error handling logic in `payWithEthereumSingleRequestForwarder` is correct and does not require changes to differentiate between ERC20 and Ethereum SingleRequestForwarder contracts.
Applied to files:
packages/request-client.js/src/api/request.tspackages/request-client.js/test/api/request.test.ts
📚 Learning: 2024-10-28T16:10:09.506Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/request-client.js/src/api/request-network.ts:542-554
Timestamp: 2024-10-28T16:10:09.506Z
Learning: In `packages/request-client.js/src/api/request-network.ts`, within the `prepareRequestDataForPayment` function, it is acceptable to use parameter spreading (`...requestData.parameters`) to include all fields from `requestData.parameters` in the returned object.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-08T20:01:10.313Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/src/payment/single-request-proxy.ts:0-0
Timestamp: 2024-11-08T20:01:10.313Z
Learning: In `packages/payment-processor/src/payment/single-request-proxy.ts`, when decoding event data, the team prefers not to include documentation of the event data structure if maintaining it would be difficult.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.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/src/api/request.tspackages/request-client.js/test/api/request.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/src/api/request.tspackages/request-client.js/test/api/request.test.ts
📚 Learning: 2024-11-04T14:31:29.664Z
Learnt from: giorgi-kiknavelidze
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/src/payment/prepared-transaction.ts:15-16
Timestamp: 2024-11-04T14:31:29.664Z
Learning: In `packages/payment-processor/src/payment/prepared-transaction.ts`, for the `IPreparedPrivateTransaction` interface, use `string[]` for the `ops` field since Hinkal's relevant functions accept arrays of `string` as inputs.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-10-28T21:11:48.524Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:123-184
Timestamp: 2024-10-28T21:11:48.524Z
Learning: When suggesting error handling improvements in `packages/payment-processor/src/payment/single-request-proxy.ts`, avoid recommending try-catch blocks that re-throw errors without additional processing.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.tspackages/request-client.js/test/api/request.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/src/api/request.ts
📚 Learning: 2024-11-08T20:01:10.313Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/src/payment/single-request-proxy.ts:0-0
Timestamp: 2024-11-08T20:01:10.313Z
Learning: In `packages/payment-processor/src/payment/single-request-proxy.ts`, the team prefers not to store or validate decoded values that are not needed.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-12T16:52:41.557Z
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.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-05T16:57:30.406Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/payment-processor/overrides/jest/index.d.ts:0-0
Timestamp: 2024-11-05T16:57:30.406Z
Learning: The `index.d.ts` file at `packages/payment-processor/overrides/jest/index.d.ts` is copied from `node_modules/types/jest` and should not be modified manually.
Applied to files:
packages/request-client.js/src/api/request.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/api/request.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/api/request.test.ts
📚 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/api/request.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/api/request.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/api/request.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/api/request.test.ts
🧬 Code graph analysis (2)
packages/request-client.js/src/api/request.ts (4)
packages/types/src/identity-types.ts (1)
IIdentity(2-7)packages/types/src/currency-types.ts (1)
EvmChainName(6-35)packages/types/src/client-types.ts (1)
IRequestDataWithEvents(22-28)packages/payment-processor/src/payment/erc20-recurring-payment-proxy.ts (1)
getRecurringPaymentProxyAddress(220-228)
packages/request-client.js/test/api/request.test.ts (2)
packages/request-client.js/src/api/request.ts (1)
Request(30-846)packages/payment-processor/test/payment/shared.ts (1)
currencyManager(4-21)
🪛 Gitleaks (8.29.0)
packages/request-client.js/test/api/request.test.ts
[high] 229-229: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 245-245: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 266-266: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/request-client.js/src/api/request.ts (1)
209-226: Update JSDoc to document the new return field.The return type now includes an optional
allowanceRevocationCalldatafield, but the@returnsJSDoc still only states "The updated request". Consider documenting what this calldata represents and how callers should use it.Apply this diff to improve the documentation:
+ * @param options.recurringPaymentInfo.tokenAddress ERC20 token contract address (calldata should be sent to this address) + * @param options.recurringPaymentInfo.network Network where the recurring payment proxy is deployed - * @returns The updated request and optional calldata for allowance revocation (if payer cancels recurring payment) + * @returns The updated request data. If payer cancels a recurring payment, includes `allowanceRevocationCalldata` + * (encoded ERC20 approve(proxy, 0) call) that should be sent to the token contract to revoke the allowance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/request-client.js/src/api/request.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
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.
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.
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.
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.
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.
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.
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`.
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:211-219
Timestamp: 2024-11-12T17:48:47.072Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, the error handling logic in `payWithEthereumSingleRequestForwarder` is correct and does not require changes to differentiate between ERC20 and Ethereum SingleRequestForwarder contracts.
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: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:123-184
Timestamp: 2024-10-28T21:11:48.524Z
Learning: When suggesting error handling improvements in `packages/payment-processor/src/payment/single-request-proxy.ts`, avoid recommending try-catch blocks that re-throw errors without additional processing.
📚 Learning: 2024-11-12T16:54:02.702Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:104-104
Timestamp: 2024-11-12T16:54:02.702Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, when the smart contract has not changed, event argument names such as `proxyAddress` remain the same, even if variable names in the code are updated to use new terminology like `forwarderAddress`.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-12T17:48:47.072Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:211-219
Timestamp: 2024-11-12T17:48:47.072Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, the error handling logic in `payWithEthereumSingleRequestForwarder` is correct and does not require changes to differentiate between ERC20 and Ethereum SingleRequestForwarder contracts.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.ts
📚 Learning: 2024-11-08T20:01:10.313Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/src/payment/single-request-proxy.ts:0-0
Timestamp: 2024-11-08T20:01:10.313Z
Learning: In `packages/payment-processor/src/payment/single-request-proxy.ts`, when decoding event data, the team prefers not to include documentation of the event data structure if maintaining it would be difficult.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-10-28T16:10:09.506Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/request-client.js/src/api/request-network.ts:542-554
Timestamp: 2024-10-28T16:10:09.506Z
Learning: In `packages/request-client.js/src/api/request-network.ts`, within the `prepareRequestDataForPayment` function, it is acceptable to use parameter spreading (`...requestData.parameters`) to include all fields from `requestData.parameters` in the returned object.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.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/src/api/request.ts
📚 Learning: 2024-10-28T21:11:48.524Z
Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:123-184
Timestamp: 2024-10-28T21:11:48.524Z
Learning: When suggesting error handling improvements in `packages/payment-processor/src/payment/single-request-proxy.ts`, avoid recommending try-catch blocks that re-throw errors without additional processing.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-04T14:32:57.040Z
Learnt from: giorgi-kiknavelidze
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/src/payment/erc20-fee-proxy.ts:186-186
Timestamp: 2024-11-04T14:32:57.040Z
Learning: In the `preparePrivateErc20FeeProxyPaymentTransaction` function in `packages/payment-processor/src/payment/erc20-fee-proxy.ts`, we should not make an allowance call on the token contract, as it is not relevant to our use case.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.ts
📚 Learning: 2024-11-04T20:53:41.522Z
Learnt from: giorgi-kiknavelidze
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/src/payment/prepare-hinkal.ts:5-13
Timestamp: 2024-11-04T20:53:41.522Z
Learning: When reviewing the RequestNetwork codebase, avoid suggesting the addition of example usage in documentation if it would make the comments overly verbose, especially for functions like `prepareHinkal` in `packages/payment-processor/src/payment/prepare-hinkal.ts`.
Applied to files:
packages/request-client.js/src/api/request.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/src/api/request.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/src/api/request.ts
📚 Learning: 2024-11-08T20:01:10.313Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/src/payment/single-request-proxy.ts:0-0
Timestamp: 2024-11-08T20:01:10.313Z
Learning: In `packages/payment-processor/src/payment/single-request-proxy.ts`, the team prefers not to store or validate decoded values that are not needed.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-12T16:52:41.557Z
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.
Applied to files:
packages/request-client.js/src/api/request.ts
📚 Learning: 2024-11-05T16:57:30.406Z
Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/payment-processor/overrides/jest/index.d.ts:0-0
Timestamp: 2024-11-05T16:57:30.406Z
Learning: The `index.d.ts` file at `packages/payment-processor/overrides/jest/index.d.ts` is copied from `node_modules/types/jest` and should not be modified manually.
Applied to files:
packages/request-client.js/src/api/request.ts
🧬 Code graph analysis (1)
packages/request-client.js/src/api/request.ts (4)
packages/types/src/identity-types.ts (1)
IIdentity(2-7)packages/types/src/currency-types.ts (1)
EvmChainName(6-35)packages/types/src/client-types.ts (1)
IRequestDataWithEvents(22-28)packages/payment-processor/src/payment/erc20-recurring-payment-proxy.ts (1)
getRecurringPaymentProxyAddress(220-228)
🔇 Additional comments (3)
packages/request-client.js/src/api/request.ts (3)
14-16: LGTM! Necessary imports for recurring payment support.The three new imports are properly used in the recurring payment cancellation logic below.
243-259: Excellent fixes! Implementation is now correct.Both previous issues have been properly addressed:
- Line 243 now correctly awaits the Promise returned by
handleRequestDataEvents- Lines 248-252 validate
recurringPaymentInfoand fail fast with a clear error message when missingThe allowance revocation logic is correctly implemented:
- Retrieves the proxy address from the network configuration
- Encodes a standard ERC20
approve(proxyAddress, 0)call to revoke allowance- The
tokenAddressfromrecurringPaymentInfois intentionally not used in encoding (the caller needs it to determine which token contract to send the transaction to)
261-264: LGTM! Clean conditional return pattern.The spread operators correctly merge the request data with the optional allowance revocation calldata, only including the calldata field when it's defined.
Member
|
Thank you @mayur1377 for this PR submission 🙏 I reviewed this PR and decided to close it, because issue RequestNetwork/request-api#346 (formerly RequestNetwork/requestNetwork#1640) was supposed to be on the Request Network API repository, not on this Request Network SDK repository - that's my mistake 🙇. It has now been transferred to the private Request API repo, thus hiding it from public view. The Request Network SDK (NPM Packages like My apologies for indirectly causing you to waste time implementing an unnecessary fix. 🙇 |
Author
welp, that's a bummer — but no worries at all! thanks for clarifying 🙌 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
fix for : https://github.com/RequestNetwork/request-api/issues/346
allows both payer and payee to cancel recurring payments. when the payer cancels, the method returns calldata to revoke their erc20 allowance. when the payee cancels, no allowance calldata is returned , also added a tests for it.
Summary by CodeRabbit
New Features
Tests
Dependencies