-
Notifications
You must be signed in to change notification settings - Fork 6
Fix: bug(facilitator-sdk): facilitatorFee 校验仅支持 hex,导致动态 fee 流程失败 #203
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,6 +35,58 @@ export function isValid256BitHex(hex: string): boolean { | |||||||||||||||||||||||||||||||||||||||||||||||
| return /^0x[a-fA-F0-9]{1,64}$/.test(hex); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||
| * Check if a string is a valid facilitator fee amount. | ||||||||||||||||||||||||||||||||||||||||||||||||
| * Accepts both decimal (atomic units) and hex formats for compatibility. | ||||||||||||||||||||||||||||||||||||||||||||||||
| * Ensures the value is non-negative and fits within uint256. | ||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||
| * @param fee - The fee amount to validate (decimal or hex string) | ||||||||||||||||||||||||||||||||||||||||||||||||
| * @returns true if the fee is valid, false otherwise | ||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||
| * @example | ||||||||||||||||||||||||||||||||||||||||||||||||
| * ```typescript | ||||||||||||||||||||||||||||||||||||||||||||||||
| * isValidFacilitatorFee("10000") // true - decimal | ||||||||||||||||||||||||||||||||||||||||||||||||
| * isValidFacilitatorFee("0x2710") // true - hex (10000) | ||||||||||||||||||||||||||||||||||||||||||||||||
| * isValidFacilitatorFee("0x0") // true - zero | ||||||||||||||||||||||||||||||||||||||||||||||||
| * isValidFacilitatorFee("-100") // false - negative | ||||||||||||||||||||||||||||||||||||||||||||||||
| * isValidFacilitatorFee("abc") // false - non-numeric | ||||||||||||||||||||||||||||||||||||||||||||||||
| * isValidFacilitatorFee("0xXYZ") // false - invalid hex | ||||||||||||||||||||||||||||||||||||||||||||||||
| * ``` | ||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||
| export function isValidFacilitatorFee(fee: string): boolean { | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (!fee || typeof fee !== "string") { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if it's a hex string (0x prefix) | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (fee.startsWith("0x") || fee.startsWith("0X")) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Must be valid hex with 1-64 hex digits (uint256 max) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return /^0x[a-fA-F0-9]{1,64}$/.test(fee); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if it's a decimal string (atomic units) | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Must be one or more digits, no sign or decimal point | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (!/^\d+$/.test(fee)) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Ensure it fits within uint256 (max value is 2^256 - 1) | ||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const value = BigInt(fee); | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if value can be represented in uint256 | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Max uint256 is approximately 1.16e77 | ||||||||||||||||||||||||||||||||||||||||||||||||
| // A simple check is that the decimal string shouldn't be longer than 78 digits | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (fee.length > 78) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| // BigInt handles arbitrary precision, so we just need to ensure it's not negative | ||||||||||||||||||||||||||||||||||||||||||||||||
| // and fits in a reasonable range for uint256 | ||||||||||||||||||||||||||||||||||||||||||||||||
| return value >= 0n; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const value = BigInt(fee); | |
| // Check if value can be represented in uint256 | |
| // Max uint256 is approximately 1.16e77 | |
| // A simple check is that the decimal string shouldn't be longer than 78 digits | |
| if (fee.length > 78) { | |
| return false; | |
| } | |
| // BigInt handles arbitrary precision, so we just need to ensure it's not negative | |
| // and fits in a reasonable range for uint256 | |
| return value >= 0n; | |
| // Quick length-based rejection: any value with more than 78 decimal digits | |
| // is guaranteed to be greater than the maximum uint256. | |
| if (fee.length > 78) { | |
| return false; | |
| } | |
| const value = BigInt(fee); | |
| const MAX_UINT256 = BigInt( | |
| "115792089237316195423570985008687907853269984665640564039457584007913129639935", | |
| ); | |
| // Value must be non-negative and must not exceed the uint256 maximum. | |
| return value >= 0n && value <= MAX_UINT256; |
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.
✅ FIXED: You're absolutely right! The length-based check alone was insufficient. I've updated the validation to properly compare against the actual MAX_UINT256 value using BigInt. The function now correctly rejects any decimal value exceeding 2^256 - 1.
Action taken: Replaced length-only check with proper BigInt comparison against MAX_UINT256 constant in isValidFacilitatorFee()
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,7 @@ import { | |||||||||||||||||||||||||||||
| isValidHex, | ||||||||||||||||||||||||||||||
| isValid32ByteHex, | ||||||||||||||||||||||||||||||
| isValid256BitHex, | ||||||||||||||||||||||||||||||
| isValidFacilitatorFee, | ||||||||||||||||||||||||||||||
| validateSettlementRouter, | ||||||||||||||||||||||||||||||
| validateSettlementExtra, | ||||||||||||||||||||||||||||||
| validateNetwork, | ||||||||||||||||||||||||||||||
|
|
@@ -81,6 +82,105 @@ describe("validation utilities", () => { | |||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| describe("isValidFacilitatorFee", () => { | ||||||||||||||||||||||||||||||
| describe("decimal format (atomic units)", () => { | ||||||||||||||||||||||||||||||
| it("should return true for valid decimal strings", () => { | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0")).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("10000")).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("1000000")).toBe(true); // 1 USDC (6 decimals) | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("1000000000000000000")).toBe(true); // 1 ETH (18 decimals) | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("1")).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("123456789")).toBe(true); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("should return false for non-numeric decimal strings", () => { | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("abc")).toBe(false); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("100abc")).toBe(false); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("12.34")).toBe(false); // decimal point not allowed | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("-100")).toBe(false); // negative sign not allowed | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("+100")).toBe(false); // plus sign not allowed | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("should return false for decimal values that exceed uint256", () => { | ||||||||||||||||||||||||||||||
| // A value with more than 78 digits exceeds uint256 max (2^256-1 ≈ 1.16e77) | ||||||||||||||||||||||||||||||
| const tooLarge = "1" + "0".repeat(78); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee(tooLarge)).toBe(false); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Test with an even larger number | ||||||||||||||||||||||||||||||
| const wayTooLarge = "9".repeat(100); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee(wayTooLarge)).toBe(false); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("should accept values within uint256 range", () => { | ||||||||||||||||||||||||||||||
| // Max uint256 is approximately 1.16e77, so 77-78 digits is the boundary | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("1" + "0".repeat(76))).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("115792089237316195423570985008687907853269984665640564039457584007913129639935")).toBe(true); // 2^256 - 1 | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| describe("hex format", () => { | ||||||||||||||||||||||||||||||
| it("should return true for valid hex strings", () => { | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0x0")).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0x1")).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0x2710")).toBe(true); // 10000 in decimal | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0x186A0")).toBe(true); // 100000 in decimal (0.1 USDC) | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0x" + "FF".repeat(32))).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0xFF")).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0xabcdef")).toBe(true); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0xABCDEF")).toBe(true); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("should return false for invalid hex strings", () => { | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0x" + "FF".repeat(33))).toBe(false); // > 256 bits | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0xGG")).toBe(false); | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0x")).toBe(false); // empty hex | ||||||||||||||||||||||||||||||
| expect(isValidFacilitatorFee("0x123")).toBe(false); // odd length | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| }); | |
| it("should return false for invalid hex strings", () => { | |
| expect(isValidFacilitatorFee("0x" + "FF".repeat(33))).toBe(false); // > 256 bits | |
| expect(isValidFacilitatorFee("0xGG")).toBe(false); | |
| expect(isValidFacilitatorFee("0x")).toBe(false); // empty hex | |
| expect(isValidFacilitatorFee("0x123")).toBe(false); // odd length | |
| expect(isValidFacilitatorFee("0x123")).toBe(true); // odd-length hex is valid numeric input | |
| }); | |
| it("should return false for invalid hex strings", () => { | |
| expect(isValidFacilitatorFee("0x" + "FF".repeat(33))).toBe(false); // > 256 bits | |
| expect(isValidFacilitatorFee("0xGG")).toBe(false); | |
| expect(isValidFacilitatorFee("0x")).toBe(false); // empty hex |
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.
✅ FIXED: You're correct! Odd-length hex strings are valid numeric representations (e.g., '0x1' = 1, '0xFFF' = 4095). The function intentionally follows the same pattern as isValid256BitHex. I've fixed the test to expect the correct behavior.
Action taken: Removed invalid expectation and added positive test cases for valid odd-length hex strings
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.
There's a mismatch between the prefix check and the regex validation. Line 62 accepts both "0x" and "0X" prefixes, but line 64's regex
/^0x[a-fA-F0-9]{1,64}$/only matches lowercase "0x" prefix. This means strings like "0X2710" would pass the startsWith check but fail the regex validation, causing the function to fall through to the decimal validation logic and incorrectly return false.The regex should be updated to accept both cases:
/^0[xX][a-fA-F0-9]{1,64}$/to match the prefix check on line 62.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.
✅ FIXED: Good catch! The regex indeed didn't match the prefix check logic. I've updated it to
/^0[xX][a-fA-F0-9]{1,64}$/to properly accept both lowercase '0x' and uppercase '0X' prefixes.Action taken: Updated regex pattern to /^0[xX][a-fA-F0-9]{1,64}$/ to match both prefix cases