fix: changed aleo validation to validate the token_id format #1643
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
WalkthroughReplaced Aleo Bech32 address validation with numeric field-element checks in currencyManager. Updated corresponding tests and ERC20 declarative test data to use field-element format. Removed Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as validateAddress(network, addr)
participant CM as currencyManager.validateAleoAddress
Caller->>CM: validateAleoAddress(addr)
CM->>CM: check typeof addr === "string"
CM->>CM: ensure addr endsWith "field"
CM->>CM: extract numeric part (before "field")
CM->>CM: check length == 76 and /^\d+$/.test(numeric)
CM-->>Caller: return true/false
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
requested review from
MantisClone,
aimensahnoun,
bassgeta and
yomarion
![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 (3)
packages/currency/src/currencyManager.ts (2)
328-336: Clarify JSDoc: this validates Aleo token_id, not a wallet addressTo avoid confusion, the JSDoc should explicitly state this validates the Aleo ERC20 token_id (a field element), not a wallet address.
Apply this diff to clarify the wording:
- /** - * Validate an Aleo currency address (field element). - * Aleo currency addresses are field elements with 76-77 digits followed by "field". - * See https://developer.aleo.org/guides/standards/token_registry#token-registry-program-constants - * - * Examples: - * - 7311977476241952331367670434347097026669181172395481678807963832961201831695field - * - 6088188135219746443092391282916151282477828391085949070550825603498725268775field - * - * @param address - The Aleo currency address to validate - * @returns True if the address is valid, false otherwise - */ + /** + * Validate an Aleo ERC20 token_id (field element). + * Aleo token identifiers are field elements represented as 76–77 decimal digits followed by "field". + * See https://developer.aleo.org/guides/standards/token_registry#token-registry-program-constants + * + * Examples: + * - 7311977476241952331367670434347097026669181172395481678807963832961201831695field + * - 6088188135219746443092391282916151282477828391085949070550825603498725268775field + * + * @param address - The Aleo token_id candidate to validate + * @returns True if the token_id is valid, false otherwise + */
341-347: Simplify and harden validation with trim + a single anchored regexFunctionally equivalent but more readable and resilient (handles accidental surrounding whitespace).
Apply this diff:
try { - if (!address || typeof address !== 'string' || !address.endsWith('field')) { - return false; - } - - const numericPart = address.slice(0, -5); - return (numericPart.length === 76 || numericPart.length === 77) && /^\d+$/.test(numericPart); + if (typeof address !== 'string') { + return false; + } + const input = address.trim(); + // 76–77 decimal digits followed by "field" + return /^\d{76,77}field$/.test(input);packages/currency/test/currencyManager.test.ts (1)
770-782: Good coverage; consider adding a 77-digit positive and a trimmed-input case
- The positive cases look good and align with the new validator.
- To fully exercise the 76–77 rule, add one 77-digit example.
- If we adopt trimming in the validator, add a case with surrounding whitespace.
Example additions:
it('should validate correct Aleo field elements', () => { @@ ).toBe(true); + // 77-digit numeric part + const seventySevenDigits = `${'1'.repeat(77)}field`; + expect(currencyManager.validateAleoAddress(seventySevenDigits)).toBe(true); }); @@ it('should reject invalid addresses', () => { @@ - expect(currencyManager.validateAleoAddress('')).toBe(false); + expect(currencyManager.validateAleoAddress('')).toBe(false); + // If trimming is accepted in validator: + expect( + currencyManager.validateAleoAddress( + ' 7311977476241952331367670434347097026669181172395481678807963832961201831695field ', + ), + ).toBe(true); });Optional: rename the describe block to reflect semantics better.
- describe('validateAleoAddress', () => { + describe('validateAleoTokenId (Aleo field element)', () => {Also applies to: 784-803
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/currency/package.json(0 hunks)packages/currency/src/currencyManager.ts(1 hunks)packages/currency/test/currencyManager.test.ts(1 hunks)packages/request-client.js/test/index.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/currency/package.json
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2024-11-05T16:53:05.280Z
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#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-11-22T13:30:25.703Z
Learnt from: rodrigopavezi
PR: RequestNetwork/requestNetwork#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.tspackages/currency/test/currencyManager.test.ts
📚 Learning: 2024-11-08T18:24:06.144Z
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#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-13T10:00:17.504Z
Learnt from: rodrigopavezi
PR: RequestNetwork/requestNetwork#1512
File: packages/integration-test/test/lit-protocol.test.ts:48-50
Timestamp: 2024-12-13T10:00:17.504Z
Learning: In `packages/integration-test/test/lit-protocol.test.ts`, the wallet private key `'0x7b595b2bb732edddc4d4fe758ae528c7a748c40f0f6220f4494e214f15c5bfeb'` is fixed and can be hardcoded in the test file.
Applied to files:
packages/request-client.js/test/index.test.ts
📚 Learning: 2024-12-18T03:53:54.370Z
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#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-10-28T20:00:33.707Z
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1474
File: packages/smart-contracts/scripts/test-deploy-single-request-proxy.ts:4-7
Timestamp: 2024-10-28T20:00:33.707Z
Learning: In TypeScript test files (e.g., `test-deploy-single-request-proxy.ts`), it's acceptable to use `string` types for Ethereum addresses in interfaces (like `FeeProxyAddresses`), as stricter type safety is not necessary.
Applied to files:
packages/request-client.js/test/index.test.tspackages/currency/test/currencyManager.test.ts
📚 Learning: 2024-10-17T18:30:55.410Z
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#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-11-06T14:48:18.698Z
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.
Applied to files:
packages/request-client.js/test/index.test.tspackages/currency/test/currencyManager.test.ts
📚 Learning: 2024-11-22T13:13:26.166Z
Learnt from: rodrigopavezi
PR: RequestNetwork/requestNetwork#1475
File: packages/transaction-manager/test/unit/utils/test-data.ts:0-0
Timestamp: 2024-11-22T13:13:26.166Z
Learning: In `packages/transaction-manager/test/unit/utils/test-data.ts`, the `FakeLitProtocolProvider` class uses `{}` as the return type for its methods `encrypt` and `decrypt`. Changing the return type to a more specific interface caused issues, so the current return type `{}` should remain as is.
Applied to files:
packages/request-client.js/test/index.test.tspackages/currency/test/currencyManager.test.ts
📚 Learning: 2024-11-12T16:54:02.702Z
Learnt from: aimensahnoun
PR: RequestNetwork/requestNetwork#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/test/index.test.ts
📚 Learning: 2024-11-20T18:59:38.738Z
Learnt from: rodrigopavezi
PR: RequestNetwork/requestNetwork#1475
File: packages/request-client.js/src/http-data-access.ts:214-216
Timestamp: 2024-11-20T18:59:38.738Z
Learning: When validating Ethereum addresses in TypeScript code, prefer using `ethers.utils.isAddress` function from the `ethers` library instead of regex patterns.
Applied to files:
packages/currency/src/currencyManager.tspackages/currency/test/currencyManager.test.ts
📚 Learning: 2024-11-21T09:06:12.938Z
Learnt from: rodrigopavezi
PR: RequestNetwork/requestNetwork#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/currency/test/currencyManager.test.ts
🧬 Code Graph Analysis (1)
packages/currency/test/currencyManager.test.ts (1)
packages/payment-processor/test/payment/shared.ts (1)
currencyManager(4-21)
⏰ 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
🔇 Additional comments (1)
packages/request-client.js/test/index.test.ts (1)
1656-1657: LGTM: Aleo token_id test data updated to field-element formatSwitching the Aleo case to a field element ending with "field" aligns the integration test with the new validation logic. No further changes needed here.
alexandre-abrioux
approved these changes
Aug 14, 2025
kevindavee
approved these changes
Aug 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow up on #1642 where we added address validation for aleo to support denominating in ERC20 currencies for aleo. That PR mistakenly was validating the aleo wallet address instead of the token_id field.
Summary by CodeRabbit
New Features
Tests
Chores