Update engine API #7427
Update engine API #7427
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: e58d5f7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis update refactors the engine client and SDK to align with a revised API specification, introducing new and restructured TypeScript types, client utilities, and endpoint functions. Execution and signing options are unified, error handling is enhanced, and new analytics and activity log endpoints are added. Supporting modules and configuration files are reorganized and expanded for improved type safety and maintainability. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant EngineClient
participant API
App->>EngineClient: call writeContract(options)
EngineClient->>API: POST /v1/write/contract (with options, headers)
API-->>EngineClient: Response (success or error)
EngineClient-->>App: Result
App->>EngineClient: call getTransactionAnalytics(options)
EngineClient->>API: POST /v1/transactions/analytics (with filters, headers)
API-->>EngineClient: Analytics Data
EngineClient-->>App: Analytics Result
App->>EngineClient: call signMessage(options)
EngineClient->>API: POST /v1/sign/message (with signingOptions, headers)
API-->>EngineClient: Signature Result
EngineClient-->>App: Signature
Possibly related PRs
Suggested reviewers
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
joaquim-verges
changed the title
joaquim-verges
force-pushed
the
Switch_Engine_API_endpoint_to_localhost_for_development
branch
from
f19431b to
d745795
Compare
![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
🧹 Nitpick comments (4)
packages/thirdweb/src/engine/server-wallet.ts (1)
152-156: Remove unnecessary type casting.The type casting of
executionOptionstoExecutionOptions | undefinedappears redundant since it's already typed in the function parameter.- const getExecutionOptions = (chainId: number): ExecutionOptions => { - const options: ExecutionOptions | undefined = executionOptions as - | ExecutionOptions - | undefined; + const getExecutionOptions = (chainId: number): ExecutionOptions => { + const options = executionOptions;packages/engine/src/client/types.gen.ts (3)
20-60: Consider using branded types for better type safetyWhile using
stringforAddressDef,BytesDef, andU256Defis functional, consider using branded types or template literal types for better compile-time validation:type AddressDef = `0x${string}` & { readonly __brand: "Address" }; type BytesDef = `0x${string}` & { readonly __brand: "Bytes" };This would prevent accidental type mixing and provide better IntelliSense support.
Also applies to: 952-952
921-921: Consider more specific types instead ofunknownThe use of
unknownformessageandtypesfields inTypedDataDefand for theValuetype reduces type safety. Consider using more specific types or generics to maintain type information:export type TypedDataDef<TMessage = unknown, TTypes = unknown> = { message: TMessage; types: TTypes; // ... };Also applies to: 928-929, 954-954
1633-1635: Clarify the baseUrl type union patternThe
baseUrltype uses an unusual pattern:"http://localhost:3009" | (string & {}). This allows any string while providing IntelliSense for the localhost option. Consider using a more explicit pattern or adding a comment explaining this TypeScript idiom for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
packages/engine/openapi-ts.config.ts(1 hunks)packages/engine/package.json(2 hunks)packages/engine/src/client/client.gen.ts(2 hunks)packages/engine/src/client/sdk.gen.ts(1 hunks)packages/engine/src/client/types.gen.ts(1 hunks)packages/thirdweb/src/engine/create-server-wallet.ts(1 hunks)packages/thirdweb/src/engine/get-status.ts(1 hunks)packages/thirdweb/src/engine/list-server-wallets.ts(1 hunks)packages/thirdweb/src/engine/search-transactions.ts(1 hunks)packages/thirdweb/src/engine/server-wallet.test.ts(3 hunks)packages/thirdweb/src/engine/server-wallet.ts(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.@(ts|tsx)`: Accept a typed 'props' object and export a named function (e.g., export function MyComponent()). Combine class names via 'cn', expose 'className' prop if useful. ...
**/*.@(ts|tsx): Accept a typed 'props' object and export a named function (e.g., export function MyComponent()).
Combine class names via 'cn', expose 'className' prop if useful.
Reuse core UI primitives; avoid re-implementing buttons, cards, modals.
Local state or effects live inside; data fetching happens in hooks.
Merge class names with 'cn' from '@/lib/utils' to keep conditional logic readable.
Stick to design-tokens: background ('bg-card'), borders ('border-border'), muted text ('text-muted-foreground') etc.
Use the 'container' class with a 'max-w-7xl' cap for page width consistency.
Spacing utilities ('px-', 'py-', 'gap-*') are preferred over custom margins.
Responsive helpers follow mobile-first ('max-sm', 'md', 'lg', 'xl').
Never hard-code colors – always go through Tailwind variables.
Tailwind CSS is the styling system – avoid inline styles or CSS modules.
Prefix files with 'import "server-only";' so they never end up in the client bundle (for server-only code).
packages/thirdweb/src/engine/create-server-wallet.tspackages/thirdweb/src/engine/list-server-wallets.tspackages/thirdweb/src/engine/get-status.tspackages/thirdweb/src/engine/search-transactions.tspackages/engine/src/client/client.gen.tspackages/engine/openapi-ts.config.tspackages/thirdweb/src/engine/server-wallet.test.tspackages/thirdweb/src/engine/server-wallet.tspackages/engine/src/client/sdk.gen.tspackages/engine/src/client/types.gen.ts
🧬 Code Graph Analysis (1)
packages/engine/src/client/client.gen.ts (1)
packages/engine/src/client/types.gen.ts (1)
ClientOptions(1633-1635)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (17)
packages/thirdweb/src/engine/server-wallet.test.ts (2)
37-43: Good: Explicit domain configuration for local testing.The explicit configuration of domain endpoints is well-structured and clearly supports local development testing against the localhost engine instance.
52-52: Address the skipped test case.The test "should create a server wallet" is being skipped, which could indicate a temporary issue or incomplete implementation. Consider either fixing the underlying issue or adding a comment explaining why the test is disabled.
Should this test remain skipped permanently, or is there a plan to re-enable it? If it's a known issue, consider adding a TODO comment explaining the reason for skipping.
packages/thirdweb/src/engine/create-server-wallet.ts (1)
50-50: LGTM: Response structure updated to use HTTP status codes.The change to access response data under the
201status code key aligns with REST API conventions for create operations and matches the pattern being applied across the codebase.packages/thirdweb/src/engine/list-server-wallets.ts (1)
39-39: LGTM: Consistent response structure update.The change to access response data under the
200status code key is appropriate for a list operation and maintains consistency with the updated API response structure.packages/thirdweb/src/engine/get-status.ts (1)
102-102: LGTM: Response structure consistently updated.The change to access transaction data under the
200status code key maintains consistency with the updated API response structure across all engine functions.packages/thirdweb/src/engine/search-transactions.ts (1)
125-125: LGTM! Response data access updated correctly.The change to access data under the
[200]status code key aligns with the new API response format consistently applied across the engine client.packages/engine/src/client/client.gen.ts (1)
25-27: Verify deployment configuration for production environments.The baseUrl change to
http://localhost:3009enables local development. However, ensure that production deployments properly override this default value to avoid connection issues.packages/engine/package.json (1)
1-61: LGTM! Package.json reorganization improves structure.The field reordering follows common conventions and the new
build:generatescript properly integrates OpenAPI generation into the build workflow.packages/thirdweb/src/engine/server-wallet.ts (2)
199-221: Transaction handling updates look good.The changes correctly implement:
- Migration to the new
writeTransactionAPI- Proper default values for optional transaction parameters
- Correct status code key
[202]for accepted responses
327-348: Message signing updates implemented correctly.The changes properly implement:
- Parameter rename from
messageFormattoformat- New
signingOptionsstructure supporting both EOA and smart account signing- Correct status code key
[200]for successful responsesAlso applies to: 361-368
packages/engine/src/client/sdk.gen.ts (1)
1-427: Auto-generated SDK updates align with API specification.The comprehensive reorganization correctly implements:
- New API endpoint mappings with descriptive function names
- Consistent client instance handling with fallback to default
- Proper Content-Type headers on all POST requests
- Security configuration for API key authentication
The changes follow the expected patterns for the updated Engine API.
packages/engine/src/client/types.gen.ts (6)
3-18: Good improvement makingoperationfield mandatoryMaking the
operationfield required inTransactionsFilterValueimproves type safety by ensuring all filters explicitly specify their combination logic. The recursive structure withTransactionsFilterNestedprovides good flexibility for complex queries.
32-48: Excellent documentation for execution optionsThe detailed documentation explaining execution strategy inference, version defaults, and field relationships is extremely helpful. The clear explanation of how the system determines optimal execution paths based on provided values will greatly assist developers.
Also applies to: 370-405
97-168: Well-structured error types for contract interactionsThe
ContractInteractionErrorKindtype provides comprehensive error categorization with appropriate context for each error variant. This discriminated union pattern will make error handling more precise and debugging easier.
988-1008: Verify the HTTP status code key pattern is intentionalThe response types consistently use numeric HTTP status codes as object keys (e.g.,
200: {...}). While this allows different response shapes per status code, it's an unusual pattern that differs from typical response wrappers. Please confirm this is the intended API design pattern for the engine client.Also applies to: 1069-1123, 1229-1283
278-349: Complex but well-structured error union typesThe
EngineErrorandRpcErrorKindtypes use complex discriminated unions, but they're well-structured with clear discrimination. Each variant includes appropriate context, making error handling and debugging more effective. The complexity appears justified by the comprehensive error information needed.Also applies to: 578-633
1113-1117: Consistent pagination structure across endpointsThe pagination metadata structure is consistently applied across all list endpoints with
totalCount,page, andlimitfields. This uniformity will make it easier to build reusable pagination components.Also applies to: 1273-1277, 1341-1345, 1401-1405
packages/engine/openapi-ts.config.ts
Outdated
| // input: "https://engine.thirdweb.com/openapi", | ||
| input: "http://localhost:3009/openapi", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider environment-based configuration for API endpoints.
Hardcoding localhost URLs in configuration files can lead to issues if accidentally deployed to production. Consider using environment variables to dynamically set the input URL based on the deployment environment.
- // input: "https://engine.thirdweb.com/openapi",
- input: "http://localhost:3009/openapi",
+ input: process.env.OPENAPI_INPUT_URL || "https://engine.thirdweb.com/openapi",📝 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.
| // input: "https://engine.thirdweb.com/openapi", | |
| input: "http://localhost:3009/openapi", | |
| input: process.env.OPENAPI_INPUT_URL || "https://engine.thirdweb.com/openapi", |
🤖 Prompt for AI Agents
In packages/engine/openapi-ts.config.ts around lines 4 to 5, the input URL is
hardcoded to localhost, which risks incorrect URLs in different environments.
Replace the hardcoded URL with a dynamic value sourced from environment
variables, such as process.env.API_BASE_URL, and provide a fallback if needed.
This ensures the input URL adapts based on the deployment environment and avoids
accidental production misconfigurations.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7427 +/- ##
==========================================
- Coverage 51.96% 51.88% -0.08%
==========================================
Files 947 947
Lines 63823 63922 +99
Branches 4217 4214 -3
==========================================
+ Hits 33163 33166 +3
- Misses 30554 30650 +96
Partials 106 106
🚀 New features to boost your workflow:
|
joaquim-verges
force-pushed
the
Switch_Engine_API_endpoint_to_localhost_for_development
branch
from
d745795 to
f82a048
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/thirdweb/src/engine/server-wallet.ts (1)
152-177: Address the type casting issues in execution options handling.The
getExecutionOptionsfunction contains unsafe type casting with FIXME comments that indicate unresolved type issues. This was previously flagged and should be properly resolved.Replace the forced type assertions with proper type guards:
- const options: ExecutionOptions | undefined = executionOptions as - | ExecutionOptions - | undefined; + const options = executionOptions ? { + ...executionOptions, + chainId + } as ExecutionOptions : undefined;Additionally, consider adding runtime validation to ensure
executionOptionsconforms to the expected structure before usage.
🧹 Nitpick comments (1)
packages/thirdweb/src/engine/server-wallet.ts (1)
199-201: Improve transaction parameter validation.The default values for transaction parameters could be more explicit about their purpose and validation.
params: transaction.map((t) => ({ - data: t.data || "0x", - to: t.to ?? "", // TODO this should be allowed to be undefined - value: t.value?.toString() || "0", + data: t.data ?? "0x", // Default to empty calldata + to: t.to ?? "", // Empty string for contract creation - consider allowing undefined + value: t.value?.toString() ?? "0", // Default to zero value })),The TODO comment suggests
toshould allowundefined- consider updating the API types to support contract creation transactions properly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/analytics/tx-table/types.ts(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/tx/[id]/transaction-details-ui.tsx(2 hunks)packages/engine/openapi-ts.config.ts(1 hunks)packages/engine/package.json(2 hunks)packages/engine/src/client/client.gen.ts(2 hunks)packages/engine/src/client/sdk.gen.ts(1 hunks)packages/engine/src/client/types.gen.ts(1 hunks)packages/thirdweb/src/engine/create-server-wallet.ts(1 hunks)packages/thirdweb/src/engine/get-status.ts(1 hunks)packages/thirdweb/src/engine/list-server-wallets.ts(1 hunks)packages/thirdweb/src/engine/search-transactions.ts(1 hunks)packages/thirdweb/src/engine/server-wallet.test.ts(3 hunks)packages/thirdweb/src/engine/server-wallet.ts(9 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/engine/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/engine/openapi-ts.config.ts
- packages/thirdweb/src/engine/server-wallet.test.ts
- packages/engine/src/client/client.gen.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.@(ts|tsx)`: Accept a typed 'props' object and export a named function (e.g., export function MyComponent()). Combine class names via 'cn', expose 'className' prop if useful. ...
**/*.@(ts|tsx): Accept a typed 'props' object and export a named function (e.g., export function MyComponent()).
Combine class names via 'cn', expose 'className' prop if useful.
Reuse core UI primitives; avoid re-implementing buttons, cards, modals.
Local state or effects live inside; data fetching happens in hooks.
Merge class names with 'cn' from '@/lib/utils' to keep conditional logic readable.
Stick to design-tokens: background ('bg-card'), borders ('border-border'), muted text ('text-muted-foreground') etc.
Use the 'container' class with a 'max-w-7xl' cap for page width consistency.
Spacing utilities ('px-', 'py-', 'gap-*') are preferred over custom margins.
Responsive helpers follow mobile-first ('max-sm', 'md', 'lg', 'xl').
Never hard-code colors – always go through Tailwind variables.
Tailwind CSS is the styling system – avoid inline styles or CSS modules.
Prefix files with 'import "server-only";' so they never end up in the client bundle (for server-only code).
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/analytics/tx-table/types.tsapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/tx/[id]/transaction-details-ui.tsxpackages/thirdweb/src/engine/server-wallet.tspackages/engine/src/client/sdk.gen.tspackages/engine/src/client/types.gen.tspackages/thirdweb/src/engine/create-server-wallet.tspackages/thirdweb/src/engine/get-status.tspackages/thirdweb/src/engine/list-server-wallets.tspackages/thirdweb/src/engine/search-transactions.ts
🪛 GitHub Check: codecov/patch
packages/thirdweb/src/engine/server-wallet.ts
[warning] 152-153: packages/thirdweb/src/engine/server-wallet.ts#L152-L153
Added lines #L152 - L153 were not covered by tests
[warning] 156-166: packages/thirdweb/src/engine/server-wallet.ts#L156-L166
Added lines #L156 - L166 were not covered by tests
[warning] 168-174: packages/thirdweb/src/engine/server-wallet.ts#L168-L174
Added lines #L168 - L174 were not covered by tests
[warning] 176-176: packages/thirdweb/src/engine/server-wallet.ts#L176
Added line #L176 was not covered by tests
[warning] 199-201: packages/thirdweb/src/engine/server-wallet.ts#L199-L201
Added lines #L199 - L201 were not covered by tests
[warning] 205-205: packages/thirdweb/src/engine/server-wallet.ts#L205
Added line #L205 was not covered by tests
[warning] 217-217: packages/thirdweb/src/engine/server-wallet.ts#L217
Added line #L217 was not covered by tests
[warning] 321-321: packages/thirdweb/src/engine/server-wallet.ts#L321
Added line #L321 was not covered by tests
[warning] 327-327: packages/thirdweb/src/engine/server-wallet.ts#L327
Added line #L327 was not covered by tests
[warning] 332-348: packages/thirdweb/src/engine/server-wallet.ts#L332-L348
Added lines #L332 - L348 were not covered by tests
[warning] 361-361: packages/thirdweb/src/engine/server-wallet.ts#L361
Added line #L361 was not covered by tests
[warning] 367-367: packages/thirdweb/src/engine/server-wallet.ts#L367
Added line #L367 was not covered by tests
[warning] 376-376: packages/thirdweb/src/engine/server-wallet.ts#L376
Added line #L376 was not covered by tests
[warning] 383-399: packages/thirdweb/src/engine/server-wallet.ts#L383-L399
Added lines #L383 - L399 were not covered by tests
[warning] 412-412: packages/thirdweb/src/engine/server-wallet.ts#L412
Added line #L412 was not covered by tests
[warning] 418-418: packages/thirdweb/src/engine/server-wallet.ts#L418
Added line #L418 was not covered by tests
packages/thirdweb/src/engine/create-server-wallet.ts
[warning] 50-50: packages/thirdweb/src/engine/create-server-wallet.ts#L50
Added line #L50 was not covered by tests
packages/thirdweb/src/engine/get-status.ts
[warning] 102-102: packages/thirdweb/src/engine/get-status.ts#L102
Added line #L102 was not covered by tests
packages/thirdweb/src/engine/list-server-wallets.ts
[warning] 39-39: packages/thirdweb/src/engine/list-server-wallets.ts#L39
Added line #L39 was not covered by tests
packages/thirdweb/src/engine/search-transactions.ts
[warning] 125-125: packages/thirdweb/src/engine/search-transactions.ts#L125
Added line #L125 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
packages/thirdweb/src/engine/create-server-wallet.ts (1)
50-50: LGTM: API response structure update is consistent.The change correctly adapts to the new API response format where data is nested under HTTP status code keys. Using
201for creation operations is semantically appropriate.Note: This line lacks test coverage according to static analysis.
packages/thirdweb/src/engine/list-server-wallets.ts (1)
39-39: LGTM: Consistent with API response format update.The change correctly adapts to the new response structure using status code
200for successful list operations, which is semantically appropriate.Note: This line lacks test coverage according to static analysis.
packages/thirdweb/src/engine/get-status.ts (1)
102-102: LGTM: API response format consistently updated.The change correctly adapts to the new response structure where search results are nested under status code
200. The transaction extraction logic remains intact.Note: This line lacks test coverage according to static analysis.
packages/thirdweb/src/engine/search-transactions.ts (1)
125-125: LGTM: Consistent API response structure adaptation.The change correctly updates the data extraction to use the new response format with status code
200for successful search operations.Note: This line lacks test coverage according to static analysis.
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/analytics/tx-table/types.ts (1)
37-44: LGTM: Enhanced error structure improves debugging capabilities.The change from a simple string
errorto a structured error object significantly improves error reporting and debugging. The new structure provides:
- Detailed error context (
stage,message,errorCode)- Nested error information (
inner_error)- Transaction-specific details (
nonce_used,account_address)This structured approach will enable better error handling and user feedback in the dashboard.
packages/thirdweb/src/engine/server-wallet.ts (1)
217-217: ```shell
#!/bin/bashFind import of writeTransaction in server-wallet.ts
grep -R "writeTransaction" -n packages/thirdweb/src/engine/server-wallet.ts
</details> <details> <summary>packages/engine/src/client/sdk.gen.ts (1)</summary> `1-426`: **Auto-generated SDK client changes align with API specification updates.** The extensive restructuring of SDK functions, type imports, and endpoint mappings is consistent with the API specification updates mentioned in the PR objectives. The new functions for transaction analytics, signing operations, contract reading, and encoding expand the API surface appropriately. Key improvements observed: - Consistent client instance selection pattern - Proper JSON content-type headers for POST requests - Updated security configurations - New endpoints for enhanced functionality (analytics, activity logs, signing operations) </details> <details> <summary>packages/engine/src/client/types.gen.ts (1)</summary> `3-1635`: **Comprehensive type definitions support the API specification overhaul.** The extensive type system updates provide a solid foundation for the engine client API changes. Key improvements include: - Unified `ExecutionOptions` type combining base and specific execution options - Detailed error handling with `EngineError` and related types - Strong typing for contract interactions, signing operations, and analytics - Structured response types with status code keys matching the new API pattern - Support for new endpoints (transaction analytics, activity logs) The type definitions align well with the changes in the SDK client and server wallet implementations, supporting enhanced type safety and API functionality. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
...p)/team/[team_slug]/[project_slug]/(sidebar)/transactions/tx/[id]/transaction-details-ui.tsx
Outdated
Show resolved
Hide resolved
…development Signed-off-by: Prithvish Baidya <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/engine/src/client/types.gen.ts (1)
3-13: Breaking change acknowledged -operationfield now required.This confirms the breaking change flagged in previous reviews where the
operationfield inTransactionsFilterValueis now mandatory. Since this is an auto-generated file, ensure all consuming code has been updated to provide the requiredoperationvalue ("AND" or "OR").
🧹 Nitpick comments (2)
packages/thirdweb/src/wallets/in-app/web/lib/in-app-gateway.test.ts (2)
16-19: TODO: Complete test productionization.The test suite is currently skipped with a TODO to productionize it. Consider creating a tracking issue for completing this work, as it provides valuable integration testing for the in-app wallet gateway functionality.
Would you like me to help create a plan for productionizing this test suite, including removing debug statements and making it suitable for CI?
113-114: Remove debug console.log statements for production.The console.log statements should be removed or replaced with proper test logging when productionizing this test suite.
- console.log(txId); + // Transaction queued successfully with ID: ${txId}- console.log(tx); + // Transaction confirmed: ${tx.transactionHash}Also applies to: 123-123
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/engine/openapi-ts.config.ts(0 hunks)packages/engine/package.json(2 hunks)packages/engine/src/client/client.gen.ts(2 hunks)packages/engine/src/client/types.gen.ts(1 hunks)packages/thirdweb/src/wallets/in-app/core/authentication/backend.ts(1 hunks)packages/thirdweb/src/wallets/in-app/web/lib/in-app-gateway.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/engine/openapi-ts.config.ts
✅ Files skipped from review due to trivial changes (1)
- packages/engine/src/client/client.gen.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/engine/package.json
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.@(ts|tsx)`: Accept a typed 'props' object and export a named function (e.g...
**/*.@(ts|tsx): Accept a typed 'props' object and export a named function (e.g., export function MyComponent()).
Combine class names via 'cn', expose 'className' prop if useful.
Reuse core UI primitives; avoid re-implementing buttons, cards, modals.
Local state or effects live inside; data fetching happens in hooks.
Merge class names with 'cn' from '@/lib/utils' to keep conditional logic readable.
Stick to design-tokens: background ('bg-card'), borders ('border-border'), muted text ('text-muted-foreground') etc.
Use the 'container' class with a 'max-w-7xl' cap for page width consistency.
Spacing utilities ('px-', 'py-', 'gap-*') are preferred over custom margins.
Responsive helpers follow mobile-first ('max-sm', 'md', 'lg', 'xl').
Never hard-code colors – always go through Tailwind variables.
Tailwind CSS is the styling system – avoid inline styles or CSS modules.
Prefix files with 'import "server-only";' so they never end up in the client bundle (for server-only code).
📄 Source: CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
List of files the instruction was applied to:
packages/thirdweb/src/wallets/in-app/core/authentication/backend.tspackages/thirdweb/src/wallets/in-app/web/lib/in-app-gateway.test.tspackages/engine/src/client/types.gen.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-25T02:13:08.257Z
Learning: All CI checks (type-check, Biome, tests) must pass before merging a pull request.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-25T02:13:08.257Z
Learning: Keep documentation focused on developer experience and practical usage, and surface breaking changes prominently in PR descriptions.
packages/thirdweb/src/wallets/in-app/core/authentication/backend.ts (1)
Learnt from: jnsdls
PR: thirdweb-dev/js#7364
File: apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx:36-41
Timestamp: 2025-06-18T02:13:34.500Z
Learning: In the logout flow in apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx, when `doLogout()` fails, the cleanup steps (resetAnalytics(), wallet disconnect, router refresh) should NOT execute. This is intentional to maintain consistency - if server-side logout fails, client-side cleanup should not occur.
packages/thirdweb/src/wallets/in-app/web/lib/in-app-gateway.test.ts (2)
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-25T02:13:08.257Z
Learning: Wallet architecture should unify Wallet and Account interfaces, support in-app wallets (social/email login), smart wallets with account abstraction, and EIP-1193, EIP-5792, EIP-7702 standards.
Learnt from: jnsdls
PR: thirdweb-dev/js#7364
File: apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx:36-41
Timestamp: 2025-06-18T02:13:34.500Z
Learning: In the logout flow in apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx, when `doLogout()` fails, the cleanup steps (resetAnalytics(), wallet disconnect, router refresh) should NOT execute. This is intentional to maintain consistency - if server-side logout fails, client-side cleanup should not occur.
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/engine/src/client/types.gen.ts (3)
1699-1701: Improved: baseUrl no longer hardcoded to localhost.The
ClientOptionstype now uses a production URL (https://engine.thirdweb.com) as the default instead of the previously hardcoded localhost, addressing the flexibility concern from earlier reviews. This allows for production deployments while maintaining type safety.
56-155: Comprehensive error handling types added.The new discriminated union types for batch results provide detailed error categorization (RPC_ERROR, VAULT_ERROR, CONTRACT_INTERACTION_ERROR, etc.) with specific error contexts. This enables robust error handling throughout the client SDK.
Also applies to: 163-242, 250-338
642-646: Unified execution options design.The
ExecutionOptionstype effectively combines base and specific execution options, providing a clean API surface while maintaining type safety through discriminated unions.packages/thirdweb/src/wallets/in-app/core/authentication/backend.ts (1)
34-35: Enhanced error handling with response body context.Including the response body text in the error message provides valuable debugging information when backend authentication fails. This improvement aligns with the broader error handling enhancements throughout the codebase.
packages/thirdweb/src/wallets/in-app/web/lib/in-app-gateway.test.ts (2)
28-43: Good test setup with proper authentication flow.The test setup correctly initializes the in-app wallet with backend strategy and validates the authentication token retrieval. The domain configuration ensures tests run against the correct environment.
45-80: Comprehensive message signing test with API validation.The test effectively validates both local wallet signing and backend API signing, ensuring consistency between the two approaches. The error handling properly extracts and reports signing failures.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/lovely-brooms-relax.md (2)
5-5: Insert the determiner “the” for grammatical correctnessMinor wording tweak improves readability:
-Support latest engine API +Support the latest engine API
5-6: Expand the summary to capture impactful changesThe one-line summary doesn’t communicate the scope (e.g., schema bump to 2.0.4, new endpoints, renamed types). A more descriptive message helps consumers understand what changed and whether action is required.
Example:Support the latest engine API - Bump schema to 2.0.4 - Add analytics & activity-log endpoints - Unify execution & signing options - BREAKING: renamed `ServerWalletExecuteTx` → `ExecuteTransaction`This richer summary surfaces breaking changes and upgrade notes directly in the changelog.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/heavy-banks-think.md(1 hunks).changeset/lovely-brooms-relax.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/heavy-banks-think.md
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-25T02:13:08.257Z
Learning: All CI checks (type-check, Biome, tests) must pass before merging a pull request.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-25T02:13:08.257Z
Learning: Keep documentation focused on developer experience and practical usage, and surface breaking changes prominently in PR descriptions.
.changeset/lovely-brooms-relax.md (1)
Learnt from: MananTank
PR: thirdweb-dev/js#7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:424-424
Timestamp: 2025-06-06T23:46:08.795Z
Learning: The thirdweb project has an ESLint rule that restricts direct usage of `defineChain`. When it's necessary to use `defineChain` directly, it's acceptable to disable the rule with `// eslint-disable-next-line no-restricted-syntax`.
🪛 LanguageTool
.changeset/lovely-brooms-relax.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: --- "thirdweb": patch --- Support latest engine API
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
joaquim-verges
deleted the
Switch_Engine_API_endpoint_to_localhost_for_development
branch
PR-Codex overview
This PR focuses on updating various components of the project to support the latest engine API and schema versions, improve error handling, and enhance type definitions. It includes updates to configuration files, JSON schemas, and TypeScript interfaces.
Detailed summary
biome.jsonfiles to2.0.4.wait-for-tx-hash.tsandbackend.ts.types.tsandparams.ts.tsconfigfiles for better development support.client.tsfor improved response parsing.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores
Tests