feat: support exponential streams + added stream preview#854
feat: support exponential streams + added stream preview#854
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds exponential ("LD") Sablier streaming: UI toggle and exponent input, validation and schema updates, LD encoding paths, parsers and live-data plumbing to surface exponent/shape, and new graph components (BaseGraph/StreamGraph) for preview and proposal display. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Form as StreamForm
participant Valid as Validation
participant Graph as StreamGraph
participant Encoder as Encoder
participant Chain as Blockchain
participant Decoder as DecodedDisplay
User->>Form: set amount, duration/dates, toggle useExponential, set exponent
Form->>Valid: validate fields (exponent required if exponential)
Valid-->>Form: validation result
Form->>Graph: compute streamPreviewData (deposit, start, end, cliff, exponent)
Graph->>Graph: generate 50-point curve (LD if exponent present else LL)
Graph-->>Form: render preview
Form->>Encoder: choose function signature (createWith...LD or ...LL) and encode payload
Encoder->>Chain: submit transaction with encoded LD/LL payload
Chain->>Decoder: emit logs/tx
Decoder->>Graph: parse and render final stream curve (show exponent if present)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/hooks/src/useStreamData.ts (3)
175-227:⚠️ Potential issue | 🔴 Critical
flattenedStaticStreamsdoesn't handle LD stream configs — times will be incorrect.The flattening logic accesses
s.durations?.cliff,s.durations?.total,s.timestamps?.start, etc. (lines 197–207). LD configs havesegmentsWithDurationorsegmentsinstead, so these optional chains all resolve toundefined→Number(undefined) = NaNortotalDur = 0, producing incorrectstartTime/endTime/cliffTimefor LD streams.This block needs LD-aware branching similar to
calculateStreamTimesinstreams.ts.
334-346:⚠️ Potential issue | 🟠 Major
calculateStreamedAmountLLis used for exponential (LD) streams.For active/settled/pending LD streams, line 336 calls
calculateStreamedAmountLLwhich assumes linear interpolation. Exponential streams follow a power curve (amount * (elapsed/total)^exponent), so the real-time streamed amount will be wrong for LD streams. This will show incorrect progress to users.Consider adding a
calculateStreamedAmountLDfunction or detecting the stream type and dispatching accordingly.
121-138:⚠️ Potential issue | 🔴 CriticalEvent ABI missing
CreateLockupDynamicStream— LD stream IDs won't be captured.The log decoding on line 124 uses
createLockupLinearStreamEventAbi, which only includes theCreateLockupLinearStreamevent. When transaction receipts emitCreateLockupDynamicStreamevents,decodeEventLogwill throw (event not in ABI), get caught, map tonull, and be filtered out on lines 136–137. No LD stream IDs will be extracted.Stream flattening incompatible with LD configs.
Lines 195–208 flatten streams by accessing
s.durationsands.timestamps, which don't exist on LD stream configs (which havesegmentsWithDuration/segmentsinstead). LD streams will have undefined properties, resulting in incorrectstartTime,endTime, andcliffTimevalues inFlattenedStaticStream.Linear calculation applied to exponential streams.
Line 336 uses
calculateStreamedAmountLLfor all stream types.calculateStreamedAmountLDexists in the codebase but is not imported or called. LD streams with exponential curves will get incorrect streamed amount calculations.exponent and shape fields not populated in StreamLiveData.
Lines 352–370 construct
StreamLiveDatabut never populate the optionalexponentandshapefields, even though they're available in the static stream configuration.
🤖 Fix all issues with AI agents
In
`@packages/ui/src/DecodedTransactions/ArgumentDisplay/StreamArgumentDisplay.tsx`:
- Around line 132-169: The segment loop currently renders the wrong heading and
duplicates the outer "Stream #{index + 1}" label; change the segment heading
inside both maps (stream.segmentsWithDuration.map and stream.segments.map) to
"Segment #{segmentIndex + 1}" (use the segmentIndex variable). Also factor the
duplicated segment rendering into a shared renderer (a small helper component or
function inside StreamArgumentDisplay.tsx, e.g., renderSegment or
SegmentDisplay) that accepts (segment, segmentIndex) and a prop/flag for whether
to show duration (use formatStreamDuration(segment.duration)) or timestamp
(segment.timestamp), and then call that shared renderer from both maps to remove
the near-identical code paths.
In `@packages/ui/src/DecodedTransactions/TransactionDisplay/DecodedDisplay.tsx`:
- Line 93: The inline comment on DecodedDisplay (near the conditional that
checks for createWithDurationsLL/createWithTimestampsLL) is stale because the
condition now also includes the LD variants; update the comment to either list
all four constructors (createWithDurationsLL, createWithTimestampsLL,
createWithDurationsLD, createWithTimestampsLD) or replace it with a generic
phrase like "Check if this is a createWithDurations/createWithTimestamps
variant" so the comment accurately reflects the condition in the DecodedDisplay
component.
In `@packages/ui/src/Graph/BaseGraph.tsx`:
- Around line 51-64: The useEffect that animates the SVG path (using lineRef,
animationKey) can call lineRef.current! inside the setTimeout after the
component may have unmounted; capture the timeout ID and add a cleanup to
clearTimeout to avoid the callback running after unmount, and inside the timeout
callback guard against a null lineRef.current before accessing its style (remove
the non-null assertion). Update the effect to store the timer ID (e.g., const t
= setTimeout(...)), return a cleanup function that calls clearTimeout(t) and
(optionally) resets any transition, and check lineRef.current exists before
setting transition/strokeDashoffset/opacity.
- Around line 125-129: The hardcoded filter id "smooth" in the BaseGraph
component will conflict across multiple instances; change BaseGraph to generate
a unique filter id (e.g., call React.useId() or combine props.animationKey) and
replace the static id="smooth" with that unique id, then update every reference
to the filter (any attributes using url(`#smooth`) or "#smooth") to use the
generated id (e.g., `${uniqueId}-smooth`); ensure the unique id generation and
substitution occur inside the BaseGraph render so each instance emits and
references its own filter.
In `@packages/ui/src/Graph/Graph.css.ts`:
- Around line 13-20: The exported symbol cursorText in Graph.css.ts is not used
anywhere; either delete the entire cursorText style definition to remove dead
code, or if it's intended for future use, add a clear JSDoc comment above
cursorText explaining its purpose and keep it exported (and optionally add a
TODO and a test or example import) so reviewers know it's intentional; search
for usages of cursorText to confirm removal won't break anything before
committing.
In `@packages/ui/src/Graph/utils.ts`:
- Around line 11-12: getTouchEventSource currently returns e.touches[0] which
can be undefined on touchend/touchcancel; update getTouchEventSource to safely
return the first available touch by checking e.touches[0] and falling back to
e.changedTouches[0] (or undefined if neither exists), and adjust the return type
of getTouchEventSource to React.Touch | undefined so callers of
getTouchEventSource (by name) handle the possible undefined result.
- Around line 14-31: The utilities use horizontal coordinates but are misnamed:
rename calculateY to calculateX (and update its parameter names: event.clientX,
paddingX remain but change the param name from e if desired), and in
calculateVisibleIndex rename parameters y -> x and paddingY -> paddingX (and
rename the function if you prefer calculateVisibleIndexByX) so the parameter
names reflect horizontal usage; update the internal math/comments to reference
width/left/x consistently, and update all callers to use the new
function/parameter names to avoid confusion.
In `@packages/utils/src/sablier/streams.ts`:
- Around line 179-205: The parseStreamDataConfigTimestampsLD function is adding
a stray cliffTime property (cliffTime: Number(streamData.cliffTime)) even though
StreamConfigTimestampsLD and encodeCreateWithTimestampsLD do not define or emit
it; remove the cliffTime assignment from the returned object in
parseStreamDataConfigTimestampsLD so the returned value strictly matches
StreamConfigTimestampsLD and avoids producing NaN/undeclared properties when the
object is inspected or spread.
🧹 Nitpick comments (9)
packages/ui/src/Fields/DatePicker.tsx (1)
78-81: Double validation on date change.
setFieldValuealready triggers validation by default. CallingsetFieldTouched(id, true, true)triggers it again. Consider passingfalseas the third argument to one of them to avoid redundant validation cycles.♻️ Suggested fix
formikRef.current.setFieldValue(id, dateStr) - formikRef.current.setFieldTouched(id, true, true) + formikRef.current.setFieldTouched(id, true, false)packages/utils/src/sablier/streams.ts (1)
161-165: Exponent UD2x18 → number conversion truncates fractional values.
Number(exponentUD2x18 / BigInt(10 ** 18))uses BigInt integer division, which truncates any fractional part. This is fine given the current schema validates integer exponents (2–100), but if the protocol or UI ever supports fractional exponents (e.g., 2.5), this will silently round down.Consider using
Number(exponentUD2x18) / 1e18for lossless conversion (safe for values ≤ 100 × 10¹⁸ which is well within float64 precision).Also applies to: 188-191
packages/ui/src/DecodedTransactions/ArgumentDisplay/StreamArgumentDisplay.tsx (1)
27-32:formatTimestampacceptsany— consider narrowing tonumber.A non-numeric value passed here would silently produce "Invalid Date". Since this is a display helper, a
numbertype would catch misuse at compile time.packages/ui/src/index.ts (1)
15-15: Generic utility names re-exported at the top level could collide.The
Graph/utils.tsexports generic names likeisTouchEvent,calculateY,calculateVisibleIndex,getMouseEventSource,getTouchEventSource. SinceGraph/index.tsdoesexport * from './utils'and this file doesexport * from './Graph', all those names become part of the@buildeross/uipublic surface. If any other module in the barrel exports a symbol with the same name, you'll get a TypeScript/bundler conflict.Consider using named re-exports in
Graph/index.tsto limit the public API surface, or keep the utils internal to the Graph module.packages/proposal-ui/src/components/ProposalDescription/StreamDetails/StreamItem.tsx (1)
314-342: Graph renders with hardcoded dimensions inside a responsive container — confirm scaling.The
StreamGraphreceiveswidth={500}andheight={220}while the parentBoxusesw="100%"withaspectRatio: '2.27/1'. SinceBaseGraphuses aviewBox, the SVG should scale. However, tooltip positioning inrenderTooltipuses absolute pixel calculations based on thewidth/heightprops. On very narrow screens, the tooltip text may overlap or clip if the rendered pixel size is much smaller than the viewBox.packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamForm.tsx (1)
10-10:SECONDS_PER_DAYis duplicated across files.This constant is also defined in
StreamTokens.tsx(Line 46). Extract it to a shared location (e.g., a constants file in the same module or@buildeross/utils) to avoid drift.packages/ui/src/Graph/BaseGraph.tsx (1)
85-96:calculateYis misnamed — it computes horizontal (X) position, not Y.
calculateYinutils.tssubtracts the element'sleftfromclientX, producing a horizontal offset. The local variableyon Line 92 is then passed tocalculateVisibleIndexas a horizontal offset. This naming inversion is confusing for future readers.packages/ui/src/Graph/StreamGraph.tsx (2)
117-168: Tooltip position logic is fragile — hardcoded offsets must match BaseGraph internals.The tooltip computes positions using
chartWidth = width - 20,chartHeight = height - 60, and offsets+10,+30,-12(Lines 125-131). These must stay perfectly in sync withBaseGraph'spaddingX = 10andpaddingY = 30. If BaseGraph's padding constants change, the tooltip will misalign without any compile-time warning.Consider exporting BaseGraph's padding constants or passing them to the
renderTooltipcallback so the tooltip can compute positions consistently.
48-115:isExponentialin the dependency array is redundant.Line 112:
isExponentialis derived fromexponent(Line 45), andexponentis already in the dependency list at Line 111. Including both is harmless but unnecessary — React will already re-run the memo whenexponentchanges.
packages/ui/src/DecodedTransactions/ArgumentDisplay/StreamArgumentDisplay.tsx
Show resolved
Hide resolved
packages/ui/src/DecodedTransactions/TransactionDisplay/DecodedDisplay.tsx
Show resolved
Hide resolved
packages/ui/src/Graph/Graph.css.ts
Outdated
| export const cursorText = style({ | ||
| fontSize: '20px', | ||
| '@media': { | ||
| 'screen and (max-width: 768px)': { | ||
| fontSize: '12px', | ||
| }, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg 'cursorText' --type ts --type tsx --type js --type jsxRepository: BuilderOSS/nouns-builder
Length of output: 95
🏁 Script executed:
rg 'cursorText'Repository: BuilderOSS/nouns-builder
Length of output: 238
🏁 Script executed:
rg 'from.*Graph\.css|cursorText' -A 2 -B 2Repository: BuilderOSS/nouns-builder
Length of output: 1437
🏁 Script executed:
rg 'import.*cursorText|cursorText\s*=' --type-listRepository: BuilderOSS/nouns-builder
Length of output: 457
🏁 Script executed:
rg '(import.*cursorText|cursorText\s*[=:]|\bcursorText\b)' -B 2 -A 2Repository: BuilderOSS/nouns-builder
Length of output: 751
🏁 Script executed:
rg 'className.*cursorText|cursorText\s*\}' -B 3 -A 1Repository: BuilderOSS/nouns-builder
Length of output: 50
🏁 Script executed:
# Also search for files that might import from Graph.css or use the exported cursorText
rg 'import.*from.*Graph' --type tsRepository: BuilderOSS/nouns-builder
Length of output: 638
🏁 Script executed:
# Search for any reference to cursorText that's not in the export definition
rg 'cursorText' -C 5 | grep -v '^export const cursorText'Repository: BuilderOSS/nouns-builder
Length of output: 1672
🏁 Script executed:
# Also check the index.ts to see what's being exported
cat packages/ui/src/Graph/index.tsRepository: BuilderOSS/nouns-builder
Length of output: 253
cursorText is unused and should be removed or documented.
The export is defined but not imported or used anywhere in the codebase. Either remove it if it's not needed, or clarify its purpose if it's intended for future use.
🤖 Prompt for AI Agents
In `@packages/ui/src/Graph/Graph.css.ts` around lines 13 - 20, The exported symbol
cursorText in Graph.css.ts is not used anywhere; either delete the entire
cursorText style definition to remove dead code, or if it's intended for future
use, add a clear JSDoc comment above cursorText explaining its purpose and keep
it exported (and optionally add a TODO and a test or example import) so
reviewers know it's intentional; search for usages of cursorText to confirm
removal won't break anything before committing.
dan13ram
force-pushed
the
feat/sablier-exponential
branch
from
950be85 to
1b1592a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/ui/src/Graph/BaseGraph.tsx`:
- Around line 145-154: The vertical guide line in BaseGraph.tsx currently sets
opacity both as an SVG attribute (opacity="0.2") and via
style.opacity={cursorOpacity}, causing the style value to override and make the
line fully opaque when cursorOpacity is 1; update the line rendering to compute
a single opacity value (e.g., const baseOpacity = 0.2; const finalOpacity =
baseOpacity * cursorOpacity) and use that single finalOpacity for the SVG
opacity prop (or for style but not both), removing the conflicting hardcoded
"0.2" attribute so the line respects the intended faded appearance; locate this
change around the <line ... /> element where strokeColor, STROKE, currentX and
cursorOpacity are used.
- Around line 110-112: The code accesses data[visibleIndex] which can be
undefined if the data array shrinks; update BaseGraph to guard and keep
visibleIndex in-bounds by clamping or resetting it when data changes (e.g., in a
useEffect watching data.length, call
setVisibleIndex(Math.min(currentVisibleIndex, Math.max(0, data.length - 1))))
and also defensively check currentPoint before reading currentPoint.y so
currentX/currentY calculations use a safe fallback when currentPoint is
undefined; reference visibleIndex, data, currentPoint, currentX, and currentY
when applying the fixes.
- Around line 85-96: The helper calculateY is misnamed and actually computes an
X coordinate—rename calculateY → calculateX in utils and update all call sites
(including BaseGraph.handleMouseMove) to use calculateX; in BaseGraph change
local variable y to x (const x = calculateX(event, e, paddingX)) and rename the
visible index variable to avoid shadowing the component state (e.g., const
newIndex = calculateVisibleIndex(x, e, paddingY, data.length) and then call
setVisibleIndex(newIndex)); also update calculateVisibleIndex signature to
rename its first parameter from y to x to reflect the horizontal position.
🧹 Nitpick comments (2)
packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamForm.tsx (1)
137-164: Cliff input duplication across days/dates branches.The two cliff
<SmartInput>blocks (lines 138–163 and 199–224) are nearly identical — only thehelperTextdiffers. Consider extracting a sharedCliffInputcomponent to reduce duplication.Example extraction
const CliffInput: React.FC<{ index: number; helperText: string }> = ({ index, helperText }) => ( <SmartInput {...formik.getFieldProps(`streams.${index}.cliffDays`)} inputLabel="Cliff Period (optional, in days)" id={`streams.${index}.cliffDays`} type={FIELD_TYPES.NUMBER} placeholder={'0'} min={0} step={1} errorMessage={ formik.touched.streams?.[index]?.cliffDays ? getFieldError('cliffDays') : undefined } helperText={helperText} onChange={(e) => { formik.handleChange(e) formik.setFieldTouched(`streams.${index}.cliffDays`, true, false) }} onBlur={(e) => { formik.handleBlur(e) formik.setFieldTouched(`streams.${index}.cliffDays`, true, true) }} onFocus={(e: React.FocusEvent<HTMLInputElement>) => { e.target.select() }} /> )Also applies to: 198-225
packages/ui/src/Graph/BaseGraph.tsx (1)
67-67:Math.max(...data.map(...))can overflow the call stack for large datasets.Spreading a large array into
Math.maxarguments hits the engine's argument-count limit. Areduceis safer and equally readable:♻️ Suggested refactor
- const maximumY = useMemo(() => Math.max(...data.map((point) => point.y), 1), [data]) + const maximumY = useMemo( + () => data.reduce((max, point) => (point.y > max ? point.y : max), 1), + [data] + )
dan13ram
force-pushed
the
feat/sablier-exponential
branch
from
1b1592a to
c0e782a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/hooks/src/useStreamData.ts (3)
175-227:⚠️ Potential issue | 🔴 Critical
flattenedStaticStreamsdoes not handle LD stream configs — LD streams will get invalid time values.This block accesses
s.durations,s.timestamps,s.cliffTime, ands.unlockAmounts, which only exist onStreamConfigDurationsandStreamConfigTimestamps(LL variants). For LD streams (StreamConfigDurationsLD/StreamConfigTimestampsLD), these properties areundefined, resulting in:
totalDur = 0→endTime = executedAt(stream appears to have zero duration)startTime = 0/endTime = 0for timestamps LDunlockStart = 0n,unlockCliff = 0n(happens to be correct for LD, but by accident)The LD variants store timing in
segmentsWithDurationorsegments+startTime, not in the LL-shaped properties. This block needs LD-aware branching similar tocalculateStreamTimes.Suggested approach
for (const s of batch.streams as any[]) { + // Import and use calculateStreamTimes for proper LD/LL handling + const times = calculateStreamTimes(s, batch.isDurationsMode, executedAt) const sender = s.sender as Address const recipient = s.recipient as Address const depositAmount = s.depositAmount as bigint const cancelable = s.cancelable as boolean const transferable = s.transferable as boolean const unlockStart = (s.unlockAmounts?.start ?? 0n) as bigint const unlockCliff = (s.unlockAmounts?.cliff ?? 0n) as bigint - let startTime: number - let endTime: number - let cliffTime: number - - if (batch.isDurationsMode) { - const cliffDur = Number(s.durations?.cliff ?? 0) - const totalDur = Number(s.durations?.total ?? 0) - startTime = executedAt - endTime = executedAt + totalDur - cliffTime = cliffDur === 0 ? 0 : executedAt + cliffDur - } else { - startTime = Number(s.timestamps?.start ?? 0) - endTime = Number(s.timestamps?.end ?? 0) - cliffTime = Number(s.cliffTime ?? 0) - } - out.push({ sender, recipient, depositAmount, cancelable, transferable, - startTime, - cliffTime, - endTime, + startTime: times.startTime, + cliffTime: times.cliffTime, + endTime: times.endTime, unlockStart, unlockCliff, tokenAddress: batch.tokenAddress, }) }
329-347:⚠️ Potential issue | 🔴 Critical
calculateStreamedAmountLLis used for all streams including exponential LD — LD streams will show incorrect progress.For active/settled/pending LD streams,
calculateStreamedAmountLLcomputes a linear interpolation, while exponential streams follow a power-curve formula. This will produce visually and numerically incorrectstreamedAmountandwithdrawableAmountfor LD streams.The code should detect whether the stream is LD (e.g., by checking for
segments/segmentsWithDurationon the static config, or storing anisExponentialflag inFlattenedStaticStream) and dispatch tocalculateStreamedAmountLDaccordingly.
121-138:⚠️ Potential issue | 🔴 CriticalUse a separate ABI for decoding
CreateLockupDynamicStreamevents, or combine both event types into a single ABI constant.The code filters for both
CreateLockupLinearStreamandCreateLockupDynamicStreamevents (line 137), butdecodeEventLoguses onlycreateLockupLinearStreamEventAbi(line 125), which is defined in constants.ts as:.filter((item) => item.type === 'event' && item.name === 'CreateLockupLinearStream')When a
CreateLockupDynamicStreamevent is processed,decodeEventLogwill throw because that event isn't in the ABI. The catch block silently returnsnull, and the filter drops it, so all LD stream IDs are lost after proposal execution, making the LD feature non-functional.Additionally,
useStreamData.tsuses onlycalculateStreamedAmountLLfor all non-canceled/non-depleted streams without type discrimination, whereasStreamGraph.tsxproperly branches betweencalculateStreamedAmountLDandcalculateStreamedAmountLL. When fixed to extract LD stream IDs, the streamed amounts will be calculated incorrectly for LD streams.packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx (1)
287-305:⚠️ Potential issue | 🟡 MinorResidual
cliffDayscan block submission when exponential mode is enabled.When a user sets
cliffDays > durationDays, then toggles on exponential mode, the cliff input disappears (Line 137 of StreamForm.tsx) butcliffDayspersists in form state. The validation at Line 299 (cliffDuration > totalDuration) will reject the submission even though cliff is irrelevant for LD streams.Either skip cliff validation when
useExponentialis true (matching the schema), or clearcliffDaysto 0 when toggling exponential on.Option A: Skip cliff validation for exponential
+ const skipCliffValidation = values.useExponential && values.exponent + if (useDurations) { // Days from now - const cliffDuration = (stream.cliffDays || 0) * SECONDS_PER_DAY + const cliffDuration = skipCliffValidation ? 0 : (stream.cliffDays || 0) * SECONDS_PER_DAY const totalDuration = (stream.durationDays || 0) * SECONDS_PER_DAYOption B: Clear cliffDays on toggle (in StreamTokens.tsx line 787-792)
onChange={(e) => { formik.setFieldValue('useExponential', e.target.checked) // Set default exponent when enabling if (e.target.checked && !formik.values.exponent) { formik.setFieldValue('exponent', 2) } + // Clear cliff days for all streams when enabling exponential + if (e.target.checked) { + formik.values.streams.forEach((_, idx) => { + formik.setFieldValue(`streams.${idx}.cliffDays`, 0) + }) + } }}
🤖 Fix all issues with AI agents
In
`@packages/ui/src/DecodedTransactions/ArgumentDisplay/StreamArgumentDisplay.tsx`:
- Around line 154-167: The UI may render two "start:" rows when both
stream.timestamps.start and stream.startTime are present; update
StreamArgumentDisplay (around the JSX rendering that uses stream.timestamps,
stream.startTime, formatTimestamp) to render only one start row by making the
stream.startTime check conditional (else) when stream.timestamps is truthy or
explicitly prefer one source (e.g., use stream.timestamps.start if available,
otherwise stream.startTime) so only a single "start:" Flex is output.
🧹 Nitpick comments (6)
packages/ui/src/DecodedTransactions/ArgumentDisplay/StreamArgumentDisplay.tsx (2)
27-32:formatTimestamplacks input validation for edge cases.If a timestamp value of
0is passed (e.g.,stream.timestamps.startat line 156), this will render"Dec 31, 1969"instead of something meaningful. Some call sites guard with> 0(lines 161, 165), but lines 156–157 do not.Consider adding a guard or typing the parameter as
number:Suggested improvement
-const formatTimestamp = (timestamp: any) => - new Date(timestamp * 1000).toLocaleDateString('en-US', { +const formatTimestamp = (timestamp: number): string => { + if (!timestamp || timestamp <= 0) return 'N/A' + return new Date(timestamp * 1000).toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric', }) +}
44-63: Token logo + amount rendering pattern is duplicated.The
<img>element with identical styling for the token logo appears in bothSegmentDisplay(lines 47–61) and the batch stream rendering (lines 126–139). Consider extracting a smallTokenAmountorTokenLogohelper to reduce duplication.Also applies to: 123-143
packages/utils/src/sablier/streams.ts (1)
276-304:calculateStreamTimesLD handling looks correct, but the type discrimination could be fragile.The property-based checks (
'segments' in stream,'segmentsWithDuration' in stream) work today because LL variants don't have these properties. However, if the union evolves, this could silently mis-classify. Consider using a discriminant field (e.g., akindortypeproperty) on each variant for safer discrimination.This is minor since the current union is well-defined and the property names are sufficiently unique.
packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.schema.ts (1)
89-102: Conditional exponent validation is correct; Biome lint warning is a false positive.The Biome error "
Do not add then to an object" on Line 95 is a false positive. This is yup's standard.when()API, wherethenis a configuration key for conditional schema — not a.then()method making the object thenable. Consider adding an inline suppression comment if this triggers in CI.Suppression example
exponent: yup .number() .optional() .when('useExponential', { is: true, + // biome-ignore lint/suspicious/noThenProperty: yup .when() API config key then: (schema) => schema .required('Exponent is required when using exponential curve') .integer('Exponent must be a whole number') .min(2, 'Exponent must be at least 2') .max(100, 'Exponent cannot exceed 100'), otherwise: (schema) => schema, }),packages/ui/src/Graph/StreamGraph.tsx (1)
117-168: Tooltip text elements usebackgroundColorwhich has no effect on SVG<text>.Lines 151 and 162 pass
backgroundColor={'accent'}to<Text as="text">. Sinceas="text"renders an SVG<text>element, the CSSbackground-colorproperty has no visual effect. If you intended a background behind the tooltip text, you'd need an SVG<rect>behind it. If the prop isn't needed, removing it reduces confusion.packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamForm.tsx (1)
137-164: Cliff input duplication acrossdays/datesbranches.The cliff
SmartInputblocks at Lines 137-164 and 198-225 are nearly identical. Consider extracting a sharedCliffInputcomponent to reduce duplication. This is pre-existing pattern though, so deferrable.Also applies to: 198-225
| {stream.timestamps && ( | ||
| <> | ||
| <Flex> | ||
| start: {new Date(stream.timestamps.start * 1000).toLocaleString()} | ||
| </Flex> | ||
| <Flex> | ||
| end: {new Date(stream.timestamps.end * 1000).toLocaleString()} | ||
| </Flex> | ||
| <Flex>start: {formatTimestamp(stream.timestamps.start)}</Flex> | ||
| <Flex>end: {formatTimestamp(stream.timestamps.end)}</Flex> | ||
| </> | ||
| )} | ||
|
|
||
| {stream.startTime !== undefined && stream.startTime > 0 && ( | ||
| <Flex>start: {formatTimestamp(stream.startTime)}</Flex> | ||
| )} | ||
|
|
||
| {stream.cliffTime !== undefined && stream.cliffTime > 0 && ( | ||
| <Flex>cliff: {new Date(stream.cliffTime * 1000).toLocaleString()}</Flex> | ||
| <Flex>cliff: {formatTimestamp(stream.cliffTime)}</Flex> | ||
| )} |
There was a problem hiding this comment.
Potential duplicate "start:" line if both stream.timestamps and stream.startTime coexist.
Lines 156 and 162 both render a start: label. If a stream object ever has both stream.timestamps.start and stream.startTime defined, the UI will show two "start:" rows. If these are guaranteed to be mutually exclusive by the data model, consider adding an else-style guard (or a comment) to make the intent explicit.
🤖 Prompt for AI Agents
In
`@packages/ui/src/DecodedTransactions/ArgumentDisplay/StreamArgumentDisplay.tsx`
around lines 154 - 167, The UI may render two "start:" rows when both
stream.timestamps.start and stream.startTime are present; update
StreamArgumentDisplay (around the JSX rendering that uses stream.timestamps,
stream.startTime, formatTimestamp) to render only one start row by making the
stream.startTime check conditional (else) when stream.timestamps is truthy or
explicitly prefer one source (e.g., use stream.timestamps.start if available,
otherwise stream.startTime) so only a single "start:" Flex is output.
47739cc to
4ab9ed4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx (1)
360-373:⚠️ Potential issue | 🟠 MajorShape should reflect stream type (linear, cliff, or exponential), not just cliff presence.
Line 372 sets
shapebased only on cliff presence, but the form supports exponential streams via theuseExponentialflag. For exponential streams, the shape should be'dynamicExponential'to match the encoding functions. Currently, exponential streams will incorrectly haveshape: 'linear', which doesn't match their actual type. WhilevalidateBatchStreamsdoesn't validate specific shape values, other parts of the system may rely on the shape field for proper stream type identification (e.g., UI display, stream parsing).Update the shape assignment to check
values.useExponentialand setshapeto'dynamicExponential'for exponential streams.
🤖 Fix all issues with AI agents
In `@packages/ui/src/Graph/StreamGraph.tsx`:
- Around line 134-146: The tooltip position calculation (constants TEXT_OFFSET,
computed x and y using index, parts, chartWidth, chartHeight, GRAPH_PADDING_X,
GRAPH_PADDING_Y, and point.y/maximumY) can produce negative coordinates near
graph edges; clamp x and y after computing them so they never go outside the SVG
viewBox (e.g., ensure x is between GRAPH_PADDING_X and chartWidth -
GRAPH_PADDING_X - some tooltip width buffer, and y is between GRAPH_PADDING_Y
and chartHeight - GRAPH_PADDING_Y - some tooltip height buffer) to keep the
<text> tooltip fully visible — update the code that computes x and y to apply
these min/max bounds before rendering the text.
In `@packages/utils/src/sablier/encoding.ts`:
- Around line 130-132: The code currently does BigInt(stream.exponent) *
BigInt(10 ** 18) inside the streams.map batch, which will throw if
stream.exponent is fractional; instead validate and scale the float before
converting to BigInt: inside the mapping for batch (referencing streams.map and
exponentUD2x18), check that stream.exponent is a finite number (Number.isFinite)
and not NaN, compute a scaled integer like const scaled =
Math.round(stream.exponent * 1e18) (or Math.floor/Math.trunc if a specific
rounding is required), then set exponentUD2x18 = BigInt(scaled); optionally
throw a clear error if validation fails so callers know the input is invalid.
- Around line 13-19: The exponent comment and type annotations claiming a range
of 2-100 are wrong because UD2x18 is backed by uint64 with 18 decimals (max
~18.446744073709551615), so update all exponent comments in
CreateWithDurationsLDParams, CreateWithTimestampsLDParams and related type
comments in streams.ts to reflect the real UD2x18 maximum (~18.44) and valid
minimum (>=2); add runtime validation in the encoding flow (the functions that
convert exponent to UD2x18 before encoding — see the encoding utilities where
exponent is multiplied by 1e18) to check the exponent is within [2,
18.446744073709551615] (or a safe integer upper bound like 18) and throw a
descriptive error if out of range, so invalid exponents are rejected before any
on-chain encoding/transaction is attempted.
🧹 Nitpick comments (7)
packages/ui/src/Accordion/AccordionItem.tsx (1)
91-104: Overflow may flash during the opening animation.Since
overflowswitches to'visible'immediately whenisOpenbecomestrue, but the height animates from0→'auto', content can momentarily overflow the still-expanding container. If this causes a visible flicker, consider deferringoverflow: 'visible'until the animation completes (e.g., via framer-motion'sonAnimationCompletecallback).If the current behavior is visually acceptable for the stream preview use case, this is fine as-is.
packages/utils/src/sablier/streams.ts (1)
161-165: Exponent decoding truncates fractional values via integer division.
Number(exponentUD2x18 / BigInt(10 ** 18))is BigInt integer division, so an exponent like2.5(encoded as2500000000000000000n) would decode as2. This is consistent with the encoding side (which only accepts integers), but worth noting if fractional exponents are ever needed. Theexponentfield onStreamConfigDurationsLD/StreamConfigTimestampsLDcould carry a brief comment.Also applies to: 188-191
packages/hooks/src/useStreamData.ts (1)
196-233: Duplicated exponent extraction logic across LD branches.Lines 214-217 and 229-232 contain identical exponent extraction from UD2x18. Consider extracting a small helper:
Proposed helper
+const extractExponentFromSegments = (segments: any[]): number | undefined => { + if (segments.length > 0 && segments[0].exponent) { + const exponentUD2x18 = BigInt(segments[0].exponent) + return Number(exponentUD2x18 / 10n ** 18n) + } + return undefined +}packages/ui/src/Graph/StreamGraph.tsx (2)
105-115:isExponentialin the dependency array is derived fromexponentand is redundant.Since
isExponentialis computed asexponent !== undefined && exponent >= 2, listing bothexponentandisExponentialin the dependency array is redundant. RemovingisExponentialwould be cleaner, though functionally harmless.♻️ Suggested change
}, [ depositAmount, decimals, startTime, endTime, cliffTime, exponent, - isExponential, unlockStart, unlockCliff, ])
117-176:renderTooltipis recreated on every render.This function is passed as a prop to
BaseGraph. Wrapping it inuseCallbackwould stabilize the reference. Not critical sinceBaseGraphisn't memoized, but good practice.packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamForm.tsx (1)
10-10:SECONDS_PER_DAYis duplicated acrossStreamForm.tsxandStreamTokens.tsx.Both files define
const SECONDS_PER_DAY = 86400. Consider extracting this to a shared constant to keep things DRY.packages/ui/src/Graph/BaseGraph.tsx (1)
125-131: Confusing function name:handleMouseLeaveOrTouchEnter.This function handles mouse enter and touch start (setting opacity to 1), but the name contains "Leave" which implies the opposite. Consider renaming for clarity.
♻️ Suggested rename
- const handleMouseLeaveOrTouchEnd = () => { + const handleCursorExit = () => { setCursorOpacity(0) } - const handleMouseLeaveOrTouchEnter = () => { + const handleCursorEnter = () => { setCursorOpacity(1) }
| // Calculate position with text offset for better visual alignment | ||
| const TEXT_OFFSET = 12 | ||
| const x = | ||
| index * (chartWidth / Math.max(1, parts - 1)) + GRAPH_PADDING_X - TEXT_OFFSET | ||
| const y = | ||
| chartHeight - (point.y / maximumY) * chartHeight + GRAPH_PADDING_Y - TEXT_OFFSET | ||
|
|
||
| // Format timestamp as date | ||
| const timestamp = new Date(point.x * 1000).toLocaleDateString('en-US', { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| year: 'numeric', | ||
| }) |
There was a problem hiding this comment.
Tooltip text can render outside the SVG viewBox near graph boundaries.
When index is 0 or point.y is near maximumY, the computed x and y values (after subtracting TEXT_OFFSET and additional offsets of 10/28) can go negative, pushing the <text> elements above or left of the viewBox. Consider clamping x and y to keep the tooltip within visible bounds.
🤖 Prompt for AI Agents
In `@packages/ui/src/Graph/StreamGraph.tsx` around lines 134 - 146, The tooltip
position calculation (constants TEXT_OFFSET, computed x and y using index,
parts, chartWidth, chartHeight, GRAPH_PADDING_X, GRAPH_PADDING_Y, and
point.y/maximumY) can produce negative coordinates near graph edges; clamp x and
y after computing them so they never go outside the SVG viewBox (e.g., ensure x
is between GRAPH_PADDING_X and chartWidth - GRAPH_PADDING_X - some tooltip width
buffer, and y is between GRAPH_PADDING_Y and chartHeight - GRAPH_PADDING_Y -
some tooltip height buffer) to keep the <text> tooltip fully visible — update
the code that computes x and y to apply these min/max bounds before rendering
the text.
| export interface CreateWithDurationsLDParams { | ||
| sender: Address | ||
| recipient: Address | ||
| depositAmount: bigint | ||
| totalDuration: number // in seconds | ||
| exponent: number // Exponent for exponential curve (2-100) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the encoding.ts file to understand the structure
fd -e ts -p "encoding.ts" | head -5Repository: BuilderOSS/nouns-builder
Length of output: 106
🏁 Script executed:
# Get an overview of the file structure and size
wc -l packages/utils/src/sablier/encoding.tsRepository: BuilderOSS/nouns-builder
Length of output: 110
🏁 Script executed:
# Look at the specific lines mentioned in the review
sed -n '1,40p' packages/utils/src/sablier/encoding.tsRepository: BuilderOSS/nouns-builder
Length of output: 1133
🏁 Script executed:
# Find the encoding functions around lines 132 and 174
sed -n '120,180p' packages/utils/src/sablier/encoding.tsRepository: BuilderOSS/nouns-builder
Length of output: 1854
🌐 Web query:
Sablier V2 UD2x18 fixed point format uint64 maximum value
💡 Result:
In Sablier V2 / PRBMath, UD2x18 is a user-defined value type backed by uint64 (scaled by 1e18, i.e., 18 decimal places). (etherscan.io)
So its maximum raw (wrapped) value is:
uMAX_UD2x18 = 18_446_744_073_709_551_615(which is2^64 - 1) (etherscan.io)
And the corresponding maximum “real” fixed-point value is:
MAX = (2^64 - 1) / 1e18 = 18.446744073709551615(etherscan.io)
Citations:
- 1: https://etherscan.io/address/0xB5Ec9706C3Be9d22326D208f491E5DEef7C8d9f0?utm_source=openai
- 2: https://etherscan.io/address/0xB5Ec9706C3Be9d22326D208f491E5DEef7C8d9f0?utm_source=openai
- 3: https://etherscan.io/address/0xB5Ec9706C3Be9d22326D208f491E5DEef7C8d9f0?utm_source=openai
🌐 Web query:
Sablier LockupDynamic exponent range constraints UD2x18
💡 Result:
In Sablier LockupDynamic, each segment’s exponent is typed as UD2x18 (PRBMath’s unsigned 2.18 fixed-point, backed by uint64). That implies these range constraints:
- Minimum:
0(cannot be negative because it’s unsigned). - Maximum:
MAX_UD2x18 = 18_446744073709551615(raw, 18-decimal fixed point), i.e.18.446744073709551615as a real number. [1] - Scaling:
1e18represents1.0inUD2x18. [1] - Type definition:
UD2x18istype UD2x18 is uint64, so values are fundamentally capped byuint64. [1][2]
So valid exponents are: 0 ≤ exponent ≤ 18.446744073709551615 (encoded as a uint64 with 18 decimals). [1][2]
🏁 Script executed:
# Search for validation or checks on exponent values
rg -i "exponent" packages/utils/src/sablier/ -A 2 -B 2Repository: BuilderOSS/nouns-builder
Length of output: 9722
🏁 Script executed:
# Search for validation patterns in the entire utils/sablier directory
rg "validate|check|assert|throw.*exponent" packages/utils/src/sablier/ -A 3Repository: BuilderOSS/nouns-builder
Length of output: 2184
🏁 Script executed:
# Check if there's a validation layer in related files
fd -e ts -p "sablier" packages/utils/src/ | xargs ls -laRepository: BuilderOSS/nouns-builder
Length of output: 686
🌐 Web query:
Sablier V2 LockupDynamic contract exponent validation constraints
💡 Result:
In Sablier V2 LockupDynamic, the per-segment exponent is not range-validated by the LockupDynamic parameter-checking logic; it’s only constrained by its Solidity type:
- Each segment stores
exponentasUD2x18(uint64-backed fixed-point). (basescan.org) - During stream creation,
Helpers.checkCreateWithMilestones(... )calls_checkSegments(...), and_checkSegmentsvalidates milestones ordering/future-ness and that depositAmount == sum(segment.amount)—but it does not checksegment.exponent. (basescan.org) - Therefore, the effective exponent constraints are just PRBMath’s
UD2x18domain:0 <= exponent <= MAX_UD2x18- where
uMAX_UD2x18 = 18_446_744_073_709_551_615(i.e.,2^64 - 1in 18-decimal fixed point, ≈ 18.446744073709551615) (basescan.org)
Citations:
- 1: https://basescan.org/address/0x461e13056a3a3265cef4c593f01b2e960755de91
- 2: https://basescan.org/address/0x461e13056a3a3265cef4c593f01b2e960755de91
- 3: https://basescan.org/address/0x461e13056a3a3265cef4c593f01b2e960755de91
UD2x18 overflow: exponent range 2-100 exceeds the maximum value constraint.
Sablier's UD2x18 format is backed by uint64 with 18 decimals, with a maximum representable value of approximately 18.446744073709551615. The comments claiming a valid range of 2-100 are incorrect—any exponent above ~18 will overflow when multiplied by 10^18 during encoding (lines 132, 174), producing invalid on-chain values that will likely cause transaction reverts.
The incorrect constraint is documented in multiple locations:
CreateWithDurationsLDParams(line 19)CreateWithTimestampsLDParams(line 36)- Type comments in
streams.ts
Add runtime validation before encoding and correct all exponent range comments to reflect the actual UD2x18 constraint (maximum ~18.44, not 100).
🤖 Prompt for AI Agents
In `@packages/utils/src/sablier/encoding.ts` around lines 13 - 19, The exponent
comment and type annotations claiming a range of 2-100 are wrong because UD2x18
is backed by uint64 with 18 decimals (max ~18.446744073709551615), so update all
exponent comments in CreateWithDurationsLDParams, CreateWithTimestampsLDParams
and related type comments in streams.ts to reflect the real UD2x18 maximum
(~18.44) and valid minimum (>=2); add runtime validation in the encoding flow
(the functions that convert exponent to UD2x18 before encoding — see the
encoding utilities where exponent is multiplied by 1e18) to check the exponent
is within [2, 18.446744073709551615] (or a safe integer upper bound like 18) and
throw a descriptive error if out of range, so invalid exponents are rejected
before any on-chain encoding/transaction is attempted.
| const batch = streams.map((stream) => { | ||
| // Convert exponent to UD2x18 format (multiply by 10^18) | ||
| const exponentUD2x18 = BigInt(stream.exponent) * BigInt(10 ** 18) |
There was a problem hiding this comment.
BigInt(stream.exponent) will throw at runtime for non-integer values.
BigInt() throws a RangeError when given a fractional number (e.g., BigInt(2.5)). Since exponent is typed as number, there's no compile-time guard. If the upstream schema or UI ever permits a decimal exponent, this will crash.
Consider either constraining the type or adding a guard:
Proposed fix
- const exponentUD2x18 = BigInt(stream.exponent) * BigInt(10 ** 18)
+ const exponentUD2x18 = BigInt(Math.round(stream.exponent)) * 10n ** 18n📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const batch = streams.map((stream) => { | |
| // Convert exponent to UD2x18 format (multiply by 10^18) | |
| const exponentUD2x18 = BigInt(stream.exponent) * BigInt(10 ** 18) | |
| const batch = streams.map((stream) => { | |
| // Convert exponent to UD2x18 format (multiply by 10^18) | |
| const exponentUD2x18 = BigInt(Math.round(stream.exponent)) * 10n ** 18n |
🤖 Prompt for AI Agents
In `@packages/utils/src/sablier/encoding.ts` around lines 130 - 132, The code
currently does BigInt(stream.exponent) * BigInt(10 ** 18) inside the streams.map
batch, which will throw if stream.exponent is fractional; instead validate and
scale the float before converting to BigInt: inside the mapping for batch
(referencing streams.map and exponentUD2x18), check that stream.exponent is a
finite number (Number.isFinite) and not NaN, compute a scaled integer like const
scaled = Math.round(stream.exponent * 1e18) (or Math.floor/Math.trunc if a
specific rounding is required), then set exponentUD2x18 = BigInt(scaled);
optionally throw a clear error if validation fails so callers know the input is
invalid.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx (2)
287-305:⚠️ Potential issue | 🟡 Minor
handleSubmitcliff validation doesn't account for exponential mode.When
useExponentialis true, cliff is irrelevant (the schema already skips cliff validation at Lines 126/177 of the schema file, and the LD encoding ignores it). However, Lines 299-305 here still validatecliffDuration > totalDurationunconditionally. If a user sets a cliff, then enables exponential mode, this handleSubmit check would reject the submission with a confusing error even though the cliff is unused.Proposed fix
if (!Number.isFinite(totalDuration) || totalDuration <= 0) { actions.setFieldError( 'streams', `Stream #${i + 1}: Duration must be greater than 0 days.` ) return } - if (cliffDuration < 0 || cliffDuration > totalDuration) { + if (!values.useExponential && (cliffDuration < 0 || cliffDuration > totalDuration)) { actions.setFieldError( 'streams', `Stream #${i + 1}: Cliff must be within the total duration.` ) return }
360-373:⚠️ Potential issue | 🟡 Minor
shapedoesn't reflect exponential mode in validation params.At line 372,
shapeis derived only from cliff presence and never set to'dynamicExponential'whenuseExponentialis true. WhilevalidateBatchStreamsonly validates shape as a string (not its semantic value), the inconsistency means the shape won't accurately represent the actual vesting curve type, which could be problematic if shape is used for UI display or downstream processing. The actual transaction creation correctly uses theexponentparameter with LockupDynamic, but shape should be updated to maintain semantic accuracy.
🧹 Nitpick comments (7)
packages/ui/src/Accordion/AccordionItem.tsx (1)
91-104: Content may overflow during the opening animation.When
isOpenbecomestrue, overflow switches tovisibleimmediately, but the height is still animating from0toauto. This can cause a brief flash where content is visible outside the expanding container.Consider deferring
overflow: 'visible'until after the open animation completes (e.g., via framer-motion'sonAnimationComplete), or movingoverflowinto the animation variants so it transitions with the height.♻️ Option: move overflow into variants
const variants = { initial: { height: 0, paddingBottom: 0, + overflow: 'hidden', }, animate: { height: 'auto', paddingBottom: 24, + overflow: 'visible', + transitionEnd: { overflow: 'visible' }, }, }Then remove the inline
overflowfrom theatoms(…)call:className={atoms({ height: 'x0', - overflow: isOpen ? 'visible' : 'hidden', paddingLeft: 'x6', paddingRight: 'x6', })}Using
transitionEndin framer-motion ensuresoverflow: 'visible'is applied only after the height animation completes, preventing content from spilling out mid-transition.packages/ui/src/DecodedTransactions/ArgumentDisplay/StreamArgumentDisplay.tsx (2)
27-32:formatTimestamplacks input validation and usesanytype.If a non-numeric or
undefinedvalue reaches this helper,new Date(NaN)renders as"Invalid Date". While most call sites guard with truthiness checks, consider adding a type annotation (number) and a safeguard.Suggested improvement
-const formatTimestamp = (timestamp: any) => - new Date(timestamp * 1000).toLocaleDateString('en-US', { +const formatTimestamp = (timestamp: number) => { + const date = new Date(timestamp * 1000) + if (isNaN(date.getTime())) return 'N/A' + return date.toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric', }) +}
132-169: Extract a shared segment renderer to eliminate near-identical blocks.The
segmentsWithDuration(lines 132–169) andsegments(lines 170–203) blocks differ only in renderingdurationvstimestamp. A small helper would cut ~35 lines of duplication.Example extraction
const renderSegment = ( segment: any, segmentIndex: number, streamIndex: number, mode: 'duration' | 'timestamp' ) => ( <Stack key={`segment-${streamIndex}-${segmentIndex}`} gap="x1"> <Text fontWeight="heading">Segment #{segmentIndex + 1}</Text> <Stack pl="x2" gap="x1"> {segment.amount && ( <Flex align="center" gap="x1"> <Text>amount:</Text> {tokenMetadata?.logo && ( <img src={tokenMetadata.logo} alt={tokenMetadata.symbol} loading="lazy" decoding="async" width="16px" height="16px" style={{ maxWidth: '16px', maxHeight: '16px', objectFit: 'contain' }} /> )} <Text>{formatAmount(BigInt(segment.amount))}</Text> </Flex> )} {segment.exponent && <Flex>exponent: {segment.exponent}</Flex>} {mode === 'duration' && segment.duration && ( <Flex>duration: {formatStreamDuration(segment.duration)}</Flex> )} {mode === 'timestamp' && segment.timestamp && ( <Flex>timestamp: {formatTimestamp(segment.timestamp)}</Flex> )} </Stack> </Stack> )packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokensDetailsDisplay.tsx (1)
5-9: UnusedstreamCountprop in type definition.
streamCountis still declared in the props interface (Line 8) but is no longer destructured or rendered. Meanwhile, the caller (StreamTokens.tsxLine 649) still passes it. Consider removing it from both the type and the call site to avoid confusion.Proposed fix
export const StreamTokensDetailsDisplay: React.FC<{ balanceError?: string totalStreamAmountWithSymbol?: string - streamCount?: number -}> = ({ totalStreamAmountWithSymbol, balanceError }) => { +}> = ({ totalStreamAmountWithSymbol, balanceError }) => {And in
StreamTokens.tsx:<StreamTokensDetailsDisplay balanceError={balanceError} totalStreamAmountWithSymbol={totalAmountString} - streamCount={formik.values.streams.length} />packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.schema.ts (1)
89-102: BiomenoThenPropertylint warning is a false positive here.The
thenkey is part of Yup's standard.when()configuration API ({ is, then, otherwise }), not a thenable/Promise property. This is idiomatic Yup usage. If Biome is enforced in CI, you may need to suppress it with an inline comment.Suppression if needed
exponent: yup .number() .optional() .when('useExponential', { is: true, + // biome-ignore lint/suspicious/noThenProperty: Yup .when() API uses `then` then: (schema) => schema .required('Exponent is required when using exponential curve')packages/ui/src/Graph/StreamGraph.tsx (1)
140-199:renderTooltipis recreated on every render.This closure captures
width,height, andsymbol, but isn't wrapped inuseCallback. IfBaseGraphperforms any reference-equality checks onrenderTooltip(e.g., viaReact.memo), this could cause unnecessary re-renders of the graph.Wrap in useCallback
- const renderTooltip = ( + const renderTooltip = React.useCallback(( index: number, data: GraphDataPoint[], cursorOpacity: number ) => { ... - } + }, [width, height, symbol])packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx (1)
776-822: Exponential toggle UI looks good.Auto-initializing exponent to
2when enabling the toggle (Line 791) is a nice UX touch. The conditional exponent input with range[2, 100]aligns with the schema validation.One consideration: when the user unchecks "Exponential Curve", the
exponentvalue remains in form state. This is fine since the encoding path checksvalues.useExponential && values.exponent(Line 441), so the stale exponent is ignored. However, you may want to clear it for a cleaner form state:Optional: clear exponent when disabling
onChange={(e) => { formik.setFieldValue('useExponential', e.target.checked) // Set default exponent when enabling if (e.target.checked && !formik.values.exponent) { formik.setFieldValue('exponent', 2) } + if (!e.target.checked) { + formik.setFieldValue('exponent', undefined) + } }}
dan13ram
force-pushed
the
feat/sablier-exponential
branch
from
4ab9ed4 to
efb36ab
Compare
dan13ram
force-pushed
the
feat/sablier-exponential
branch
from
efb36ab to
e6c4a5c
Compare
dan13ram
force-pushed
the
feat/sablier-exponential
branch
from
176f07c to
3109a81
Compare
dan13ram
force-pushed
the
feat/sablier-exponential
branch
from
308d55e to
4674ecd
Compare
1 participant
Summary by CodeRabbit
New Features
Improvements
Removals