feat(processor): any-declarative payment in NEAR #1428
Conversation
WalkthroughThe updates to the Payment Processor package introduce enhanced validation for payment references and add support for NEAR network payments. Key changes include the addition of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PaymentProcessor
participant NearNetwork
participant ValidationUtil
User->>PaymentProcessor: Initiate Payment
PaymentProcessor->>ValidationUtil: validatePaymentReference(paymentReference)
ValidationUtil-->>PaymentProcessor: Validation Result
PaymentProcessor->>NearNetwork: Process NEAR Payment
NearNetwork-->>PaymentProcessor: Payment Confirmation
PaymentProcessor-->>User: Confirm Payment
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (8)
- packages/payment-processor/src/payment/any-to-erc20-proxy.ts (2 hunks)
- packages/payment-processor/src/payment/near-any-declarative.ts (1 hunks)
- packages/payment-processor/src/payment/near-conversion.ts (2 hunks)
- packages/payment-processor/src/payment/near-fungible.ts (2 hunks)
- packages/payment-processor/src/payment/near-input-data.ts (2 hunks)
- packages/payment-processor/src/payment/utils.ts (1 hunks)
- packages/payment-processor/src/utils/validation.ts (1 hunks)
- packages/payment-processor/test/payment/any-to-near.test.ts (1 hunks)
Additional comments not posted (12)
packages/payment-processor/src/utils/validation.ts (1)
1-8: LGTM!The
validatePaymentReferencefunction is well-implemented and ensures that a payment reference is present before proceeding with payment processing.packages/payment-processor/src/payment/near-any-declarative.ts (1)
1-39: LGTM!The
payNearAnyDeclarativeRequestfunction is well-structured and ensures proper validation and processing of NEAR token transactions. The use ofvalidatePaymentReferenceandassertChainSupportedenhances the robustness of the function.packages/payment-processor/src/payment/near-input-data.ts (1)
Line range hint
1-38:
LGTM!The
payNearInputDataRequestfunction includes proper validation steps and the use ofvalidatePaymentReferenceimproves the robustness of the function. The changes are consistent with the overall structure.packages/payment-processor/src/payment/near-fungible.ts (1)
Line range hint
1-41:
LGTM!The
payNearFungibleRequestfunction includes proper validation steps and the use ofvalidatePaymentReferenceimproves the robustness of the function. The changes are consistent with the overall structure.packages/payment-processor/src/payment/near-conversion.ts (1)
15-15: LGTM!The addition of
validatePaymentReferenceensures that the payment reference is validated before proceeding.packages/payment-processor/test/payment/any-to-near.test.ts (4)
Line range hint
49-91:
LGTM!The test case correctly verifies the functionality of
payNearConversionRequestand ensures that the correct parameters are passed toprocessNearPaymentWithConversion.
Line range hint
92-117:
LGTM!The test case correctly verifies that
payNearConversionRequestthrows an error when trying to pay another payment extension.
Line range hint
118-144:
LGTM!The test case correctly verifies that
payNearConversionRequestthrows an error when trying to pay with an unsupported currency.
Line range hint
145-173:
LGTM!The test case correctly verifies that
payNearConversionRequestthrows an error when the network is not near and the error message has been updated to reflect the new message.packages/payment-processor/src/payment/any-to-erc20-proxy.ts (2)
20-20: LGTM!The addition of
validatePaymentReferenceensures that the payment reference is validated before proceeding.
157-157: LGTM!The addition of
validatePaymentReferenceensures that the payment reference is validated before proceeding.packages/payment-processor/src/payment/utils.ts (1)
356-368: LGTM!The function now correctly prioritizes a user-specified amount over the default calculation based on the request balance.
| * @param walletConnection the Near provider. | ||
| * @param amount optionally, the amount to pay in request currency. Defaults to remaining amount of the request. |
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure mockedNearWalletConnection conforms to NearProvider type
The mockedNearWalletConnection in the test file packages/payment-processor/test/payment/any-to-near.test.ts is currently cast to any, which bypasses type checking. To ensure type safety and correctness, it should be explicitly typed to conform to the NearProvider type.
- Update the definition of
mockedNearWalletConnectionto explicitly use theNearProvidertype.
Analysis chain
Verify the usage of NearProvider type for walletConnection.
Ensure that all instances where payNearConversionRequest is called have been updated to use NearProvider for the walletConnection parameter.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all function calls to `payNearConversionRequest` use `NearProvider` for the `walletConnection` parameter.
# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type typescript -A 5 $'payNearConversionRequest'
Length of output: 87
Script:
#!/bin/bash
# Description: Verify all function calls to `payNearConversionRequest` use `NearProvider` for the `walletConnection` parameter.
# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type ts -A 5 $'payNearConversionRequest'
Length of output: 3780
Script:
#!/bin/bash
# Description: Verify the type of `mockedNearWalletConnection` used in `payNearConversionRequest` calls.
# Test: Search for the definition or initialization of `mockedNearWalletConnection`.
rg 'mockedNearWalletConnection' packages/payment-processor/test/payment/any-to-near.test.ts
Length of output: 577
Script:
#!/bin/bash
# Description: Verify the type of `mockedNearWalletConnection` used in `payNearConversionRequest` calls by inspecting its full definition.
# Test: Search for the complete definition of `mockedNearWalletConnection` and capture context lines.
rg 'const mockedNearWalletConnection' -A 10 packages/payment-processor/test/payment/any-to-near.test.ts
Length of output: 1468
There was a problem hiding this comment.
This seems to be a correct observation - the typing in the test could use NearProvider instead of any 👍
There was a problem hiding this comment.
@MantisClone, great! I'm glad you agree with the observation. I'll make a note of this preference for future reviews.
Learnings added
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1428
File: packages/payment-processor/src/payment/near-conversion.ts:20-21
Timestamp: 2024-07-15T14:13:22.837Z
Learning: Ensure that `mockedNearWalletConnection` in test files uses the `NearProvider` type instead of `any`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
I don't understand how a more precise type for a mocked value in the tests positively impacts the library. Can you explain? I'll merge this one because of emergencies and the feedback is not related to the core of my change, but happy to later improve.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- packages/payment-processor/src/payment/near-amount-with-ref.ts (1 hunks)
Additional comments not posted (3)
packages/payment-processor/src/payment/near-amount-with-ref.ts (3)
1-8: Imports look good.The imported modules and functions are appropriate and necessary for the functionality.
21-28: Function signature looks good.The function signature is appropriate for the described functionality.
29-39: Function body looks good.The function body is concise and uses the
processNearPaymentutility correctly.
| /** | ||
| * Processes a transaction to make a payment in NEAR token with a reference. | ||
| * | ||
| * @notice This is used to pay a declarative request, with low-level arguments. | ||
| * | ||
| * @param paymentAddress must be a valid NEAR address on the given network. | ||
| * @param network e.g. 'near' | ||
| * @param paymentReference used to index payments. | ||
| * @param walletConnection the Near provider. | ||
| * @param amount amount to pay, in NEAR tokens. | ||
| */ | ||
| // FIXME: We could improve the method's interface by enforcing a type on `paymentInfo` for declarative requests. |
There was a problem hiding this comment.
Documentation is clear and helpful.
The function documentation provides a clear description of the function's purpose and parameters.
Offer assistance for FIXME comment.
The FIXME comment suggests improving the method's interface by enforcing a type on paymentInfo.
Would you like me to help enforce a type on paymentInfo for declarative requests?
| * @param walletConnection the Near provider. | ||
| * @param amount optionally, the amount to pay in request currency. Defaults to remaining amount of the request. |
There was a problem hiding this comment.
This seems to be a correct observation - the typing in the test could use NearProvider instead of any 👍
Description of the changes
payment-processorutil to make a payment in NEAR (native) token on a declarative request, through low-level parameters:paymentAddress,paymentReferenceandnetwork.Summary by CodeRabbit
New Features
payNearAnyDeclarativeRequest,payNearAmountWithReference.validatePaymentReferencefunction for enhanced payment reference validation.Improvements
validatePaymentReferencefor better validation.getAmountToPayfunction to prioritize user-specified amounts, improving payment flexibility.Bug Fixes
payNearConversionRequesttests for better clarity.