-
Notifications
You must be signed in to change notification settings - Fork 83
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: support .tg extension for NEAR #1484
Conversation
WalkthroughThe changes involve an update to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CurrencyUtils
User->>CurrencyUtils: Validate Address
CurrencyUtils->>CurrencyUtils: Check format against regex
alt Valid Address
CurrencyUtils-->>User: Address is valid
else Invalid Address
CurrencyUtils-->>User: Address is invalid
end
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 2
🧹 Outside diff range comments (1)
packages/currency/src/currency-utils.ts (1)
Line range hint
3-6
: Update function documentation to include .tg extension support.The function documentation should be updated to explicitly mention support for both
.near
and.tg
extensions when using the 'near' network./** * Checks if a Near address is valid according to a currency network. + * For 'near' network, addresses must end with either .near or .tg * Returns true if the currency network is not given and the address is correct for any network. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
packages/currency/src/currency-utils.ts
(1 hunks)
🔇 Additional comments (1)
packages/currency/src/currency-utils.ts (1)
30-30
: LGTM! Regex pattern correctly implements .tg support.
The updated regular expression properly validates addresses ending with either .near
or .tg
while maintaining the existing validation rules.
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
packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts
Outdated
Show resolved
Hide resolved
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.
LGTM!
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: 4
🧹 Outside diff range comments (1)
packages/currency/src/currency-utils.ts (1)
Line range hint
1-38
: Update documentation to reflect .tg extension support.Since this is a protocol-level change adding support for
.tg
extension:
- Update the function's JSDoc to mention both
.near
and.tg
support- Consider adding a link to documentation about
.tg
addresses in the commentsApply this diff to enhance the documentation:
/** * Checks if a Near address is valid according to a currency network. * Returns true if the currency network is not given and the address is correct for any network. + * + * Supported formats for mainnet: + * - <account>.near + * - <account>.tg + * + * Supported formats for testnet: + * - <account>.testnet */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts
(2 hunks)packages/advanced-logic/test/utils/payment-network/any/generator-data-create.ts
(1 hunks)packages/currency/src/currency-utils.ts
(1 hunks)packages/currency/test/address-validation/near-validation.test.ts
(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts (1)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#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.
🔇 Additional comments (4)
packages/currency/test/address-validation/near-validation.test.ts (2)
4-4
: LGTM! Constant follows established naming convention.
The new constant validNearTgAccount
is well-named and follows the existing pattern for test data.
Line range hint 1-24
: Verify NEAR address format requirements.
Let's confirm that the test cases align with NEAR's official address format requirements.
✅ Verification successful
Let me search for the actual NEAR address validation implementation to understand the rules being enforced.
Let me try to find the actual implementation file since we only found test files. Let's search in the src directory.
Test cases correctly align with NEAR's address format requirements
The test cases in near-validation.test.ts
accurately reflect NEAR's address format requirements as implemented in isValidNearAddress()
:
- Hex format (64 characters):
f336b7833496cdcae230463c3daff7b2fe187a93be8df5b1326ce7a595033163
- Account format with valid TLDs (.near, .tg):
testaccount.near
,testaccount.tg
- Invalid cases:
- Bad TLD:
testaccount.badnear
- Invalid format with hex:
f336b7833496cdcae230463c3daff7b2fe187a93be8df5b1326ce7a595033163.badnear
- Bad TLD:
The implementation follows NEAR's specifications from:
- Account structure: https://nomicon.io/DataStructures/Account.html
- Top-level accounts documentation: https://docs.near.org/docs/videos/accounts-keys
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for NEAR address format documentation or validation rules in the codebase
rg -i "near.*address.*format|near.*account.*format" --type ts
Length of output: 188
Script:
#!/bin/bash
# Find the implementation of isValidNearAddress function
ast-grep --pattern 'function isValidNearAddress($_) {
$$$
}'
# Also search for any NEAR-related constants or regex patterns
rg -i "near.*pattern|near.*regex|near.*validation" --type ts
Length of output: 1547
Script:
#!/bin/bash
# Find the implementation file
fd "currency-utils.ts" --type f
# Once found, let's see its content
cat packages/currency/src/currency-utils.ts
Length of output: 2314
packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts (2)
8-8
: LGTM! Import added for .tg extension test data.
The new import provides the necessary test data constant for validating .tg
extension support.
312-327
: LGTM! Test case correctly validates .tg extension support.
The test case follows the established pattern and effectively verifies that the payment network can handle .tg
addresses. The test is focused, has clear assertions, and uses predefined test data for validation.
Description of the changes
Support .tg for NEAR: https://app.asana.com/0/1207673178190193/1208099463835050
Summary by CodeRabbit
New Features
.tg
in addition to.near
.Bug Fixes
Tests
.tg
payment addresses in the payment network integration.testaccount.tg
) in the address validation tests.