feat(transactions): rich decoded view for Smart Invoice escrow deploys - #995
feat(transactions): rich decoded view for Smart Invoice escrow deploys#995sktbrd wants to merge 3 commits into
Conversation
deployEscrow calls to the known escrow bundlers aren't recognized by the generic ABI-fetch decoder, so proposal escrow txs (e.g. milestone payouts) render as a wall of 'Raw only' calldata. Detect known bundler addresses and decode the calldata locally (parseEscrowDeploy, pure + unit-tested) into a rich card: milestone breakdown with proportion bars + total, client/provider/resolver identities (ENS-resolved), token symbol, and safety-valve date. The raw-calldata toggle is preserved. No change to the shared decode hook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@sktbrd is attempting to deploy a commit to the Nouns Builder Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds Smart Invoice escrow deployment decoding for v2 and legacy bundlers. The transaction display detects escrow calldata and renders a card with token amounts, milestones, parties, payouts, resolver data, and safety-valve dates. ChangesEscrow transaction flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TransactionDisplay
participant EscrowTransactionCard
participant parseEscrowDeploy
participant TokenMetadata
TransactionDisplay->>EscrowTransactionCard: pass chainId, target, and calldata
EscrowTransactionCard->>parseEscrowDeploy: decode escrow deployment
parseEscrowDeploy-->>EscrowTransactionCard: return ParsedEscrowDeploy
EscrowTransactionCard->>TokenMetadata: load token metadata
TokenMetadata-->>EscrowTransactionCard: return token decimals and symbol
EscrowTransactionCard-->>TransactionDisplay: render escrow details
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/ui/src/DecodedTransactions/EscrowTransactionCard/EscrowTransactionCard.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/ui/src/DecodedTransactions/EscrowTransactionCard/escrowDeploy.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. packages/ui/src/DecodedTransactions/EscrowTransactionCard/escrowDeploy.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/DecodedTransactions/TransactionDisplay/TransactionDisplay.tsx (1)
120-141: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFall back to raw calldata when escrow parsing fails.
matchEscrowBundlervalidates onlydecoded.target. A known bundler can receive a different function call or malformed hexadecimal calldata. In that case,showEscrowCardis true, butEscrowTransactionCardreturnsnull, so the default display is empty.Require
parseEscrowDeployto succeed before selecting the escrow-card branch. Otherwise, renderRawCalldataDisplay.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/DecodedTransactions/TransactionDisplay/TransactionDisplay.tsx` around lines 120 - 141, Update the escrow-card selection in TransactionDisplay to require successful parseEscrowDeploy validation in addition to matchEscrowBundler and the existing raw-calldata checks. When parsing fails, bypass EscrowTransactionCard and preserve the fallback branch that renders RawCalldataDisplay.
🧹 Nitpick comments (1)
packages/ui/src/DecodedTransactions/EscrowTransactionCard/escrowDeploy.test.ts (1)
69-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a legacy deployment parsing test.
The legacy test only verifies bundler matching. It does not encode
deployEscrowAbiLegacycalldata or assert decoded milestones, funding, and escrow fields. Add this test to protect the legacy positional-argument branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/DecodedTransactions/EscrowTransactionCard/escrowDeploy.test.ts` around lines 69 - 101, Add a legacy deployment case to the parseEscrowDeploy test suite by encoding calldata with deployEscrowAbiLegacy and invoking parseEscrowDeploy with it. Assert the legacy positional-argument branch decodes milestone amounts and total, fund amount, provider, escrow parties, token, termination time, and recipient addresses using the expected fixtures, while retaining the existing bundler-mismatch coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/DecodedTransactions/EscrowTransactionCard/escrowDeploy.ts`:
- Line 84: Update the escrow deploy decoding flow around decodeEscrowData so
invalid nested escrow data returns null instead of constructing a partial
ParsedEscrowDeploy from an empty object. Validate the required decoded fields or
propagate a decoder error signal before the return, and add a regression test
covering ABI-valid outer calldata with malformed escrowData.
In
`@packages/ui/src/DecodedTransactions/EscrowTransactionCard/EscrowTransactionCard.tsx`:
- Around line 48-59: Update EscrowTransactionCard’s amount-rendering flow so
decimal token amounts are not formatted or displayed until
tokenMetadata?.decimals is available; remove the 18-decimal fallback. Preserve
the existing raw-calldata amount fallback for missing metadata, while continuing
to use tokenMetadata decimals and symbol when metadata exists.
---
Outside diff comments:
In
`@packages/ui/src/DecodedTransactions/TransactionDisplay/TransactionDisplay.tsx`:
- Around line 120-141: Update the escrow-card selection in TransactionDisplay to
require successful parseEscrowDeploy validation in addition to
matchEscrowBundler and the existing raw-calldata checks. When parsing fails,
bypass EscrowTransactionCard and preserve the fallback branch that renders
RawCalldataDisplay.
---
Nitpick comments:
In
`@packages/ui/src/DecodedTransactions/EscrowTransactionCard/escrowDeploy.test.ts`:
- Around line 69-101: Add a legacy deployment case to the parseEscrowDeploy test
suite by encoding calldata with deployEscrowAbiLegacy and invoking
parseEscrowDeploy with it. Assert the legacy positional-argument branch decodes
milestone amounts and total, fund amount, provider, escrow parties, token,
termination time, and recipient addresses using the expected fixtures, while
retaining the existing bundler-mismatch coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c2bb7b71-3ff3-4e0c-b4ab-9793ee304620
📒 Files selected for processing (6)
packages/ui/src/DecodedTransactions/EscrowTransactionCard/EscrowTransactionCard.css.tspackages/ui/src/DecodedTransactions/EscrowTransactionCard/EscrowTransactionCard.tsxpackages/ui/src/DecodedTransactions/EscrowTransactionCard/escrowDeploy.test.tspackages/ui/src/DecodedTransactions/EscrowTransactionCard/escrowDeploy.tspackages/ui/src/DecodedTransactions/EscrowTransactionCard/index.tspackages/ui/src/DecodedTransactions/TransactionDisplay/TransactionDisplay.tsx
| const tokenAddress = parsed?.escrow.tokenAddress as Address | undefined | ||
| const { tokenMetadata } = useTokenMetadataSingle(chainId, tokenAddress) | ||
|
|
||
| if (!parsed) return null | ||
|
|
||
| const decimals = tokenMetadata?.decimals ?? 18 | ||
| const symbol = | ||
| tokenMetadata?.symbol ?? (tokenAddress ? walletSnippet(tokenAddress) : '') | ||
| const fmt = (v: bigint) => | ||
| `${Number(formatUnits(v, decimals)).toLocaleString(undefined, { | ||
| maximumFractionDigits: 4, | ||
| })} ${symbol}`.trim() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not assume 18 token decimals.
When tokenMetadata is unavailable, decimals defaults to 18. A 6-decimal escrow token then renders its amount 10^12 times too small until metadata arrives, or permanently if metadata loading fails.
Do not render decimal token amounts until the token decimals are known. Preserve the raw-calldata fallback when metadata is unavailable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/ui/src/DecodedTransactions/EscrowTransactionCard/EscrowTransactionCard.tsx`
around lines 48 - 59, Update EscrowTransactionCard’s amount-rendering flow so
decimal token amounts are not formatted or displayed until
tokenMetadata?.decimals is available; remove the 18-decimal fallback. Preserve
the existing raw-calldata amount fallback for missing metadata, while continuing
to use tokenMetadata decimals and symbol when metadata exists.
Per feedback, three separate per-milestone bars read like progress indicators. Replace them with a single allocation bar split into one segment per milestone (distinct green shades), plus a labeled amount list with matching colour dots.
…sume 18 decimals - parseEscrowDeploy returns null when the nested escrow data decodes empty (no client/resolver), so the raw calldata view shows instead of a blank card. - EscrowTransactionCard no longer defaults to 18 decimals for an arbitrary token: use live metadata, fall back to 18 only for the chain's wrapped native (reliably 18), and show a placeholder otherwise — a 6-decimal USDC escrow no longer renders 1e12 too small before metadata resolves.
1 participant
Description
deployEscrowcalls to the Smart Invoice escrow bundlers aren't recognized by the generic ABI-fetch decoder, so proposal escrow transactions (milestone payouts, e.g. this proposal's) render as a wall of "Raw only" calldata — proposers can't see what they're funding.This detects known escrow bundler addresses and decodes the calldata locally into a rich card: the milestone breakdown with proportion bars + total, the client / provider / resolver as ENS-resolved identities, the escrow token symbol, and the safety-valve date. The raw-calldata toggle is preserved, and the shared decode hook is untouched.
Result
The same escrow that previously showed as raw hex, now:
Implementation
EscrowTransactionCard/escrowDeploy.ts— pure, deterministic decode:matchEscrowBundler(known v2/legacy bundler addresses) +parseEscrowDeploy(decodeFunctionDatawith the existingdeployEscrowAbi→ milestones, total, parties viadecodeEscrowData). No network, no side effects.EscrowTransactionCard/escrowDeploy.test.ts— 8 unit tests (round-trip encode→decode: bundler matching, milestone totals, party/token extraction, negative cases).EscrowTransactionCard/EscrowTransactionCard.tsx(+.css.ts) — the rich view; token symbol viauseTokenMetadataSingle, identities viauseEnsData.TransactionDisplay.tsx— 3-line change: when the target is a known escrow bundler, render the card (raw toggle preserved). No change touseDecodedTransactions.Verification
type-checkclean,lintclean (0 errors), 8/8 tests pass forpackages/ui.Bonus contribution from the Gnars team, alongside Prop 61 Milestone 3.
Summary by CodeRabbit