feat: add support for sablier streams #847
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughThis PR adds Sablier streaming and multi-recipient token-send features, replaces NULL_ADDRESS with viem's zeroAddress/NATIVE_TOKEN_ADDRESS, refactors Escrow into MilestonePayments, updates transaction types and UI mappings, bumps viem, and adds related utilities (weth, sablier), schemas, and hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Form
participant StreamTokens as StreamTokens Component
participant ENS as ENS Resolver
participant SablierSvc as Sablier contracts module
participant Validator as Batch Validator
participant Encoder as Sablier Encoder
participant Store as Proposal Store
participant Chain as Blockchain
User->>StreamTokens: Submit stream tokens form
StreamTokens->>ENS: Resolve sender/recipient ENS
ENS->>Chain: ENS lookup
Chain-->>ENS: Resolved addresses
ENS-->>StreamTokens: Addresses
StreamTokens->>SablierSvc: Get Sablier contract addresses
SablierSvc->>Chain: Fetch latest release & addresses
Chain-->>SablierSvc: Addresses or null
SablierSvc-->>StreamTokens: Addresses
StreamTokens->>Validator: validateBatchStreams
Validator-->>StreamTokens: ValidationResult
alt Validation Success
StreamTokens->>Encoder: Encode batch create (durations/dates)
Encoder-->>StreamTokens: Encoded calldata (approve + create)
StreamTokens->>Store: Queue STREAM_TOKENS transaction
Store-->>User: Transaction queued
else Validation Failed
Validator-->>StreamTokens: Errors
StreamTokens-->>User: Display errors
end
sequenceDiagram
participant User as User/Form
participant SendTokens as SendTokens Component
participant ENS as ENS Resolver
participant Validation as Validation Engine
participant Encoder as ERC20/ETH Encoder
participant Store as Proposal Store
participant Chain as Blockchain
User->>SendTokens: Submit send tokens form
SendTokens->>ENS: Resolve ENS names (sender/recipients)
ENS->>Chain: ENS lookup
Chain-->>ENS: Resolved addresses
ENS-->>SendTokens: Addresses
SendTokens->>Validation: Validate token metadata & recipients
Validation-->>SendTokens: Validation result
alt Validation Success
SendTokens->>Encoder: Encode transfers / approvals
Encoder-->>SendTokens: Encoded transaction calldata
SendTokens->>Store: Queue transactions (type: SEND_TOKENS)
Store-->>User: Transaction queued
else Validation Failed
Validation-->>SendTokens: Errors
SendTokens-->>User: Display errors
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 5
🤖 Fix all issues with AI agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.tsx`:
- Around line 150-151: The code in SablierStream uses non-null assertions on
stream.startDate and stream.endDate when computing startTime/endTime; add a
defensive check in the component (e.g., in the submit handler or where
startTime/endTime are computed inside SablierStream) that verifies durationType
=== 'dates' implies stream.startDate and stream.endDate are defined and valid
before calling new Date(...). If either date is missing, short-circuit with a
controlled response (return/throw a clear error, or set a validation state to
prevent submit) and ensure you only compute Math.floor(new Date(...)/1000) after
the presence check to avoid runtime exceptions.
- Line 119: The code currently computes useDurations from
values.streams[0].durationType which forces every stream to be encoded using the
first stream's duration type and can corrupt data when streams vary; update the
logic to either (A) enforce a single global duration type by moving the
durationType selection out of StreamForm into the parent (so useDurations is
derived from a shared value) or (B) validate before submit that all
values.streams[*].durationType are identical and surface an error if mixed
(reject submit). Locate references to useDurations, values.streams, StreamForm
and adjust the form state or validation routine to implement one of these fixes
so encoding uses the correct per-batch or per-stream duration policy rather than
always using values.streams[0].durationType.
In
`@packages/proposal-ui/src/components/GovernorContractButton/GovernorContractButton.tsx`:
- Line 66: Remove the unnecessary "as any" cast when calling writeContractAsync;
replace the call to writeContractAsync(data.request as any) with
writeContractAsync(data.request) so TypeScript enforces correct types (this
value originates from useSimulateContract's data.request). Ensure the
surrounding guard that checks data is defined (in GovernorContractButton)
remains in place so the call is safe and no additional casting is used.
In `@packages/utils/src/sablier/encoding.ts`:
- Around line 103-198: The ABI for batchLockupCreateWithTimestampsLLAbi
incorrectly includes a cliff component inside the Lockup.Timestamps tuple;
remove the 'cliff' component from the Lockup.Timestamps components so that
Lockup.Timestamps contains only 'start' and 'end' (both uint40), leaving the
separate 'cliffTime' field in the batch entry untouched; update the components
array inside the tuple with internalType 'struct Lockup.Timestamps' in
batchLockupCreateWithTimestampsLLAbi to reflect only start and end to avoid
malformed calldata.
In `@packages/utils/src/sablier/validation.ts`:
- Around line 58-96: Validate the token address using isAddress instead of only
checking ZERO_ADDRESS (replace the existing params.tokenAddress check to call
isAddress(params.tokenAddress) and reject if false), and add non-negative checks
for timestamps and amounts: ensure params.startTime, params.endTime,
params.cliffTime are numbers (or BigInt as used) and >= 0 and maintain ordering
(start < end, start < cliff < end when cliff > 0), and ensure
params.startUnlockAmount, params.cliffUnlockAmount, and params.depositAmount are
numeric/BigInt and >= 0 and that totalUnlockAmount = params.startUnlockAmount +
params.cliffUnlockAmount <= params.depositAmount; keep the existing shape length
check using params.shape.trim() and MAX_SHAPE_LENGTH.
🧹 Nitpick comments (6)
packages/sdk/src/contract/requests/getDAOAddresses.ts (1)
28-40: Consider using a type guard for cleaner type narrowing.The
as DaoContractAddressesassertion on line 34 is applied before validation, when values could still beundefined. While the runtime behavior is correct (validation catches missing addresses), the type assertion bypasses TypeScript's checks.A type guard would provide proper type narrowing:
♻️ Suggested refactor with type guard
- const daoAddresses = { + const daoAddresses: Partial<DaoContractAddresses> = { token: tokenAddress, auction, governor, metadata, treasury, - } as DaoContractAddresses - const hasMissingAddresses = Object.values(daoAddresses).some( - (address) => address === zeroAddress || address === undefined - ) - if (hasMissingAddresses) return null + } + + const isValidAddress = (address: AddressType | undefined): address is AddressType => + address !== undefined && address !== zeroAddress + + if (!Object.values(daoAddresses).every(isValidAddress)) return null return daoAddresses as RequiredDaoContractAddressesAlternatively, if you want to keep the current structure, the runtime logic is correct and this is a minor type hygiene concern that can be deferred.
packages/constants/tsconfig.json (1)
7-8:skipLibCheckis inherited from the base tsconfig; explicitly setting it here is redundant but consistent with the project pattern.This setting is already defined in
@buildeross/tsconfig/base.json(which this tsconfig extends), and is consistently applied across the project (apps/web, multiple UI packages, etc.). The CI already runs type checking viatsc --noEmit, which validates application code even withskipLibCheckenabled (it only skips checking.d.tsfiles in node_modules).If you want additional safety against dependency type regressions, consider adding a separate CI check that runs
tsc --noEmit --skipLibCheck falseon a subset of packages, but this is optional given the current project-wide approach.packages/create-proposal-ui/src/components/TransactionForm/SablierStream/StreamForm.tsx (2)
48-59: Consider usingmin={1}or adding validation for positive amounts.The
min={0}allows zero-value streams which would likely fail validation or be rejected by the Sablier contracts. A minimum of 1 (or relying solely on schema validation) would provide earlier feedback.
66-100: Consider extracting inline styles to the design system.The inline styles for the radio button labels could be extracted to a shared style or use existing design system primitives for consistency across the codebase.
packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.tsx (2)
181-186: Replacealert()with in-form error display.Using
alert()for validation errors provides poor user experience - it blocks the UI thread and doesn't integrate with the form's error display. The form already has error display infrastructure (lines 478-505).Suggested approach
Set Formik errors instead of using alert:
const validationResult = validateBatchStreams(validationParams) if (!validationResult.isValid) { console.error('Stream validation failed:', validationResult.errors) - alert('Stream validation failed:\n' + validationResult.errors.join('\n')) + actions.setStatus({ validationErrors: validationResult.errors }) return }Then display
formik.status?.validationErrorsin the error section alongside other errors.
49-53: Consider adding error handling for contract address fetch.If
getSablierContractsfails, the state remains with null addresses and the submit button will be disabled, but the user won't know why. Consider adding error state and displaying a message.Suggested improvement
+ const [contractError, setContractError] = useState<string | null>(null) useEffect(() => { if (chainSupported) { - getSablierContracts(chain.id).then(setContractAddresses) + getSablierContracts(chain.id) + .then(setContractAddresses) + .catch((err) => { + console.error('Failed to load Sablier contracts:', err) + setContractError('Failed to load Sablier contract addresses') + }) } }, [chain.id, chainSupported])
packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.tsx
Outdated
Show resolved
Hide resolved
packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx
Show resolved
Hide resolved
| try { | ||
| setIsPending(true) | ||
| const hash = await writeContractAsync(data.request) | ||
| const hash = await writeContractAsync(data.request as any) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/proposal-ui/src/components/GovernorContractButton/GovernorContractButton.tsx | head -80Repository: BuilderOSS/nouns-builder
Length of output: 2967
🏁 Script executed:
# Search for useSimulateContract and useWriteContract usage patterns in the codebase
rg "useSimulateContract|useWriteContract" -A 3 -B 3 --type tsx --type tsRepository: BuilderOSS/nouns-builder
Length of output: 95
🏁 Script executed:
# Check if there are other files using data.request from useSimulateContract
rg "data\.request" packages/proposal-ui/src -A 2 -B 2 --type tsx --type tsRepository: BuilderOSS/nouns-builder
Length of output: 95
🏁 Script executed:
# Search for other uses of writeContractAsync in the codebase
rg "writeContractAsync" -A 2 -B 2Repository: BuilderOSS/nouns-builder
Length of output: 6480
🏁 Script executed:
# Check wagmi version and see if there's type info we can access
cat packages/proposal-ui/package.json | grep -A 5 -B 5 "wagmi\|viem"Repository: BuilderOSS/nouns-builder
Length of output: 335
🌐 Web query:
wagmi useSimulateContract useWriteContract data.request type compatibility v2
💡 Result:
In wagmi v2, useSimulateContract returns data typed as SimulateContractReturnType, which includes a request object intended to be passed directly into writeContract / useWriteContract (i.e., the simulated “write request that includes parameters”). [1]
React (wagmi v2) — type-compatible composition
import { useSimulateContract, useWriteContract } from 'wagmi'
function App() {
const { data } = useSimulateContract({
abi,
address,
functionName: 'transferFrom',
args: [from, to, 123n],
})
const { writeContract } = useWriteContract()
return (
<button
disabled={!data?.request}
onClick={() => {
if (!data?.request) return
writeContract(data.request)
}}
>
Write
</button>
)
}This is the pattern wagmi documents (note the writeContract(data!.request) example). [2]
About data.request + type (EIP-1559 / legacy / 2930)
If you set type on simulateContract, it narrows the transaction request type ('legacy' | 'eip1559' | 'eip2930') and that narrowed request is what you’ll later pass to writeContract. [1]
Key TS gotcha: data (and therefore data.request) is undefined until the simulation succeeds, so you must guard (if (!data?.request) return) or use a non-null assertion (data!.request). [2]
Sources:
[1] wagmi simulateContract action docs (return request, and type option) (wagmi.sh)
[2] wagmi useSimulateContract docs (“Composing with useWriteContract” example) (wagmi.sh)
Citations:
- 1: https://wagmi.sh/core/api/actions/simulateContract?utm_source=openai
- 2: https://wagmi.sh/react/api/hooks/useSimulateContract?utm_source=openai
Remove the as any cast — types are compatible.
According to wagmi v2 documentation, data.request from useSimulateContract is typed to pass directly to writeContractAsync without casting. Since the code already guards against data being undefined (line 62), use:
const hash = await writeContractAsync(data.request)
Another file in the codebase (PreAuction.tsx) uses this pattern without the cast. The as any unnecessarily bypasses type safety and should be removed.
🤖 Prompt for AI Agents
In
`@packages/proposal-ui/src/components/GovernorContractButton/GovernorContractButton.tsx`
at line 66, Remove the unnecessary "as any" cast when calling
writeContractAsync; replace the call to writeContractAsync(data.request as any)
with writeContractAsync(data.request) so TypeScript enforces correct types (this
value originates from useSimulateContract's data.request). Ensure the
surrounding guard that checks data is defined (in GovernorContractButton)
remains in place so the call is safe and no additional casting is used.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.tsx`:
- Around line 129-193: The ENS resolution and address normalization can throw or
return null (getEnsAddress/getAddress), which will reject the Promise.all and
crash submission; update the flow to guard both the senderAddress and each
recipientAddress resolution by wrapping getEnsAddress calls in try/catch, check
for null before calling getAddress, and if any resolution fails set a form error
via actions.setErrors (mentioning which stream or the sender) and return to
short‑circuit; replace the Promise.all mapping with a try/catch around the whole
batch build (or use a for-loop to await each stream) so RPC/transport errors are
caught and surfaced instead of bubbling up.
- Around line 376-379: The render computes totalInUnits by calling parseUnits on
each stream amount which can throw for empty/invalid inputs; update the
SablierStream component to guard parseUnits in the total calculation (the code
referencing formik.values.streams, parseUnits, decimals, and totalInUnits) by
skipping or treating invalid/empty amounts as 0 (e.g., check that stream.amount
is a non-empty numeric string / finite number before calling parseUnits or wrap
the parseUnits call in a try/catch and return 0n on failure) so intermediate
typing states no longer crash the form.
🧹 Nitpick comments (1)
packages/utils/src/sablier/encoding.ts (1)
231-246: Consider makingcancelableandtransferableconfigurable.The function hardcodes
cancelable: trueandtransferable: false, which limits flexibility. If the use case requires these specific values, this is fine. However, if users might need non-cancelable streams (for trustless vesting) or transferable streams, consider adding optional parameters with sensible defaults.♻️ Optional: Make stream properties configurable
export interface CreateWithDurationsLLParams { sender: Address recipient: Address depositAmount: bigint cliffDuration: number // in seconds totalDuration: number // in seconds + cancelable?: boolean // defaults to true + transferable?: boolean // defaults to false } export function encodeCreateWithDurationsLL( lockupLinearAddress: Address, tokenAddress: Address, streams: CreateWithDurationsLLParams[] ): string { const batch = streams.map((stream) => ({ sender: stream.sender, recipient: stream.recipient, depositAmount: stream.depositAmount, - cancelable: true, - transferable: false, + cancelable: stream.cancelable ?? true, + transferable: stream.transferable ?? false, durations: { cliff: stream.cliffDuration, total: stream.totalDuration, },
packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.tsx
Outdated
Show resolved
Hide resolved
packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx
Show resolved
Hide resolved
dan13ram
force-pushed
the
feat/sablier
branch
from
17f3ae9 to
e2a813f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.schema.ts`:
- Around line 7-14: Change the StreamFormValues.amount from number to string and
update all related initializations and parsing sites: replace the type in
StreamFormValues (amount: number → amount: string), update any initial form
values (e.g., amount: 0 → amount: ''), and replace usages that call
parseUnits(stream.amount.toString(), tokenDecimals) with
parseUnits(stream.amount, tokenDecimals). Also add validation for amount as a
decimal string (e.g., regex to reject scientific notation/invalid decimals)
before calling parseUnits to ensure safe parsing.
In `@packages/utils/src/sablier/validation.ts`:
- Around line 81-100: The timestamp checks currently use typeof === 'number'
which permits NaN/Infinity; update the guards for params.startTime,
params.endTime, and params.cliffTime to use Number.isFinite(...) (e.g., if
(!Number.isFinite(params.startTime)) errors.push(...)) so non-finite values are
rejected explicitly, while preserving the existing negative and zero checks for
startTime and the other comparisons in the same validation block.
🧹 Nitpick comments (1)
packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.tsx (1)
61-66: Prevent stale contract address updates on chain switch.The async
getSablierContractscall can resolve after a chain change and overwrite state with old addresses. Add a cancellation guard (and optionally reset when unsupported).🛠️ Suggested fix
useEffect(() => { - if (chainSupported) { - getSablierContracts(chain.id).then(setContractAddresses) - } + let cancelled = false + if (chainSupported) { + getSablierContracts(chain.id).then((result) => { + if (!cancelled) setContractAddresses(result) + }) + } else { + setContractAddresses({ batchLockup: null, lockup: null }) + } + return () => { + cancelled = true + } }, [chain.id, chainSupported])
packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.schema.ts
Show resolved
Hide resolved
dan13ram
force-pushed
the
feat/sablier
branch
from
e2a813f to
88d8fe6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.schema.ts`:
- Around line 26-34: The bigintSchema's transform currently converts any integer
number to BigInt using Number.isInteger, which allows unsafe integers to be
accepted; modify the transform in bigintSchema to use Number.isSafeInteger
instead of Number.isInteger and ensure unsafe numeric inputs are left unchanged
so the subsequent .test('is-bigint', ...) will fail validation for unsafe
numbers; locate the transform on bigintSchema and replace the Number.isInteger
check with Number.isSafeInteger and keep the rest of the transform/test logic
intact.
In `@packages/utils/src/sablier/validation.ts`:
- Around line 145-149: The current validation calls params.shape.trim() and uses
.length (UTF‑16 units) which fails for non-strings and multibyte characters;
update the check to first guard that params.shape is a string (e.g., if (typeof
params.shape !== 'string') push an error), avoid calling .trim() on non-strings,
then compute byte length using TextEncoder (const byteLen = new
TextEncoder().encode(params.shape.trim()).length) and compare byteLen against
MAX_SHAPE_LENGTH, pushing an error via the existing errors array when it exceeds
the byte limit; keep the error message descriptive and reference params.shape,
MAX_SHAPE_LENGTH, and errors to locate the change.
🧹 Nitpick comments (3)
packages/utils/src/sablier/contracts.ts (1)
44-75: Consider extracting a shared helper to reduce duplication.This function is nearly identical to
getSablierBatchLockupAddress, differing only in thecontractNameparameter. Extracting common logic into a helper would improve maintainability.♻️ Suggested refactor
+type SablierContractName = 'SablierBatchLockup' | 'SablierLockup' + +async function getSablierContractAddress( + chainId: CHAIN_ID, + contractName: SablierContractName +): Promise<Address | null> { + try { + const latestRelease = sablier.releases.getLatest({ + protocol: 'lockup', + }) + + if (!latestRelease) { + console.error('No latest Sablier lockup release found') + return null + } + + const contract = sablier.contracts.get({ + chainId, + contractName, + release: latestRelease, + }) + + if (!contract?.address) { + console.error(`${contractName} not found for chain ${chainId}`) + return null + } + + return contract.address as Address + } catch (error) { + console.error(`Error getting ${contractName} address for chain ${chainId}:`, error) + return null + } +} + +export async function getSablierBatchLockupAddress( + chainId: CHAIN_ID +): Promise<Address | null> { + return getSablierContractAddress(chainId, 'SablierBatchLockup') +} + +export async function getSablierLockupAddress( + chainId: CHAIN_ID +): Promise<Address | null> { + return getSablierContractAddress(chainId, 'SablierLockup') +}packages/utils/src/sablier/encoding.ts (1)
226-255: Consider usingHexreturn type for better type safety.
encodeFunctionDatafrom viem returnsHex(`0x${string}`), notstring. Using the precise return type improves type safety when this calldata is passed to other viem functions.💡 Suggested improvement
+import { Address, encodeFunctionData, Hex } from 'viem' export function encodeCreateWithDurationsLL( lockupLinearAddress: Address, tokenAddress: Address, streams: CreateWithDurationsLLParams[], cancelable: boolean = true, transferable: boolean = false -): string { +): Hex { export function encodeCreateWithTimestampsLL( lockupLinearAddress: Address, tokenAddress: Address, streams: CreateWithTimestampsLLParams[], cancelable: boolean = true, transferable: boolean = false -): string { +): Hex {Also applies to: 260-290
packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.schema.ts (1)
35-45: Consider reusing a shared TokenMetadataSchema to prevent drift.
TokenMetadataSchemamatches the one inpackages/create-proposal-ui/src/components/TransactionForm/Escrow/EscrowForm.schema.ts(Lines 90-100). Extracting a shared schema would avoid inconsistencies over time.
| const bigintSchema = yup | ||
| .mixed() | ||
| .transform((value) => { | ||
| if (typeof value === 'string' && /^\d+$/.test(value)) return BigInt(value) | ||
| if (typeof value === 'number' && Number.isInteger(value)) return BigInt(value) | ||
| return value | ||
| }) | ||
| .test('is-bigint', '${path} must be a BigInt', (value) => typeof value === 'bigint') | ||
|
|
There was a problem hiding this comment.
Guard against unsafe integer → BigInt coercion.
Line 30 accepts any integer number, including values beyond Number.MAX_SAFE_INTEGER, which can silently corrupt balances before conversion. Prefer Number.isSafeInteger so unsafe inputs fail validation.
🛠️ Proposed fix
- if (typeof value === 'number' && Number.isInteger(value)) return BigInt(value)
+ if (typeof value === 'number' && Number.isSafeInteger(value)) return BigInt(value)📝 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 bigintSchema = yup | |
| .mixed() | |
| .transform((value) => { | |
| if (typeof value === 'string' && /^\d+$/.test(value)) return BigInt(value) | |
| if (typeof value === 'number' && Number.isInteger(value)) return BigInt(value) | |
| return value | |
| }) | |
| .test('is-bigint', '${path} must be a BigInt', (value) => typeof value === 'bigint') | |
| const bigintSchema = yup | |
| .mixed() | |
| .transform((value) => { | |
| if (typeof value === 'string' && /^\d+$/.test(value)) return BigInt(value) | |
| if (typeof value === 'number' && Number.isSafeInteger(value)) return BigInt(value) | |
| return value | |
| }) | |
| .test('is-bigint', '${path} must be a BigInt', (value) => typeof value === 'bigint') |
🤖 Prompt for AI Agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/SablierStream/SablierStream.schema.ts`
around lines 26 - 34, The bigintSchema's transform currently converts any
integer number to BigInt using Number.isInteger, which allows unsafe integers to
be accepted; modify the transform in bigintSchema to use Number.isSafeInteger
instead of Number.isInteger and ensure unsafe numeric inputs are left unchanged
so the subsequent .test('is-bigint', ...) will fail validation for unsafe
numbers; locate the transform on bigintSchema and replace the Number.isInteger
check with Number.isSafeInteger and keep the rest of the transform/test logic
intact.
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 (2)
packages/proposal-ui/src/constants/transactionTypes.ts (1)
108-113: Invalid RGB value in iconBackdrop.The red component value
350exceeds the valid range of 0-255 for RGB values. While browsers typically clamp invalid values, this should be corrected for consistency and maintainability.Proposed fix
[TransactionType.MIGRATION]: { title: 'Migration', subTitle: 'Migrate from L1 to L2', icon: 'migrate', - iconBackdrop: 'rgba(350,100,0,.1)', + iconBackdrop: 'rgba(255, 100, 0, 0.1)', },packages/create-proposal-ui/src/components/TransactionForm/MilestonePayments/MilestonePaymentsForm.tsx (1)
74-81: Guard total amount calculation against invalid milestone inputs.
Lines 79–81 callparseUnitsdirectly during render without validation. When users enter empty or partial amounts while typing,parseUnitsthrows and crashes the form. Other forms in the codebase (StreamTokens, SendTokens) already wrapparseUnitsin try-catch blocks. Wrap this calculation with the same defensive pattern.🛠️ Proposed fix
- const totalInUnits = formik.values.milestones - .map((x) => parseUnits(x.amount.toString(), decimals)) - .reduce((acc, x) => acc + x, 0n) + const totalInUnits = formik.values.milestones + .map((x) => { + const raw = x.amount + const amount = + typeof raw === 'string' ? raw.trim() : raw === undefined ? '' : String(raw) + if (amount === '' || amount === '.') return 0n + if (!/^(\d+\.?\d*|\.\d+)$/.test(amount)) return 0n + try { + return parseUnits(amount, decimals) + } catch { + return 0n + } + }) + .reduce((acc, x) => acc + x, 0n)
🤖 Fix all issues with AI agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/MilestonePayments/MilestonePaymentsUtils.ts`:
- Around line 20-25: values.tokenAddress is optional and calling toLowerCase()
on undefined will throw; update the token resolution logic in
MilestonePaymentsUtils (the wrappedTokenAddress / selectedTokenAddress /
tokenAddress computation) to guard against undefined by defaulting
selectedTokenAddress to NATIVE_TOKEN_ADDRESS or checking for undefined before
calling toLowerCase(), then derive tokenAddress (use wrappedTokenAddress when
the selected value equals NATIVE_TOKEN_ADDRESS). Ensure the change references
the same symbols: wrappedTokenAddress, selectedTokenAddress, tokenAddress, and
NATIVE_TOKEN_ADDRESS.
In
`@packages/create-proposal-ui/src/components/TransactionForm/SendTokens/SendTokensDetailsDisplay.tsx`:
- Around line 9-19: The Box element in SendTokensDetailsDisplay is using an
inline style with a media query which React ignores; replace the style-based
maxWidth/media block with a responsive prop on the Box instead (match the
pattern used for position). Specifically, remove the style.maxWidth and
style.@media entries and add maxWidth={{ '@initial': '100%', '@768': '50%' }} to
the Box props so the component uses responsive props for maxWidth.
In
`@packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokensDetailsDisplay.tsx`:
- Around line 11-21: The inline style on the Box in
StreamTokensDetailsDisplay.tsx contains an invalid nested '@media' that React
will ignore; move that responsive rule into a vanilla-extract style and apply it
to the Box (or use Box's responsive prop). Create a container style in
StreamTokensDetailsDisplay.css.ts that includes height, maxWidth and the
'@media' rule for '(min-width: 768px)' setting maxWidth: '50%', export it (e.g.,
container), then import that style in StreamTokensDetailsDisplay.tsx and replace
the inline style's media part by applying the container className to the Box;
alternatively remove the '@media' object and use Box's responsive prop for
maxWidth if you prefer not to add a CSS file.
🧹 Nitpick comments (1)
packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamForm.tsx (1)
43-53: Consider using NUMBER type for the amount field.The
amountfield usesFIELD_TYPES.TEXTbut represents a numeric value. UsingFIELD_TYPES.NUMBERwould provide better UX with numeric keyboard on mobile and built-in browser validation.Suggested change
<SmartInput {...formik.getFieldProps(`streams.${index}.amount`)} inputLabel="Amount" id={`streams.${index}.amount`} - type={FIELD_TYPES.TEXT} + type={FIELD_TYPES.NUMBER} placeholder={'100'} + min={0} errorMessage={ formik.touched.streams?.[index]?.amount ? getFieldError('amount') : undefined } helperText="Amount of tokens to stream to this recipient" />
| const wrappedTokenAddress = getWrappedTokenAddress(chainId) | ||
| const selectedTokenAddress = values.tokenAddress as Address | ||
| const tokenAddress = | ||
| selectedTokenAddress.toLowerCase() === NULL_ADDRESS | ||
| selectedTokenAddress.toLowerCase() === NATIVE_TOKEN_ADDRESS.toLowerCase() | ||
| ? wrappedTokenAddress | ||
| : selectedTokenAddress |
There was a problem hiding this comment.
Guard optional tokenAddress before calling toLowerCase.
MilestonePaymentsFormValues.tokenAddress is optional, so values.tokenAddress can be undefined, which will throw at toLowerCase(). Consider defaulting to NATIVE_TOKEN_ADDRESS or explicitly validating before encoding.
🐛 Suggested defensive fix (default to native token when undefined)
- const selectedTokenAddress = values.tokenAddress as Address
+ const selectedTokenAddress = values.tokenAddress ?? NATIVE_TOKEN_ADDRESS
const tokenAddress =
selectedTokenAddress.toLowerCase() === NATIVE_TOKEN_ADDRESS.toLowerCase()
? wrappedTokenAddress
- : selectedTokenAddress
+ : (selectedTokenAddress as Address)📝 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 wrappedTokenAddress = getWrappedTokenAddress(chainId) | |
| const selectedTokenAddress = values.tokenAddress as Address | |
| const tokenAddress = | |
| selectedTokenAddress.toLowerCase() === NULL_ADDRESS | |
| selectedTokenAddress.toLowerCase() === NATIVE_TOKEN_ADDRESS.toLowerCase() | |
| ? wrappedTokenAddress | |
| : selectedTokenAddress | |
| const wrappedTokenAddress = getWrappedTokenAddress(chainId) | |
| const selectedTokenAddress = values.tokenAddress ?? NATIVE_TOKEN_ADDRESS | |
| const tokenAddress = | |
| selectedTokenAddress.toLowerCase() === NATIVE_TOKEN_ADDRESS.toLowerCase() | |
| ? wrappedTokenAddress | |
| : (selectedTokenAddress as Address) |
🤖 Prompt for AI Agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/MilestonePayments/MilestonePaymentsUtils.ts`
around lines 20 - 25, values.tokenAddress is optional and calling toLowerCase()
on undefined will throw; update the token resolution logic in
MilestonePaymentsUtils (the wrappedTokenAddress / selectedTokenAddress /
tokenAddress computation) to guard against undefined by defaulting
selectedTokenAddress to NATIVE_TOKEN_ADDRESS or checking for undefined before
calling toLowerCase(), then derive tokenAddress (use wrappedTokenAddress when
the selected value equals NATIVE_TOKEN_ADDRESS). Ensure the change references
the same symbols: wrappedTokenAddress, selectedTokenAddress, tokenAddress, and
NATIVE_TOKEN_ADDRESS.
| <Box | ||
| position={{ '@initial': 'relative', '@768': 'absolute' }} | ||
| style={{ | ||
| height: '100%', | ||
| maxWidth: '100%', | ||
| '@media': { | ||
| '(min-width: 768px)': { | ||
| maxWidth: '50%', | ||
| }, | ||
| }, | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "SendTokensDetailsDisplay.tsx" | head -20Repository: BuilderOSS/nouns-builder
Length of output: 168
🏁 Script executed:
cat -n ./packages/create-proposal-ui/src/components/TransactionForm/SendTokens/SendTokensDetailsDisplay.tsxRepository: BuilderOSS/nouns-builder
Length of output: 1989
🏁 Script executed:
# Find the zord package or its definition in node_modules or package.json
fd "zord" -t f | grep -E "(package\.json|index|types)" | head -20Repository: BuilderOSS/nouns-builder
Length of output: 50
🏁 Script executed:
cat -n ./package.jsonRepository: BuilderOSS/nouns-builder
Length of output: 2207
🏁 Script executed:
cat -n ./packages/create-proposal-ui/package.jsonRepository: BuilderOSS/nouns-builder
Length of output: 4132
🏁 Script executed:
find . -type f -path "*packages/zord*" -name "*.ts" -o -path "*packages/zord*" -name "*.tsx" | grep -i box | head -20Repository: BuilderOSS/nouns-builder
Length of output: 144
🏁 Script executed:
cat -n ./packages/zord/src/elements/Box.tsxRepository: BuilderOSS/nouns-builder
Length of output: 6138
🏁 Script executed:
cat -n ./packages/zord/src/atoms/atoms.ts | head -100Repository: BuilderOSS/nouns-builder
Length of output: 134
🏁 Script executed:
find ./packages/zord -type f -name "*.ts" -o -name "*.tsx" | grep -i atomRepository: BuilderOSS/nouns-builder
Length of output: 129
🏁 Script executed:
cat -n ./packages/zord/src/atoms.ts | head -150Repository: BuilderOSS/nouns-builder
Length of output: 102
🏁 Script executed:
cat -n ./packages/zord/src/atoms.css.ts | head -200Repository: BuilderOSS/nouns-builder
Length of output: 5617
Use responsive prop instead of inline style for responsive maxWidth.
React inline styles don't support media queries. The @media object will be ignored, so maxWidth: '50%' at 768px+ won't apply. The Box component supports responsive props directly—use maxWidth={{ '@initial': '100%', '@768': '50%' }} like you do with position.
Suggested fix
<Box
position={{ '@initial': 'relative', '@768': 'absolute' }}
- style={{
- height: '100%',
- maxWidth: '100%',
- '@media': {
- '(min-width: 768px)': {
- maxWidth: '50%',
- },
- },
- }}
+ height="100%"
+ maxWidth={{ '@initial': '100%', '@768': '50%' }}📝 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.
| <Box | |
| position={{ '@initial': 'relative', '@768': 'absolute' }} | |
| style={{ | |
| height: '100%', | |
| maxWidth: '100%', | |
| '@media': { | |
| '(min-width: 768px)': { | |
| maxWidth: '50%', | |
| }, | |
| }, | |
| }} | |
| <Box | |
| position={{ '@initial': 'relative', '@768': 'absolute' }} | |
| height="100%" | |
| maxWidth={{ '@initial': '100%', '@768': '50%' }} |
🤖 Prompt for AI Agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/SendTokens/SendTokensDetailsDisplay.tsx`
around lines 9 - 19, The Box element in SendTokensDetailsDisplay is using an
inline style with a media query which React ignores; replace the style-based
maxWidth/media block with a responsive prop on the Box instead (match the
pattern used for position). Specifically, remove the style.maxWidth and
style.@media entries and add maxWidth={{ '@initial': '100%', '@768': '50%' }} to
the Box props so the component uses responsive props for maxWidth.
| <Box | ||
| position={{ '@initial': 'relative', '@768': 'absolute' }} | ||
| style={{ | ||
| height: '100%', | ||
| maxWidth: '100%', | ||
| '@media': { | ||
| '(min-width: 768px)': { | ||
| maxWidth: '50%', | ||
| }, | ||
| }, | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and inspect the relevant code section
head -n 30 packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokensDetailsDisplay.tsxRepository: BuilderOSS/nouns-builder
Length of output: 920
🏁 Script executed:
# Inspect the Box component implementation to understand style handling
cat -n packages/zord/src/elements/Box.tsx | head -n 100Repository: BuilderOSS/nouns-builder
Length of output: 3331
🏁 Script executed:
# Check if a CSS file exists for this component
fd -e "css.ts" StreamTokensDetailsDisplayRepository: BuilderOSS/nouns-builder
Length of output: 173
🏁 Script executed:
# Search for how style prop is used in Box component
rg -n "style" packages/zord/src/elements/Box.tsx -A 2 -B 2 | head -n 50Repository: BuilderOSS/nouns-builder
Length of output: 806
Media query in style will be ignored by React.
The @media nested inside the style prop is invalid React inline style syntax and will be silently ignored. The desktop maxWidth: '50%' responsive rule never takes effect. Move this to vanilla-extract or use a responsive prop instead.
♻️ Suggested adjustment (move to vanilla-extract)
-import { link } from './StreamTokensDetailsDisplay.css'
+import { link, container } from './StreamTokensDetailsDisplay.css'
...
- <Box
- position={{ '@initial': 'relative', '@768': 'absolute' }}
- style={{
- height: '100%',
- maxWidth: '100%',
- '@media': {
- '(min-width: 768px)': {
- maxWidth: '50%',
- },
- },
- }}
+ <Box
+ position={{ '@initial': 'relative', '@768': 'absolute' }}
+ className={container}Add to StreamTokensDetailsDisplay.css.ts:
export const container = style({
height: '100%',
maxWidth: '100%',
'@media': {
'(min-width: 768px)': {
maxWidth: '50%',
},
},
})🤖 Prompt for AI Agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokensDetailsDisplay.tsx`
around lines 11 - 21, The inline style on the Box in
StreamTokensDetailsDisplay.tsx contains an invalid nested '@media' that React
will ignore; move that responsive rule into a vanilla-extract style and apply it
to the Box (or use Box's responsive prop). Create a container style in
StreamTokensDetailsDisplay.css.ts that includes height, maxWidth and the
'@media' rule for '(min-width: 768px)' setting maxWidth: '50%', export it (e.g.,
container), then import that style in StreamTokensDetailsDisplay.tsx and replace
the inline style's media part by applying the container className to the Box;
alternatively remove the '@media' object and use Box's responsive prop for
maxWidth if you prefer not to add a CSS file.
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/MilestonePayments/MilestonePayments.schema.ts (1)
106-112: Date threshold is evaluated at schema definition time, not validation time.The
new Date(Date.now() + 30 * ...)expression on line 110 is evaluated once when the schema module is loaded. If the schema is reused over time (e.g., during a long browser session), the "30 days from today" threshold becomes stale.Consider using a function that evaluates the date at validation time:
Suggested fix
safetyValveDate: yup .date() .required('Safety valve date is required.') - .min( - new Date(Date.now() + 30 * 24 * 60 * 60 * 1000), - 'Safety valve date must be at least 30 days from today or the last milestone.' - ) + .test( + 'min-30-days-from-now', + 'Safety valve date must be at least 30 days from today or the last milestone.', + function (value) { + if (!value) return true // required() handles this + const minDate = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000) + return new Date(value).getTime() >= minDate.getTime() + } + )
🤖 Fix all issues with AI agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.schema.ts`:
- Around line 53-60: Add a cross-field validation that ensures cliffDays does
not exceed the stream duration by updating the existing cross-validation/test
block that has access to durationType; when durationType === 'days' assert
cliffDays <= durationDays, and when durationType === 'dates' compute days
between startDate and endDate and assert cliffDays <= that difference; reference
the cliffDays field and use durationType, durationDays, startDate and endDate in
the test so the schema rejects forms where the cliff period is longer than the
total stream duration.
In
`@packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx`:
- Line 236: The submit handler currently calls parseUnits(stream.amount,
tokenDecimals) directly (assigned to depositAmount) which can throw on empty or
malformed input; add the same defensive validation used in the render logic
before calling parseUnits: ensure stream.amount is a non-empty string that
matches the decimal/format expected and tokenDecimals is defined, or
convert/normalize it (e.g., fallback to "0" or return early with validation
error) so parseUnits only receives valid numeric input; update the submit
handler (where depositAmount is computed) to perform this guard and handle the
invalid case (abort submission or set an error) rather than letting parseUnits
throw.
In `@packages/utils/src/sablier/contracts.ts`:
- Around line 12-39: The sablier API calls are using incorrect methods and
parameter names: replace the non-existent sablier.releases.getLatest() with
sablier.releases.getAll({ protocol: 'lockup' }).find(r => r.isLatest) to obtain
latestRelease, and change the sablier.contracts.get call to pass name:
contractName (not contractName key) along with release: latestRelease; also keep
the existing null checks/console.error paths if latestRelease or
contract?.address are missing and ensure the try/catch remains.
🧹 Nitpick comments (2)
packages/utils/src/sablier/validation.ts (1)
116-119: Simplify redundant condition.The condition
params.cliffTime >= 0 && params.cliffTime > 0can be simplified to justparams.cliffTime > 0, since any value greater than 0 is inherently >= 0.Suggested fix
if ( Number.isFinite(params.cliffTime) && - params.cliffTime >= 0 && params.cliffTime > 0 ) {packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx (1)
126-130: Consider usingsetFieldErrorfor type-safe error handling.The
as anycasts withsetErrorsare used throughout the submit handler. While functional, usingformik.setFieldError(fieldName, message)would be more type-safe and clearer for setting individual field errors.Example refactor
- actions.setErrors({ - tokenAddress: 'WETH is not available on this network', - } as any) + actions.setFieldError('tokenAddress', 'WETH is not available on this network')
| cliffDays: yup | ||
| .number() | ||
| .optional() | ||
| .test( | ||
| 'is-non-negative', | ||
| 'Cliff period cannot be negative', | ||
| (value) => value === undefined || value >= 0 | ||
| ), |
There was a problem hiding this comment.
Missing validation: cliff period should not exceed stream duration.
The cliffDays field is validated to be non-negative, but there's no constraint ensuring the cliff doesn't exceed the stream's total duration. A cliff period longer than the stream duration would create an invalid Sablier configuration where no tokens are ever unlocked before the stream ends.
This validation needs to happen in the cross-validation test since it depends on durationType:
- When
durationType === 'days':cliffDays <= durationDays - When
durationType === 'dates':cliffDays <= (endDate - startDate)in days
🛠️ Suggested addition to the cross-validation test (lines 84-141)
if (durationType === 'days') {
if (!stream.durationDays || stream.durationDays <= 0) {
errors.push(
this.createError({
path: `streams[${index}].durationDays`,
message: 'Duration must be greater than 0',
})
)
+ } else if (stream.cliffDays !== undefined && stream.cliffDays > stream.durationDays) {
+ errors.push(
+ this.createError({
+ path: `streams[${index}].cliffDays`,
+ message: 'Cliff period cannot exceed stream duration',
+ })
+ )
}
} else if (durationType === 'dates') {
// ... existing date validation ...
+
+ if (stream.startDate && stream.endDate && stream.cliffDays !== undefined) {
+ const durationMs = new Date(stream.endDate).getTime() - new Date(stream.startDate).getTime()
+ const durationDays = durationMs / (1000 * 60 * 60 * 24)
+ if (stream.cliffDays > durationDays) {
+ errors.push(
+ this.createError({
+ path: `streams[${index}].cliffDays`,
+ message: 'Cliff period cannot exceed stream duration',
+ })
+ )
+ }
+ }
}🤖 Prompt for AI Agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.schema.ts`
around lines 53 - 60, Add a cross-field validation that ensures cliffDays does
not exceed the stream duration by updating the existing cross-validation/test
block that has access to durationType; when durationType === 'days' assert
cliffDays <= durationDays, and when durationType === 'dates' compute days
between startDate and endDate and assert cliffDays <= that difference; reference
the cliffDays field and use durationType, durationDays, startDate and endDate in
the test so the schema rejects forms where the cliff period is longer than the
total stream duration.
| return | ||
| } | ||
|
|
||
| const depositAmount = parseUnits(stream.amount, tokenDecimals) |
There was a problem hiding this comment.
Unguarded parseUnits call can throw on invalid input.
Unlike the render-time calculation (lines 463-490) which has defensive guards, this parseUnits call in the submit handler lacks validation. If stream.amount is empty or malformed (e.g., user submits before validation completes), this will throw.
Consider adding a guard similar to the render-time calculation, or rely on schema validation to guarantee the amount is valid before reaching this point.
Suggested fix
+ // Validate amount before parsing
+ if (!stream.amount || typeof stream.amount !== 'string' || stream.amount.trim() === '') {
+ actions.setErrors({
+ streams: `Stream #${i + 1}: Amount is required.`,
+ } as any)
+ return
+ }
+
const depositAmount = parseUnits(stream.amount, tokenDecimals)🤖 Prompt for AI Agents
In
`@packages/create-proposal-ui/src/components/TransactionForm/StreamTokens/StreamTokens.tsx`
at line 236, The submit handler currently calls parseUnits(stream.amount,
tokenDecimals) directly (assigned to depositAmount) which can throw on empty or
malformed input; add the same defensive validation used in the render logic
before calling parseUnits: ensure stream.amount is a non-empty string that
matches the decimal/format expected and tokenDecimals is defined, or
convert/normalize it (e.g., fallback to "0" or return early with validation
error) so parseUnits only receives valid numeric input; update the submit
handler (where depositAmount is computed) to perform this guard and handle the
invalid case (abort submission or set an error) rather than letting parseUnits
throw.
| try { | ||
| // Get the latest release first | ||
| const latestRelease = sablier.releases.getLatest({ | ||
| protocol: 'lockup', | ||
| }) | ||
|
|
||
| if (!latestRelease) { | ||
| console.error('No latest Sablier lockup release found') | ||
| return null | ||
| } | ||
|
|
||
| // Get the contract | ||
| const contract = sablier.contracts.get({ | ||
| chainId, | ||
| contractName, | ||
| release: latestRelease, | ||
| }) | ||
|
|
||
| if (!contract?.address) { | ||
| console.error(`${contractName} not found for chain ${chainId}`) | ||
| return null | ||
| } | ||
|
|
||
| return contract.address as Address | ||
| } catch (error) { | ||
| console.error(`Error getting ${contractName} address for chain ${chainId}:`, error) | ||
| return null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file in question
cat -n packages/utils/src/sablier/contracts.tsRepository: BuilderOSS/nouns-builder
Length of output: 2453
🏁 Script executed:
# Check package.json to see which version of sablier is being used
cat packages/utils/package.json | grep -A 5 -B 5 "sablier"Repository: BuilderOSS/nouns-builder
Length of output: 837
🏁 Script executed:
# Search for where sablier is imported and used
rg "sablier" packages/utils/src/sablier/ -A 3 -B 3Repository: BuilderOSS/nouns-builder
Length of output: 1842
🏁 Script executed:
# Look for node_modules or any type definition files for sablier
fd "sablier" --type d | head -20Repository: BuilderOSS/nouns-builder
Length of output: 95
🏁 Script executed:
# Look for type definitions or d.ts files for sablier
fd "\.d\.ts" | xargs grep -l "sablier" 2>/dev/null | head -10Repository: BuilderOSS/nouns-builder
Length of output: 50
🏁 Script executed:
# Check if there are any other usages of sablier in the codebase
rg "sablier\." --type ts --type tsx -cRepository: BuilderOSS/nouns-builder
Length of output: 95
🏁 Script executed:
# Search for any imports from sablier package
rg "from ['\"]sablier['\"]" -A 5Repository: BuilderOSS/nouns-builder
Length of output: 450
🌐 Web query:
sablier v2.0.1 npm package releases.getLatest contracts.get API
💡 Result:
What “v2.0.1” refers to
In Sablier’s docs, Lockup v2.0 is explicitly labeled as the v2.0 release of @sablier/lockup@2.0.1. (docs.sablier.com)
Also note: Lockup v3.0 is the latest (as of Oct 2025 in the docs). (docs.sablier.com)
contracts.get (from the sablier npm package)
The sablier package documents contracts.get like this: (npmjs.com)
import { releases, sablier } from "sablier";
const lockup = sablier.contracts.get({
name: "SablierLockup",
release: releases.lockup["v2.0"], // Lockup v2.0 (docs: `@sablier/lockup`@2.0.1)
});releases.getLatest
In the public README/API excerpt for the sablier npm package, the documented release query is sablier.releases.getAll({ protocol }) (not releases.getLatest). (npmjs.com)
Because each Release includes an isLatest: boolean, you can derive “latest” like this:
import { sablier } from "sablier";
const latestLockup = sablier.releases
.getAll({ protocol: "lockup" })
.find((r) => r.isLatest);Example: Lockup v2.0 (=@sablier/lockup@2.0.1) known deployment address
For Abstract mainnet, the docs list SablierLockup at:
0x14Eb4AB47B2ec2a71763eaBa202a252E176FAE88 (docs.sablier.com)
Citations:
- 1: https://docs.sablier.com/guides/lockup/previous-deployments/v2.0?utm_source=openai
- 2: https://docs.sablier.com/guides/lockup/deployments?utm_source=openai
- 3: https://www.npmjs.com/package/sablier?utm_source=openai
- 4: https://www.npmjs.com/package/sablier?utm_source=openai
- 5: https://docs.sablier.com/guides/lockup/previous-deployments/v2.0?utm_source=openai
Fix sablier API calls—both method and parameter names are incorrect.
The code uses two methods that don't match the sablier v2.0.1 API:
sablier.releases.getLatest()does not exist. Usesablier.releases.getAll({ protocol: 'lockup' })and filter:.find((r) => r.isLatest)instead.sablier.contracts.get()expects the parametername, notcontractName.
Example:
const latestRelease = sablier.releases.getAll({ protocol: 'lockup' }).find((r) => r.isLatest)
const contract = sablier.contracts.get({
name: contractName, // not contractName
release: latestRelease,
})🤖 Prompt for AI Agents
In `@packages/utils/src/sablier/contracts.ts` around lines 12 - 39, The sablier
API calls are using incorrect methods and parameter names: replace the
non-existent sablier.releases.getLatest() with sablier.releases.getAll({
protocol: 'lockup' }).find(r => r.isLatest) to obtain latestRelease, and change
the sablier.contracts.get call to pass name: contractName (not contractName key)
along with release: latestRelease; also keep the existing null
checks/console.error paths if latestRelease or contract?.address are missing and
ensure the try/catch remains.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/hooks/src/useStreamData.ts`:
- Around line 93-106: The current mapping decodes every log with the
CreateLockupLinearStream signature; first filter the input logs array by the
emitting contract address (e.g., log.address === lockupContractAddress or the
variable you use for the lockup contract) before calling decodeEventLog so
unrelated contracts' events are ignored; update the pipeline around parsedLogs
to apply a .filter(log => log && log.address === <lockup contract address
variable>) prior to the .map that calls decodeEventLog (keep using
createLockupLinearStreamEventAbi and still filter for eventName ===
'CreateLockupLinearStream' after decoding).
In
`@packages/proposal-ui/src/components/ProposalDescription/StreamDetails/StreamItem.tsx`:
- Around line 152-174: The handleCancelAsProposal callback does not catch errors
from addTransaction or onOpenProposalReview; wrap the body of
handleCancelAsProposal in a try/catch block (keeping the early return for
missing lockupLinearAddress/liveData) so any exceptions from
addTransaction(cancelTxnData) or onOpenProposalReview() are caught, then handle
the error consistently with other handlers (e.g., call the same error logging or
user notification used in handleCancelDirect/handleWithdraw) and rethrow or
suppress as appropriate.
In `@packages/ui/src/DecodedTransactions/TransactionDisplay/DecodedDisplay.tsx`:
- Around line 60-99: streamData is computed in the React.useMemo block but never
appended to the payload used for AI summaries, so update the code that builds
transactionData (where escrowData is added) to also include the streamData
object when present; specifically, in the component/function that constructs
transactionData (look for transactionData and escrowData in DecodedDisplay or
the surrounding scope), merge/add streamData: streamData (or streamData ?? null)
so the summary generation/AI payload receives lockupAddress, tokenAddress and
streams extracted by getSablierContracts/useMemo.
In `@packages/utils/src/sablier/streams.ts`:
- Around line 158-179: In calculateStreamTimes, don't set cliffTime to 0 when
durationStream.durations.cliff is 0 (which shows as 1970-01-01 in UIs); instead
assign cliffTime to startTime (or make it nullable) so the UI shows the stream
start rather than Unix epoch—update the cliffTime calculation in the durations
branch (function calculateStreamTimes, variable durationStream and
durations.cliff) to use startTime when cliff is zero.
🧹 Nitpick comments (14)
packages/utils/src/sablier/constants.ts (2)
3-13: Consider an allowlist (or contract‑derived) support check.
A blocklist will treat any new/unknown chain as supported, which can enable Sablier flows where contracts aren’t deployed. A supported‑chain allowlist (or deriving support fromgetSablierContracts) would avoid false positives.
15-185: Verify ABI/event definitions against the deployed LockupLinear release.
Manual ABI copies can drift across releases, which will break decoding/reads if the struct shape or event signature changes. If possible, consider sourcing ABIs from the official Sablier artifacts/package or pinning them to a specific release.packages/utils/src/escrow.ts (1)
76-98: Consider deduplicating legacy/current bundler address maps.
Right nowgetEscrowBundlerandgetEscrowBundlerLegacyduplicate the chain switch. A shared map keyed by{chainId, version}would reduce drift risk when addresses change.packages/hooks/src/useStreamData.ts (1)
142-207: Consider batching contract reads to reduce RPC load.
Each stream currently performs 5 sequential reads; for large batches this can be slow. If the provider supportsmulticall/readContracts, batching would improve latency and reduce RPC overhead.packages/ui/src/DecodedTransactions/ArgumentDisplay/StreamArgumentDisplay.tsx (2)
11-21: Unused prop and loose typing in interface.The
streamDataprop is declared but never used in the component (only noted as "For potential future use"). Additionally,streams: any[]inDecodedStreamBatchcould benefit from stricter typing.Consider defining a Stream type
+interface StreamData { + recipient?: string + sender?: string + depositAmount?: string | bigint + durations?: { total: number; cliff: number } + timestamps?: { start: number; end: number } + cliffTime?: number + cancelable?: boolean + transferable?: boolean +} + export interface DecodedStreamBatch { lockupAddress: string tokenAddress: string - streams: any[] + streams: StreamData[] }
109-112: Potential runtime error withBigIntconversion.If
stream.depositAmountcontains an invalid value (non-numeric string),BigInt(stream.depositAmount)will throw. While the data should come from properly decoded transactions, a defensive try-catch would prevent UI crashes.Add defensive handling
- <Text>{formatAmount(BigInt(stream.depositAmount))}</Text> + <Text> + {(() => { + try { + return formatAmount(BigInt(stream.depositAmount)) + } catch { + return String(stream.depositAmount) + } + })()} + </Text>packages/proposal-ui/src/components/ProposalDescription/StreamDetails/StreamDetails.tsx (2)
100-100: Avoidanytype for stream parameter.Using
anybypasses TypeScript's type checking. The stream should be typed asStreamConfigDurations | StreamConfigTimestampsto match theStreamItemPropsinterface inStreamItem.tsx.♻️ Suggested fix
- <Accordion - items={streamData.streams.map((stream: any, index: number) => { + <Accordion + items={streamData.streams.map((stream: StreamConfigDurations | StreamConfigTimestamps, index: number) => {Add the import at the top of the file:
import { StreamConfigDurations, StreamConfigTimestamps } from '@buildeross/utils/sablier/streams'
104-118: Clarify that StreamItem returns an Accordion item object, not a React element.
StreamItemis invoked as a function returning{ title, description }for the Accordion'sitemsprop. While this works, it's unconventional—most React components return JSX. Consider adding a comment or renaming tocreateStreamItem/getStreamItemConfigto clarify it's a factory function, not a component rendered via JSX.packages/proposal-ui/src/components/ProposalDescription/MilestoneDetails/MilestoneDetails.tsx (2)
99-105: Remove unusedproposalparameter.The
proposalprop is passed toEscrowInstanceand destructured in the component signature but never used. This creates unnecessary prop drilling.♻️ Suggested fix
Remove from interface and component:
interface EscrowInstanceProps { escrow: EscrowInstanceData escrowIndex: number totalEscrows: number - proposal: Proposal onOpenProposalReview: () => Promise<void> hasThreshold: boolean } const EscrowInstance = ({ escrow, escrowIndex, totalEscrows, - proposal, onOpenProposalReview, hasThreshold, }: EscrowInstanceProps) => {And remove from the call site (lines 81):
<EscrowInstance key={escrowIndex} escrow={escrow} escrowIndex={escrowIndex} totalEscrows={escrows.length} - proposal={proposal} onOpenProposalReview={onOpenProposalReview} hasThreshold={hasThreshold} />
200-225: Consider providing user feedback on transaction errors.The
handleReleaseMilestoneDirectfunction catches errors and logs them to the console, but the user receives no feedback when a transaction fails. Consider adding a toast notification or error state to inform users of failures.packages/proposal-ui/src/components/ProposalDescription/StreamDetails/StreamItem.tsx (2)
30-30: Avoidanytype fortokenMetadata.Using
anyloses type safety. Consider defining a proper type or using the type fromuseTokenMetadataSinglereturn value.♻️ Suggested fix
- tokenMetadata?: any + tokenMetadata?: { decimals?: number; symbol?: string; name?: string }
91-99: Consider usingisAddressEqualfrom viem for address comparisons.Using
toLowerCase()for address comparisons works but viem providesisAddressEqualwhich handles checksummed addresses correctly and is more idiomatic.♻️ Suggested improvement
+import { Address, encodeFunctionData, formatUnits, isAddressEqual } from 'viem' - const isRecipient = address && stream.recipient.toLowerCase() === address.toLowerCase() - const isSender = - address && - (liveData?.sender.toLowerCase() === address.toLowerCase() || - stream.sender.toLowerCase() === address.toLowerCase()) - const isSenderTreasury = - addresses.treasury && - (liveData?.sender.toLowerCase() === addresses.treasury.toLowerCase() || - stream.sender.toLowerCase() === addresses.treasury.toLowerCase()) + const isRecipient = address && isAddressEqual(stream.recipient, address) + const isSender = + address && + ((liveData?.sender && isAddressEqual(liveData.sender, address)) || + isAddressEqual(stream.sender, address)) + const isSenderTreasury = + addresses.treasury && + ((liveData?.sender && isAddressEqual(liveData.sender, addresses.treasury)) || + isAddressEqual(stream.sender, addresses.treasury))packages/hooks/src/useInvoiceData.ts (2)
142-147: RedundantBigIntconversion.The
milestoneAmountsare alreadybigint[]fromdecodeFunctionDatadecoding. The.map((x) => BigInt(x))conversion is unnecessary.♻️ Suggested fix
return { invoiceCid: ipfsCid, clientAddress: clientAddress as AddressType, tokenAddress: tokenAddress as AddressType, - milestoneAmounts: milestoneAmounts.map((x) => BigInt(x)), + milestoneAmounts, }
205-222: Consider usingPromise.allSettledfor resilient IPFS fetching.If one IPFS fetch fails,
Promise.allwill reject immediately, potentially losing successfully fetched data from other CIDs. UsingPromise.allSettledwould allow partial success.♻️ Suggested improvement
async ([, ..._invoiceCids]) => { - return Promise.all( + const results = await Promise.allSettled( _invoiceCids.map(async (cid) => { - try { - const text = await fetchFromURI(`ipfs://${cid}`) - return JSON.parse(text) as InvoiceMetadata - } catch (error) { - console.error('Failed to fetch invoice data:', error) - return undefined - } + const text = await fetchFromURI(`ipfs://${cid}`) + return JSON.parse(text) as InvoiceMetadata }) ) + return results.map((result) => { + if (result.status === 'fulfilled') { + return result.value + } + console.error('Failed to fetch invoice data:', result.reason) + return undefined + }) }
| const parsedLogs = logs | ||
| .map((log) => { | ||
| try { | ||
| return decodeEventLog({ | ||
| abi: createLockupLinearStreamEventAbi, | ||
| data: log?.data, | ||
| topics: log?.topics, | ||
| }) | ||
| } catch { | ||
| return null | ||
| } | ||
| }) | ||
| .filter((log) => log !== null && log.eventName === 'CreateLockupLinearStream') | ||
|
|
There was a problem hiding this comment.
Filter logs by lockup contract address before decoding.
decodeEventLog will accept any log with the matching signature. Filtering by log.address prevents unrelated events from being treated as streams.
🛠️ Suggested patch
- const parsedLogs = logs
- .map((log) => {
+ const parsedLogs = logs
+ .filter((log) => toLower(log.address) === toLower(_lockupAddress))
+ .map((log) => {
try {
return decodeEventLog({
abi: createLockupLinearStreamEventAbi,
data: log?.data,
topics: log?.topics,
})🤖 Prompt for AI Agents
In `@packages/hooks/src/useStreamData.ts` around lines 93 - 106, The current
mapping decodes every log with the CreateLockupLinearStream signature; first
filter the input logs array by the emitting contract address (e.g., log.address
=== lockupContractAddress or the variable you use for the lockup contract)
before calling decodeEventLog so unrelated contracts' events are ignored; update
the pipeline around parsedLogs to apply a .filter(log => log && log.address ===
<lockup contract address variable>) prior to the .map that calls decodeEventLog
(keep using createLockupLinearStreamEventAbi and still filter for eventName ===
'CreateLockupLinearStream' after decoding).
| const handleCancelAsProposal = useCallback(async () => { | ||
| if (!lockupLinearAddress || !liveData) return | ||
|
|
||
| const cancelTransaction = { | ||
| target: lockupLinearAddress as AddressType, | ||
| functionSignature: 'cancel(uint256)', | ||
| calldata: encodeFunctionData({ | ||
| abi: lockupLinearAbi, | ||
| functionName: 'cancel', | ||
| args: [liveData.streamId], | ||
| }), | ||
| value: '', | ||
| } | ||
|
|
||
| const cancelTxnData = { | ||
| type: TransactionType.CUSTOM, | ||
| summary: `Cancel Sablier Stream #${liveData.streamId}`, | ||
| transactions: [cancelTransaction], | ||
| } | ||
|
|
||
| addTransaction(cancelTxnData) | ||
| onOpenProposalReview() | ||
| }, [onOpenProposalReview, addTransaction, lockupLinearAddress, liveData]) |
There was a problem hiding this comment.
Add error handling to handleCancelAsProposal.
Unlike handleCancelDirect and handleWithdraw, this callback lacks try/catch. If addTransaction or onOpenProposalReview throws, the error will be unhandled.
🛡️ Suggested fix
const handleCancelAsProposal = useCallback(async () => {
if (!lockupLinearAddress || !liveData) return
+ try {
const cancelTransaction = {
target: lockupLinearAddress as AddressType,
functionSignature: 'cancel(uint256)',
calldata: encodeFunctionData({
abi: lockupLinearAbi,
functionName: 'cancel',
args: [liveData.streamId],
}),
value: '',
}
const cancelTxnData = {
type: TransactionType.CUSTOM,
summary: `Cancel Sablier Stream #${liveData.streamId}`,
transactions: [cancelTransaction],
}
addTransaction(cancelTxnData)
- onOpenProposalReview()
+ await onOpenProposalReview()
+ } catch (error) {
+ console.error('Error creating cancel proposal:', error)
+ }
}, [onOpenProposalReview, addTransaction, lockupLinearAddress, liveData])🤖 Prompt for AI Agents
In
`@packages/proposal-ui/src/components/ProposalDescription/StreamDetails/StreamItem.tsx`
around lines 152 - 174, The handleCancelAsProposal callback does not catch
errors from addTransaction or onOpenProposalReview; wrap the body of
handleCancelAsProposal in a try/catch block (keeping the early return for
missing lockupLinearAddress/liveData) so any exceptions from
addTransaction(cancelTxnData) or onOpenProposalReview() are caught, then handle
the error consistently with other handlers (e.g., call the same error logging or
user notification used in handleCancelDirect/handleWithdraw) and rethrow or
suppress as appropriate.
| // Prepare stream data once | ||
| const streamData = React.useMemo(() => { | ||
| const sablierContracts = getSablierContracts(chainId) | ||
| const isSablierTarget = | ||
| (sablierContracts.batchLockup && | ||
| target.toLowerCase() === sablierContracts.batchLockup.toLowerCase()) || | ||
| (sablierContracts.lockup && | ||
| target.toLowerCase() === sablierContracts.lockup.toLowerCase()) | ||
|
|
||
| if (!isSablierTarget) return null | ||
|
|
||
| // Check if this is a createWithDurationsLL or createWithTimestampsLL function | ||
| if ( | ||
| transaction.functionName !== 'createWithDurationsLL' && | ||
| transaction.functionName !== 'createWithTimestampsLL' | ||
| ) { | ||
| return null | ||
| } | ||
|
|
||
| // Extract calldata - we need to reconstruct it from the transaction | ||
| // The transaction already has decoded args, but we need the raw calldata | ||
| // Since we don't have direct access to calldata here, we'll extract from args | ||
| try { | ||
| // Get lockup, asset, and batch from args | ||
| const lockupArg = transaction.args['lockup'] || transaction.args['_lockup'] | ||
| const assetArg = transaction.args['asset'] || transaction.args['_asset'] | ||
| const batchArg = transaction.args['batch'] || transaction.args['_batch'] | ||
|
|
||
| if (!lockupArg || !assetArg || !batchArg) return null | ||
|
|
||
| return { | ||
| lockupAddress: lockupArg.value as string, | ||
| tokenAddress: assetArg.value as string, | ||
| streams: Array.isArray(batchArg.value) ? batchArg.value : [], | ||
| } | ||
| } catch (e) { | ||
| console.warn('Failed to extract stream data', e) | ||
| return null | ||
| } | ||
| }, [transaction.args, transaction.functionName, target, chainId]) |
There was a problem hiding this comment.
Stream data is computed but not included in the AI summary payload.
streamData is derived (Line 60+) but never added to transactionData, so summaries won’t reflect stream details. Consider passing it through alongside escrowData.
💡 Suggested patch
const transactionData = React.useMemo(() => {
if (isLoadingMetadata || DISABLE_AI_SUMMARY) return null
return {
chainId,
addresses,
transaction: { args: transaction.args, functionName: transaction.functionName },
target: target as `0x${string}`,
tokenMetadata: tokenMetadata || undefined,
nftMetadata: nftMetadata || undefined,
escrowData: escrowData || undefined,
+ streamData: streamData || undefined,
}
}, [
transaction,
target,
chainId,
addresses,
tokenMetadata,
nftMetadata,
escrowData,
+ streamData,
isLoadingMetadata,
])🤖 Prompt for AI Agents
In `@packages/ui/src/DecodedTransactions/TransactionDisplay/DecodedDisplay.tsx`
around lines 60 - 99, streamData is computed in the React.useMemo block but
never appended to the payload used for AI summaries, so update the code that
builds transactionData (where escrowData is added) to also include the
streamData object when present; specifically, in the component/function that
constructs transactionData (look for transactionData and escrowData in
DecodedDisplay or the surrounding scope), merge/add streamData: streamData (or
streamData ?? null) so the summary generation/AI payload receives lockupAddress,
tokenAddress and streams extracted by getSablierContracts/useMemo.
| export function calculateStreamTimes( | ||
| stream: StreamConfigDurations | StreamConfigTimestamps, | ||
| isDurationsMode: boolean, | ||
| creationTimestamp?: number | ||
| ): { startTime: number; cliffTime: number; endTime: number } { | ||
| if (!isDurationsMode) { | ||
| const timestampStream = stream as StreamConfigTimestamps | ||
| return { | ||
| startTime: timestampStream.timestamps.start, | ||
| cliffTime: timestampStream.cliffTime, | ||
| endTime: timestampStream.timestamps.end, | ||
| } | ||
| } | ||
|
|
||
| const durationStream = stream as StreamConfigDurations | ||
| const startTime = creationTimestamp || Math.floor(Date.now() / 1000) | ||
| const cliffTime = | ||
| durationStream.durations.cliff > 0 ? startTime + durationStream.durations.cliff : 0 | ||
| const endTime = startTime + durationStream.durations.total | ||
|
|
||
| return { startTime, cliffTime, endTime } | ||
| } |
There was a problem hiding this comment.
Cliff time shouldn’t default to Unix epoch when there is no cliff.
Returning 0 can render as “1970‑01‑01” in UIs. Using startTime (or a nullable value) avoids misleading dates.
💡 Suggested patch
- const cliffTime =
- durationStream.durations.cliff > 0 ? startTime + durationStream.durations.cliff : 0
+ const cliffTime =
+ durationStream.durations.cliff > 0
+ ? startTime + durationStream.durations.cliff
+ : startTime🤖 Prompt for AI Agents
In `@packages/utils/src/sablier/streams.ts` around lines 158 - 179, In
calculateStreamTimes, don't set cliffTime to 0 when
durationStream.durations.cliff is 0 (which shows as 1970-01-01 in UIs); instead
assign cliffTime to startTime (or make it nullable) so the UI shows the stream
start rather than Unix epoch—update the cliffTime calculation in the durations
branch (function calculateStreamTimes, variable durationStream and
durations.cliff) to use startTime when cliff is zero.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.