fix(web): enforce min start time of now + 60s in stream validation schema - #502
Conversation
…hema Add validateStartTime() and getMinStartTime() to stream-validation.ts - Update calculateEndTime() to use getMinStartTime() when startTime is null - Update validateEndTime() to reject explicit start times < now + 60s - Update CreatePaymentStream.tsx fee estimation to use now + 60s start time - Write comprehensive unit tests covering success, failure, and edge cases
|
@Cerome360 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughStream timing validation now requires starts to be at least 60 seconds in the future, derives null starts from that threshold, validates end times accordingly, adds comprehensive tests, and uses the buffered start time for fee estimation. ChangesStream timing validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx (1)
104-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuffer duplicates the
MIN_START_TIME_OFFSET_SECONDSconstant instead of importing it.Hardcoding
60here duplicates the value now owned bystream-validation.ts; if that constant changes, fee estimation and actual validation will silently diverge.♻️ Proposed fix
- const startTime = BigInt(Math.floor(Date.now() / 1000) + 60); // 60s buffer for on-chain latency + const startTime = BigInt(getMinStartTime()); // buffered per MIN_START_TIME_OFFSET_SECONDS🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx` at line 104, Update the startTime calculation in CreatePaymentStream to import and use MIN_START_TIME_OFFSET_SECONDS from stream-validation.ts instead of the hardcoded 60-second offset, keeping fee estimation aligned with validation.apps/web/src/lib/stream-validation.ts (3)
40-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winError message hardcodes "60" instead of referencing
MIN_START_TIME_OFFSET_SECONDS.If the offset constant ever changes, this message silently goes stale. Interpolate the constant instead.
♻️ Proposed fix
const minStart = getMinStartTime(); if (startTime < minStart) { - return "Start time must be at least 60 seconds from now"; + return `Start time must be at least ${MIN_START_TIME_OFFSET_SECONDS} seconds from now`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/stream-validation.ts` around lines 40 - 52, Update the error message in validateStartTime to interpolate MIN_START_TIME_OFFSET_SECONDS instead of hardcoding “60”, keeping the existing validation logic and return behavior unchanged.
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMagic number
60duplicated across files instead of usingMIN_START_TIME_OFFSET_SECONDS. Three sites re-derive the 60-second buffer independently rather than deriving from the single exported constant; a future change to the constant would silently desync the error messages and the fee-estimation buffer from the actual validation minimum.
apps/web/src/lib/stream-validation.ts#L40-52: interpolateMIN_START_TIME_OFFSET_SECONDSinto thevalidateStartTimeerror string instead of the literal "60 seconds".apps/web/src/lib/stream-validation.ts#L94-121: interpolateMIN_START_TIME_OFFSET_SECONDSinto thevalidateEndTimeerror string as well.apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx#L104-104: replaceMath.floor(Date.now() / 1000) + 60withgetMinStartTime()(imported fromstream-validation.ts) so the fee-estimation buffer stays in sync with the validation minimum.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/stream-validation.ts` at line 1, Replace the hardcoded “60 seconds” text in validateStartTime and validateEndTime with MIN_START_TIME_OFFSET_SECONDS interpolation, and update CreatePaymentStream’s fee-estimation timestamp to use getMinStartTime() instead of adding 60 directly. Reuse these existing symbols so validation messages and the estimation buffer remain synchronized.
94-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame magic-number issue in the end-time error message.
Same as the
validateStartTimemessage — hardcode should referenceMIN_START_TIME_OFFSET_SECONDS.♻️ Proposed fix
if (endTime <= minEndTime) { - return "Stream end time must be at least 60 seconds from now"; + return `Stream end time must be at least ${MIN_START_TIME_OFFSET_SECONDS} seconds from now`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/stream-validation.ts` around lines 94 - 121, Update the end-time validation error message in the surrounding stream validation function to reference MIN_START_TIME_OFFSET_SECONDS instead of hardcoding “60 seconds,” matching the constant-based message used by validateStartTime while preserving the existing validation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/lib/__tests__/stream-validation.test.ts`:
- Around line 149-158: Remove the duplicate test case following the long-enough
duration test, or replace it with a distinct supported DurationUnit scenario; do
not label it as a minute test because DurationUnit has no minute unit. Keep the
existing unique coverage and assertions in validateEndTime tests unchanged.
---
Nitpick comments:
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx`:
- Line 104: Update the startTime calculation in CreatePaymentStream to import
and use MIN_START_TIME_OFFSET_SECONDS from stream-validation.ts instead of the
hardcoded 60-second offset, keeping fee estimation aligned with validation.
In `@apps/web/src/lib/stream-validation.ts`:
- Around line 40-52: Update the error message in validateStartTime to
interpolate MIN_START_TIME_OFFSET_SECONDS instead of hardcoding “60”, keeping
the existing validation logic and return behavior unchanged.
- Line 1: Replace the hardcoded “60 seconds” text in validateStartTime and
validateEndTime with MIN_START_TIME_OFFSET_SECONDS interpolation, and update
CreatePaymentStream’s fee-estimation timestamp to use getMinStartTime() instead
of adding 60 directly. Reuse these existing symbols so validation messages and
the estimation buffer remain synchronized.
- Around line 94-121: Update the end-time validation error message in the
surrounding stream validation function to reference
MIN_START_TIME_OFFSET_SECONDS instead of hardcoding “60 seconds,” matching the
constant-based message used by validateStartTime while preserving the existing
validation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 29c3aab2-882e-4ee2-ba3b-22e34a9533e3
📒 Files selected for processing (3)
apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsxapps/web/src/lib/__tests__/stream-validation.test.tsapps/web/src/lib/stream-validation.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
Closes #398
Add validateStartTime() and getMinStartTime() to stream-validation.ts - Update calculateEndTime() to use getMinStartTime() when startTime is null - Update validateEndTime() to reject explicit start times < now + 60s - Update CreatePaymentStream.tsx fee estimation to use now + 60s start time - Write comprehensive unit tests covering success, failure, and edge cases
Summary by CodeRabbit