-
Notifications
You must be signed in to change notification settings - Fork 1
Feat/tenderly simulations #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds Tenderly-based simulation support: new env vars, backend routes for contract info and simulation (with validation and rate limiting), Tenderly utilities, a hook for contract info, Lists context simulation integration, UI updates (simulate action, Tenderly link, dynamic ABI feedback), and a small dependency bump. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI
participant ListsContext
participant API
participant Tenderly
Note over UI,ListsContext: User initiates simulation flow
User->>UI: Click "Simulate" (listId)
UI->>ListsContext: simulateList(listId)
ListsContext->>API: POST /api/simulate { networkId, governorAddress, transactions }
API->>Tenderly: POST /encode_state (build state override)
Tenderly-->>API: encoded state
API->>Tenderly: POST /simulate (with encoded state)
Tenderly-->>API: { simulationId }
API->>ListsContext: { status: true, simulationLink }
ListsContext->>UI: simulation result
UI->>User: show "Inspect on Tenderly" link / success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (1)
Comment |
✅ Deploy Preview for kleros-governor-v2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (7)
web/.env.example (1)
4-10: Tighten.envhygiene (ordering, quoting & trailing newline)Minor nits picked up by
dotenv-linter:
- Keys are currently out of alphabetical order – helps merge-conflict hygiene.
- Placeholder values don’t need to be wrapped in quotes; keeps the file free of superfluous chars.
- Missing terminal newline.
ALLOWED_ORIGINSwill be parsed as a single entry; if multiple origins are expected prefer a comma-separated list and document it in a comment.-export NEXT_PUBLIC_COURT_SITE="https://dev--kleros-v2-testnet.netlify.app/#" -# tenderly simulations -export TENDERLY_ACCOUNT_NAME="test" -export TENDERLY_PROJECT_NAME="governor-test" -export TENDERLY_ACCESS_KEY="abcabcabcabcabcabc" -export ALLOWED_ORIGINS="http://localhost:3000" +# tenderly simulations +export ALLOWED_ORIGINS=http://localhost:3000 +export TENDERLY_ACCOUNT_NAME=test +export TENDERLY_PROJECT_NAME=governor-test +export TENDERLY_ACCESS_KEY=abcabcabcabcabcabc +Not blocking, but adopting the linter suggestions keeps the file clean and predictable.
web/src/components/ExternalLink.tsx (1)
1-6: Slim down & make the link truly externalA couple of quick wins:
Reactimport is redundant in a Next 13/14 project using the new JSX runtime.next/linkis optimised for internal routing; for external links an<a>avoids unnecessary prefetching.
– If you keepLink, addprefetch={false}to suppress the prefetch.- Consider an
aria-labelfor better accessibility.-import React, { HTMLAttributes } from "react"; - -import clsx from "clsx"; -import Link from "next/link"; +import { HTMLAttributes } from "react"; +import clsx from "clsx"; @@ - <Link - href={url} - className={clsx("block w-fit text-center hover:brightness-[1.2]", className)} - target="_blank" - rel="noreferrer noopener" + <a + href={url} + className={clsx("block w-fit text-center hover:brightness-[1.2]", className)} + target="_blank" + rel="noreferrer noopener" + aria-label={text} > @@ - </Link> + </a>Entirely optional, but keeps bundle size and runtime behaviour tidy.
Also applies to: 14-24
web/src/utils/txnBuilder/constructSubmissionData.ts (1)
13-20: String concatenation & BigInt parsing could be simplified and hardened
titlesis manually concatenated inside the loop.Array.prototype.join()is clearer and O(n) instead of the current O(n²) string-copy pattern.
BigInt(txn.txnValue)will throw iftxnValueis not a strict base-10 integer string.
• If you ever accept"0x…"or decimal‐float strings, wrap the cast with validation or pass a radix.
• Consider returning a descriptive error instead of a hard crash.Example refactor:
-const addresses: Address[] = []; -const values: bigint[] = []; -let data: Hex = "0x"; -const dataSizes: bigint[] = []; -let titles = ""; - -transactions.forEach((txn) => { - addresses.push(txn.to); - values.push(BigInt(txn.txnValue)); - dataSizes.push(BigInt(txn.data.slice(2).length / 2)); - titles += `${txn.name},`; - data += txn.data.slice(2); -}); - -titles = titles.replace(/,$/, ""); // trim trailing comma +const addresses = transactions.map((t) => t.to); +const values = transactions.map((t) => BigInt(t.txnValue)); +const dataSizes = transactions.map((t) => BigInt(t.data.length / 2 - 1)); // skip 0x +const titles = transactions.map((t) => t.name).join(","); +const data = transactions.reduce((acc, t) => acc + t.data.slice(2), "0x");web/src/context/NewListsContext.tsx (1)
60-63: UnusedpublicClient– remove or leverage
usePublicClientis fetched but never used except for the undefined-check shortcut. If Tenderly does not rely on it, drop the hook to reduce re-renders; otherwise actually forward the livechainIdinstead of the constant default (see next comment).web/src/utils/tenderly/simulateWithTenderly.ts (1)
24-28: Declare explicit return type forsimulateWithTenderlyThe function currently relies on inference and effectively returns
any.
Explicitly declaringPromise<SimulationResult>makes the compiler catch mismatched shapes coming back from Tenderly.-export const simulateWithTenderly = async ( +export const simulateWithTenderly = async ( networkId: number, governorAddress: Address, transactions: ListTransaction[] -) => { +): Promise<SimulationResult> => {web/src/utils/tenderly/encodeState.ts (1)
62-69: Normalise governor address casing onceTo avoid key-matching issues across the codebase, convert addresses to lowercase (or checksum) at the point of building the object:
- [governorAddress]: { + [governorAddress.toLowerCase()]: {Do the same for the map lookup when parsing the response.
web/src/utils/simulateRouteUtils.ts (1)
68-90: In-memory rate-limit map leaks over time
ipRequestMapgrows unbounded with new IPs and never evicts old entries.
Add a clean-up step after each request:// Reset count if the window has passed if (now > rateData.resetTime) { rateData.count = 0; rateData.resetTime = now + RATE_LIMIT_WINDOW; } + +// GC old entries once per call +for (const [key, { resetTime }] of ipRequestMap) { + if (now > resetTime) ipRequestMap.delete(key); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
web/src/assets/svgs/icons/link-arrow.svgis excluded by!**/*.svgweb/src/assets/svgs/logos/tenderly.svgis excluded by!**/*.svg
📒 Files selected for processing (13)
web/.env.example(1 hunks)web/src/app/(main)/governor/[governorAddress]/MyLists/AddTxnModal.tsx(1 hunks)web/src/app/(main)/governor/[governorAddress]/MyLists/Header.tsx(1 hunks)web/src/app/(main)/governor/[governorAddress]/MyLists/Lists.tsx(4 hunks)web/src/app/(main)/governor/[governorAddress]/MyLists/SubmissionButton.tsx(1 hunks)web/src/app/(main)/governor/[governorAddress]/MyLists/index.tsx(1 hunks)web/src/app/api/simulate/route.ts(1 hunks)web/src/components/ExternalLink.tsx(1 hunks)web/src/context/NewListsContext.tsx(4 hunks)web/src/utils/simulateRouteUtils.ts(1 hunks)web/src/utils/tenderly/encodeState.ts(1 hunks)web/src/utils/tenderly/simulateWithTenderly.ts(1 hunks)web/src/utils/txnBuilder/constructSubmissionData.ts(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
web/.env.example
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_COURT_SITE key should go before the NEXT_PUBLIC_REOWN_PROJECT_ID key
[warning] 7-7: [QuoteCharacter] The value has quote characters (', ")
[warning] 8-8: [QuoteCharacter] The value has quote characters (', ")
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
[warning] 9-9: [UnorderedKey] The TENDERLY_ACCESS_KEY key should go before the TENDERLY_ACCOUNT_NAME key
[warning] 10-10: [EndingBlankLine] No blank line at the end of the file
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
[warning] 10-10: [UnorderedKey] The ALLOWED_ORIGINS key should go before the TENDERLY_ACCESS_KEY key
🔇 Additional comments (6)
web/src/app/(main)/governor/[governorAddress]/MyLists/AddTxnModal.tsx (1)
8-10: Correct context import – looks goodThe switch to
NewListsContextaligns this component with the updated provider and simulation features. No further issues spotted in this hunk.web/src/app/(main)/governor/[governorAddress]/MyLists/index.tsx (1)
5-5: Import path update is accurateImporting
ListsProviderfrom the new context avoids the typo in the previous path and enables simulation state – all good.web/src/app/(main)/governor/[governorAddress]/MyLists/SubmissionButton.tsx (1)
9-10: Context path fix acknowledgedThe updated import resolves the casing typo and connects the component to the new lists API – no concerns.
web/src/app/(main)/governor/[governorAddress]/MyLists/Header.tsx (1)
4-4: Import path update looks correctThe switch to
NewListsContextaligns this component with the new simulation-aware provider. No further issues spotted.web/src/app/api/simulate/route.ts (1)
50-57: Hard dependency onsimulationResult.simulation.idwithout null-checking
simulateWithTenderlycan fail silently and return{error: …}. Guard against this to avoidCannot read property 'id' of undefined'.- if (!isUndefined(simulationResult.simulation.id)) { + if (simulationResult?.simulation?.id) {web/src/context/NewListsContext.tsx (1)
170-171:Array.prototype.toReversed()is still Stage 3Node 18 / many browsers don’t implement it yet. Use
slice().reverse()for wider compatibility or ensure the target runtime supports ES2023 array methods.- lists: [...lists.values()].toReversed(), + lists: [...lists.values()].slice().reverse(),
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
web/src/context/NewListsContext.tsx (3)
61-61: GlobalisSimulatingstate couples all listsThe global simulation state still prevents concurrent simulations of different lists, which can degrade user experience when managing multiple lists.
Also applies to: 183-183
76-83:Content-Typeheader missing in POST requestThe POST request is still missing the required Content-Type header, which can cause issues with gateways, proxies, or CORS pre-flight checks.
79-79: Chain mismatch – hard-codingDEFAULT_CHAIN.idignores current networkThe code still hard-codes
DEFAULT_CHAIN.idinstead of using the dynamicchainIdvalue, which can produce incorrect simulations when users are connected to a different chain.
🧹 Nitpick comments (2)
web/src/context/NewListsContext.tsx (2)
105-105: RemovepublicClientfrom dependency array to prevent unnecessary re-rendersIncluding
publicClientin the dependency array causes thesimulateListfunction to be recreated whenever the client changes, which can happen frequently and cause performance issues.-[lists, governorAddress, publicClient] +[lists, governorAddress]
95-95: Improve error logging for debuggingThe current error logging doesn't provide enough context for debugging simulation failures in production environments.
-console.log(error instanceof Error ? error.message : `Simulation Failed`); +console.error('Simulation failed for list:', listId, error instanceof Error ? error.message : 'Unknown error', error);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/app/(main)/governor/[governorAddress]/MyLists/Lists.tsx(4 hunks)web/src/context/NewListsContext.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/app/(main)/governor/[governorAddress]/MyLists/Lists.tsx
🔇 Additional comments (1)
web/src/context/NewListsContext.tsx (1)
67-67:simulationsMap recreated on every renderThe
useMemowith an empty dependency array still creates a new Map instance on every render, defeating its purpose and causing unnecessary re-renders of consuming components.-const simulations = useMemo(() => new Map<string, string>(), []); +const [simulations] = useState(() => new Map<string, string>());Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
web/.env.example (2)
6-11: Add documentation comments for new environment variables.Consider adding comments above each new environment variable section to explain their purpose and usage, especially for sensitive keys like API keys.
+# Tenderly integration for transaction simulations export TENDERLY_ACCOUNT_NAME="test" export TENDERLY_PROJECT_NAME="governor-test" export TENDERLY_ACCESS_KEY="abcabcabcabcabcabc" + +# CORS configuration for API routes export ALLOWED_ORIGINS="http://localhost:3000" + +# Etherscan API for contract verification fallback export ETHERSCAN_API_KEY="abcabcabcabcabcabc" +
11-11: Add blank line at end of file.The file should end with a blank line for consistent formatting.
export ETHERSCAN_API_KEY="abcabcabcabcabcabc" +web/src/hooks/useContractInfo.ts (1)
24-25: Consider finite staleTime for better data freshness.While contract ABIs rarely change, using
staleTime: Infinitycould lead to stale data issues if a contract is upgraded or re-verified. Consider using a longer but finite staleTime.- staleTime: Infinity, + staleTime: 60 * 60 * 1000, // 1 hour gcTime: 24 * 60 * 60 * 1000,web/src/app/api/contract/route.ts (1)
96-96: Fix variable naming inconsistency.The variable is named
arbiscanDatabut contains data from Etherscan API.- const arbiscanData = await response.json(); + const etherscanData = await response.json(); - if (arbiscanData.status !== "1" || !arbiscanData.result) { + if (etherscanData.status !== "1" || !etherscanData.result) { return NextResponse.json({ error: "Contract not verified on Etherscan" }, { status: 404 }); } - const abi = JSON.parse(arbiscanData.result); + const abi = JSON.parse(etherscanData.result);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
web/.env.example(1 hunks)web/src/app/(main)/governor/[governorAddress]/MyLists/AddTxnModal.tsx(4 hunks)web/src/app/(main)/governor/[governorAddress]/MyLists/JsonInput.tsx(2 hunks)web/src/app/api/contract/route.ts(1 hunks)web/src/app/api/simulate/route.ts(1 hunks)web/src/context/NewListsContext.tsx(4 hunks)web/src/hooks/useContractInfo.ts(1 hunks)web/src/utils/simulateRouteUtils.ts(1 hunks)web/src/utils/tenderly/encodeState.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- web/src/app/api/simulate/route.ts
- web/src/utils/simulateRouteUtils.ts
- web/src/utils/tenderly/encodeState.ts
- web/src/context/NewListsContext.tsx
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
web/.env.example
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_COURT_SITE key should go before the NEXT_PUBLIC_REOWN_PROJECT_ID key
[warning] 7-7: [QuoteCharacter] The value has quote characters (', ")
[warning] 8-8: [QuoteCharacter] The value has quote characters (', ")
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
[warning] 9-9: [UnorderedKey] The TENDERLY_ACCESS_KEY key should go before the TENDERLY_ACCOUNT_NAME key
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
[warning] 10-10: [UnorderedKey] The ALLOWED_ORIGINS key should go before the TENDERLY_ACCESS_KEY key
[warning] 11-11: [EndingBlankLine] No blank line at the end of the file
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
[warning] 11-11: [UnorderedKey] The ETHERSCAN_API_KEY key should go before the TENDERLY_ACCESS_KEY key
🔇 Additional comments (5)
web/src/app/(main)/governor/[governorAddress]/MyLists/JsonInput.tsx (1)
95-97: LGTM! Clean implementation of dynamic ABI prop handling.The useEffect implementation correctly handles the optional ABI prop and integrates well with the existing
handleAbiChangelogic. The dependency array and undefined check are appropriate.web/src/app/(main)/governor/[governorAddress]/MyLists/AddTxnModal.tsx (2)
36-45: LGTM! Well-structured dynamic UI feedback implementation.The
useMemoimplementation provides clean, efficient UI state management based on contract info loading states. The logic correctly handles loading, success, and default states for user feedback.
133-135: Good integration of dynamic contract validation feedback.The TextField enhancements provide real-time feedback to users about contract verification status, improving the user experience during transaction creation.
web/src/hooks/useContractInfo.ts (1)
30-42: LGTM! Robust error handling and API integration.The fetch implementation includes proper error checking, response validation, and detailed error logging. The error propagation allows React Query to handle retry logic appropriately.
web/src/app/api/contract/route.ts (1)
30-74: LGTM! Well-structured API route with proper error handling.The main GET handler correctly implements rate limiting, environment variable validation, and graceful fallback logic. The error handling and logging are comprehensive.
The base branch was changed.
Summary by CodeRabbit
New Features
Chores