Add base account sdk #8634
Add base account sdk #8634
Conversation
|
|
@montycheese is attempting to deploy a commit to the thirdweb Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request integrates Base Account SDK wallet support into the Thirdweb SDK. Changes include adding the Changes
Sequence DiagramssequenceDiagram
participant App as Application
participant Wallet as Base Account Wallet
participant Provider as Base Account SDK Provider
participant Chain as Chain Service
App->>Wallet: createWallet(BASE_ACCOUNT)
Wallet->>Wallet: baseAccountWalletSDK(providerFactory)
App->>Wallet: connect(options)
Wallet->>Provider: getBaseAccountWebProvider()
Provider-->>Wallet: ProviderInterface (cached)
Wallet->>Provider: eth_requestAccounts()
Provider-->>Wallet: [account_address]
Wallet->>Provider: eth_chainId()
Provider-->>Wallet: chain_id
Wallet->>Chain: getCachedChainIfExists()
Chain-->>Wallet: Chain metadata
Wallet->>Wallet: emit accountsChanged
Wallet->>Wallet: emit chainChanged
Wallet-->>App: {account, chain, switchChain}
sequenceDiagram
participant App as Application
participant Wallet as Base Account Wallet
participant Provider as Base Account SDK Provider
participant Signer as Signing Service
App->>Wallet: signMessage("hello")
Wallet->>Signer: Hex.fromString("hello")
Signer-->>Wallet: formatted_message
Wallet->>Provider: personal_sign(formatted_message, account)
Provider-->>Wallet: signature_hex
Wallet->>Signer: Hex.validate(signature_hex)
Signer-->>Wallet: is_valid
Wallet-->>App: signature
App->>Wallet: sendTransaction({to, data, value})
Wallet->>Provider: eth_sendTransaction()
Provider-->>Wallet: tx_hash
Wallet-->>App: tx_hash
sequenceDiagram
participant App as Application
participant Wallet as Base Account Wallet
participant Provider as Base Account SDK Provider
participant Chain as Chain Service
App->>Wallet: switchChain(chain)
Wallet->>Provider: wallet_switchEthereumChain({chainId})
alt Chain already exists
Provider-->>Wallet: success
else Chain not found (4902)
Provider-->>Wallet: 4902 error
Wallet->>Provider: wallet_addEthereumChain(chainConfig)
Provider-->>Wallet: success
end
Wallet->>Wallet: emit chainChanged
Wallet-->>App: switched
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
- Add @base-org/account 2.5.0 dependency
- Implement Base Account SDK wallet connector
- Add base-account-web.ts with EIP-1193 provider implementation
- Add base-account-wallet.ts with core wallet logic
- Export types and helper functions
- Maintain backwards compatibility with coinbase/wallet-sdk
montycheese
force-pushed
the
add-base-account-sdk
branch
from
45bf727 to
47438c6
Compare
|
ty @montycheese ! can you add a changeset ? pnpm changeset from the root of the repo, select thirdweb, enter to skip and make it a minor also you can run pnpm fix / lint for linting issues |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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/thirdweb/package.json`:
- Line 14: Update package.json to include a size-limit configuration array and
add an entry tracking the new `@base-org/account` dependency (identify the package
by its name "@base-org/account" and include a descriptive label in the
size-limit entry), and change the dependency version to the latest patch (2.5.1)
instead of 2.5.0; ensure the size-limit array follows the existing project
format and includes the package path or import target used by the build so
bundle budgets are enforced for `@base-org/account`.
In `@packages/thirdweb/src/wallets/base-account/base-account-wallet.ts`:
- Around line 44-52: The accountChanged subscription is never cleaned up which
can leak on repeated connects; capture its unsubscribe function (e.g., const
unsubscribeAccountChanged = emitter.subscribe("accountChanged", ...)) and call
unsubscribeAccountChanged() inside the disconnect handler alongside
unsubscribeChainChanged() and unsubscribeDisconnect() (and any other teardown
paths) so all three subscriptions are properly unsubscribed when the wallet
disconnects; update references in base-account-wallet.ts to use
unsubscribeAccountChanged where accountChanged is subscribed.
In `@packages/thirdweb/src/wallets/base-account/base-account-web.ts`:
- Around line 442-462: The catch block around the wallet_switchEthereumChain
flow swallows all errors except when error.code === 4902; update the catch in
base-account-web.ts to re-throw any error that is not the "chain not added" case
so callers get notified of failures (e.g., user rejection 4001). Specifically,
inside the catch handling for wallet_switchEthereumChain, keep the existing
handling that calls getChainMetadata and provider.request for the 4902 case but
after that if the caught error's code !== 4902 throw the original error
(preserve the error object) so failures from provider.request or
wallet_switchEthereumChain propagate to callers.
- Around line 150-178: The sendTransaction method builds the eth_sendTransaction
params but omits EIP-1559 gas fields; inside sendTransaction (the
provider.request call that constructs the tx object) include tx.gasPrice,
tx.maxFeePerGas, and tx.maxPriorityFeePerGas when present (convert numeric
values to hex with numberToHex like gas is handled) so the provider receives
explicit gas pricing; keep getAddress(address), accessList, data, to, value
handling unchanged and continue passing gasPrice through to trackTransaction as
before.
In `@packages/thirdweb/src/wallets/create-wallet.ts`:
- Around line 128-141: The example in createWallet's usage shows a non-existent
option `testnet: false`; update the example to remove `testnet` and demonstrate
using the supported `chains` option from BaseAccountWalletCreationOptions (see
base-account-web.ts) instead — e.g., replace `testnet` with an appropriate
`chains` array or comment explaining how to select testnet chains when calling
createWallet("org.base.account", { appMetadata, chains }) so the example matches
the actual createWallet/BaseAccountWalletCreationOptions API.
🧹 Nitpick comments (4)
packages/thirdweb/src/wallets/base-account/base-account-web.test.ts (1)
124-177: Sign tests validate mocked patterns rather than actual implementation.These tests define local
accountobjects withsignMessageandsignTypedDataimplementations that mimic the expected behavior, then verify those local implementations. This approach validates thatox/Hexandox/TypedDatausage patterns work correctly, but doesn't exercise the actualcreateAccountfunction frombase-account-web.ts.Consider refactoring to test the real implementation by:
- Exposing
createAccountfor testing, or- Testing through
connectBaseAccountSDKwhich returns the real account objectExample approach using connectBaseAccountSDK
test("signMessage works through connected account", async () => { provider.request = vi .fn() .mockResolvedValueOnce(["0x123"]) // eth_requestAccounts .mockResolvedValueOnce("0x1") // eth_chainId .mockResolvedValue("0xsignature"); // personal_sign const emitter = { emit: vi.fn() }; const options = { client: {} }; const [account] = await connectBaseAccountSDK( options as any, emitter as any, provider, ); const signature = await account.signMessage({ message: "hello" }); expect(signature).toBe("0xsignature"); });packages/thirdweb/src/wallets/base-account/base-account-web.ts (2)
179-202: Redundant address check insignMessage.The check at lines 180-182 verifies
account.address, but this property is set on line 149 during account creation and cannot be undefined. This check is defensive but unnecessary. The same applies tosignTypedData(lines 204-206).
100-125: The singleton pattern is intentional and acceptable by design.The module-level
_providersingleton is not a bug. EachcreateWallet()call creates a new wallet instance with its ownproviderFactoryclosure that captures the specific options. The options are established at wallet instantiation time, not provided on subsequent calls. The singleton is reused only within the same wallet instance to maintain connection state.This design is intentional and consistent with other wallet implementations (Coinbase). Consider adding a more explicit explanatory comment to match what
coinbase-web.tsprovides: "this should be ok since all the creation options are provided at build time."packages/thirdweb/src/wallets/wallet-types.ts (1)
96-111: LGTM!The type branching for
WalletConnectionOptioncorrectly routesBASE_ACCOUNTtoBaseAccountSDKWalletConnectionOptions.Nit: Line 99 has a typo "inhected" → "injected".
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/thirdweb/package.jsonpackages/thirdweb/src/exports/wallets.tspackages/thirdweb/src/wallets/base-account/base-account-wallet.tspackages/thirdweb/src/wallets/base-account/base-account-web.test.tspackages/thirdweb/src/wallets/base-account/base-account-web.tspackages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/wallets/create-wallet.tspackages/thirdweb/src/wallets/wallet-types.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from@/typesor local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/exports/wallets.tspackages/thirdweb/src/wallets/base-account/base-account-wallet.tspackages/thirdweb/src/wallets/wallet-types.tspackages/thirdweb/src/wallets/base-account/base-account-web.tspackages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/wallets/base-account/base-account-web.test.tspackages/thirdweb/src/wallets/create-wallet.ts
packages/thirdweb/src/exports/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/exports/**/*.{ts,tsx}: Export everything viaexports/directory, grouped by feature in SDK development
Every public symbol must have comprehensive TSDoc with at least one@exampleblock that compiles and custom annotation tags (@beta,@internal,@experimental)Export everything in packages/thirdweb via the exports/ directory, grouped by feature. Every public symbol must have comprehensive TSDoc including at least one
@exampleblock that compiles and one custom annotation (@beta,@internal,@experimental, etc.)
Files:
packages/thirdweb/src/exports/wallets.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/exports/wallets.tspackages/thirdweb/src/wallets/base-account/base-account-wallet.tspackages/thirdweb/src/wallets/wallet-types.tspackages/thirdweb/src/wallets/base-account/base-account-web.tspackages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/wallets/base-account/base-account-web.test.tspackages/thirdweb/src/wallets/create-wallet.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/exports/wallets.tspackages/thirdweb/src/wallets/base-account/base-account-wallet.tspackages/thirdweb/src/wallets/wallet-types.tspackages/thirdweb/src/wallets/base-account/base-account-web.tspackages/thirdweb/package.jsonpackages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/wallets/base-account/base-account-web.test.tspackages/thirdweb/src/wallets/create-wallet.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/exports/wallets.tspackages/thirdweb/src/wallets/base-account/base-account-wallet.tspackages/thirdweb/src/wallets/wallet-types.tspackages/thirdweb/src/wallets/base-account/base-account-web.tspackages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/wallets/base-account/base-account-web.test.tspackages/thirdweb/src/wallets/create-wallet.ts
packages/thirdweb/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Track bundle budgets via
package.json#size-limit
Files:
packages/thirdweb/package.json
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.tsin the same directory
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
Use predefined test accounts fromtest/src/test-wallets.tsin tests
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co-locate tests with source files using the pattern foo.ts ↔ foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
For network interactions in tests, use Mock Service Worker (MSW) to intercept fetch/HTTP calls, mocking only scenarios that are hard to reproduce
Keep tests deterministic and side-effect free; Vitest is pre-configured
Files:
packages/thirdweb/src/wallets/base-account/base-account-web.test.ts
🧬 Code graph analysis (2)
packages/thirdweb/src/wallets/wallet-types.ts (2)
packages/thirdweb/src/wallets/constants.ts (1)
BASE_ACCOUNT(5-5)packages/thirdweb/src/wallets/base-account/base-account-web.ts (2)
BaseAccountSDKWalletConnectionOptions(78-98)BaseAccountWalletCreationOptions(41-73)
packages/thirdweb/src/wallets/create-wallet.ts (3)
packages/thirdweb/src/wallets/constants.ts (1)
BASE_ACCOUNT(5-5)packages/thirdweb/src/wallets/base-account/base-account-wallet.ts (1)
baseAccountWalletSDK(17-116)packages/thirdweb/src/wallets/base-account/base-account-web.ts (1)
getBaseAccountWebProvider(106-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (13)
packages/thirdweb/src/exports/wallets.ts (1)
14-18: Base Account SDK exports follow established patterns.The new exports align with the existing pattern used for Coinbase SDK (lines 19-22). The type exports and runtime type guard (
isBaseAccountSDKWallet) provide a complete public API surface for the new wallet integration.packages/thirdweb/src/wallets/base-account/base-account-web.test.ts (1)
84-122: Connection tests validate the core flow.The
connectBaseAccountSDKandautoConnectBaseAccountSDKtests properly mock the provider and verify account address/chain extraction from the SDK responses. The test setup correctly resets modules between runs to avoid cached state issues.packages/thirdweb/src/wallets/base-account/base-account-wallet.ts (2)
55-95: Connection methods follow lazy-loading best practices.Both
autoConnectandconnectcorrectly use dynamic imports to lazy-load the web module, keeping the initial bundle lean per coding guidelines. The implementations properly track connections for analytics and maintain consistent state updates.The similar structure between
autoConnect(lines 55-75) andconnect(lines 76-95) could potentially be refactored to share common logic, but the current approach maintains clarity and matches patterns used in other wallet implementations.
96-114: Wallet lifecycle methods are properly implemented.The
disconnectcorrectly resets local state before invoking the provider disconnect handler. TheswitchChainmethod properly delegates to the handler established during connection. ThegetChainmethod usesgetCachedChainIfExistsfor performance optimization.packages/thirdweb/src/wallets/base-account/base-account-web.ts (2)
300-352: Event handling is well-structured.The
onConnectfunction properly manages provider event subscriptions with corresponding cleanup indisconnect. TheonAccountsChangedhandler correctly handles the edge case of an empty accounts array by triggering disconnect. The returned tuple provides all necessary lifecycle functions.
84-98: JSDoc comment is incomplete - missing closing tag.The JSDoc comment for the
chainproperty (starting at line 84) is missing the closing*/before line 97. This may cause documentation generation issues.Proposed fix
* const address = await wallet.connect({ chain: myChain }) + */ chain?: Chain; };Likely an incorrect or invalid review comment.
packages/thirdweb/src/wallets/constants.ts (1)
5-5: LGTM!The new
BASE_ACCOUNTconstant follows the established reverse domain notation pattern consistent with other wallet constants.packages/thirdweb/src/wallets/create-wallet.ts (2)
13-17: LGTM!Imports follow the established pattern, mirroring the Coinbase wallet import structure.
212-224: LGTM!The Base Account SDK wallet case follows the established pattern. The lazy provider loading via
providerFactoryaligns with the coding guidelines for keeping the initial bundle lean.packages/thirdweb/src/wallets/wallet-types.ts (4)
9-17: LGTM!Imports are correctly structured, following the established pattern for wallet-specific types and constants.
43-43: LGTM!Adding
typeof BASE_ACCOUNTto theWalletIdunion follows the pattern established by other wallet constants, enabling type-safe wallet creation.
127-142: LGTM!The
WalletAutoConnectionOptiontype branch correctly mirrors the connection option pattern forBASE_ACCOUNT.
154-164: LGTM!The
WalletCreationOptionstype branch correctly returnsBaseAccountWalletCreationOptionsforBASE_ACCOUNT, enabling type-safe wallet creation with the appropriate options.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "url": "https://github.com/thirdweb-dev/js/issues" | ||
| }, | ||
| "dependencies": { | ||
| "@base-org/account": "2.5.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Query npm registry for `@base-org/account` version 2.5.0
curl -s https://registry.npmjs.org/@base-org/account | jq '.versions["2.5.0"] | {version, dist: .dist | {tarball, shasum}}'Repository: thirdweb-dev/js
Length of output: 240
🏁 Script executed:
# Also check the latest version available
curl -s https://registry.npmjs.org/@base-org/account | jq '.["dist-tags"]'Repository: thirdweb-dev/js
Length of output: 204
🏁 Script executed:
# Check the current package.json to understand size-limit configuration
head -50 packages/thirdweb/package.jsonRepository: thirdweb-dev/js
Length of output: 1487
🏁 Script executed:
# Search for size-limit configuration in package.json
grep -A 20 "size-limit" packages/thirdweb/package.jsonRepository: thirdweb-dev/js
Length of output: 2457
🏁 Script executed:
# Also check if the change included updates to size-limit
git diff HEAD -- packages/thirdweb/package.json | head -100Repository: thirdweb-dev/js
Length of output: 41
🏁 Script executed:
# Search for size-limit configuration array in the entire package.json
cat packages/thirdweb/package.json | jq '.["size-limit"]' 2>/dev/null || echo "No size-limit array found in package.json"Repository: thirdweb-dev/js
Length of output: 63
🏁 Script executed:
# Also check the end of the file to see if size-limit config is there
tail -100 packages/thirdweb/package.jsonRepository: thirdweb-dev/js
Length of output: 3731
Add size-limit configuration for the new @base-org/account dependency.
The @base-org/account package at version 2.5.0 is valid and follows the existing exact-version pinning pattern. However, per the coding guidelines, bundle budgets must be tracked via package.json#size-limit. The current package.json lacks a size-limit configuration array—add an entry for this dependency to track its bundle size impact. Additionally, consider updating to version 2.5.1 (the latest patch), as it's more recent than 2.5.0.
🤖 Prompt for AI Agents
In `@packages/thirdweb/package.json` at line 14, Update package.json to include a
size-limit configuration array and add an entry tracking the new
`@base-org/account` dependency (identify the package by its name
"@base-org/account" and include a descriptive label in the size-limit entry),
and change the dependency version to the latest patch (2.5.1) instead of 2.5.0;
ensure the size-limit array follows the existing project format and includes the
package path or import target used by the build so bundle budgets are enforced
for `@base-org/account`.
| const unsubscribeDisconnect = emitter.subscribe("disconnect", () => { | ||
| reset(); | ||
| unsubscribeChainChanged(); | ||
| unsubscribeDisconnect(); | ||
| }); | ||
|
|
||
| emitter.subscribe("accountChanged", (_account) => { | ||
| account = _account; | ||
| }); |
There was a problem hiding this comment.
Potential subscription leak for accountChanged event.
The chainChanged and disconnect subscriptions are properly cleaned up on disconnect (lines 46-47), but the accountChanged subscription (lines 50-52) is never unsubscribed. This could lead to memory leaks if the wallet is repeatedly connected and disconnected.
Proposed fix
+ const unsubscribeAccountChanged = emitter.subscribe("accountChanged", (_account) => {
+ account = _account;
+ });
+
const unsubscribeDisconnect = emitter.subscribe("disconnect", () => {
reset();
unsubscribeChainChanged();
unsubscribeDisconnect();
+ unsubscribeAccountChanged();
});
- emitter.subscribe("accountChanged", (_account) => {
- account = _account;
- });📝 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 unsubscribeDisconnect = emitter.subscribe("disconnect", () => { | |
| reset(); | |
| unsubscribeChainChanged(); | |
| unsubscribeDisconnect(); | |
| }); | |
| emitter.subscribe("accountChanged", (_account) => { | |
| account = _account; | |
| }); | |
| const unsubscribeAccountChanged = emitter.subscribe("accountChanged", (_account) => { | |
| account = _account; | |
| }); | |
| const unsubscribeDisconnect = emitter.subscribe("disconnect", () => { | |
| reset(); | |
| unsubscribeChainChanged(); | |
| unsubscribeDisconnect(); | |
| unsubscribeAccountChanged(); | |
| }); |
🤖 Prompt for AI Agents
In `@packages/thirdweb/src/wallets/base-account/base-account-wallet.ts` around
lines 44 - 52, The accountChanged subscription is never cleaned up which can
leak on repeated connects; capture its unsubscribe function (e.g., const
unsubscribeAccountChanged = emitter.subscribe("accountChanged", ...)) and call
unsubscribeAccountChanged() inside the disconnect handler alongside
unsubscribeChainChanged() and unsubscribeDisconnect() (and any other teardown
paths) so all three subscriptions are properly unsubscribed when the wallet
disconnects; update references in base-account-wallet.ts to use
unsubscribeAccountChanged where accountChanged is subscribed.
| async sendTransaction(tx: SendTransactionOption) { | ||
| const transactionHash = (await provider.request({ | ||
| method: "eth_sendTransaction", | ||
| params: [ | ||
| { | ||
| accessList: tx.accessList, | ||
| data: tx.data, | ||
| from: getAddress(address), | ||
| gas: tx.gas ? numberToHex(tx.gas) : undefined, | ||
| to: tx.to as Address, | ||
| value: tx.value ? numberToHex(tx.value) : undefined, | ||
| }, | ||
| ], | ||
| })) as Hex; | ||
|
|
||
| trackTransaction({ | ||
| chainId: tx.chainId, | ||
| client: client, | ||
| contractAddress: tx.to ?? undefined, | ||
| gasPrice: tx.gasPrice, | ||
| transactionHash, | ||
| walletAddress: getAddress(address), | ||
| walletType: BASE_ACCOUNT, | ||
| }); | ||
|
|
||
| return { | ||
| transactionHash, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
sendTransaction omits EIP-1559 gas parameters.
The transaction request (lines 153-162) includes gas, value, data, accessList, but omits gasPrice, maxFeePerGas, and maxPriorityFeePerGas. While some providers may estimate these, explicitly passing them when provided ensures the transaction uses the caller's intended gas settings.
Proposed fix to include gas pricing parameters
params: [
{
accessList: tx.accessList,
data: tx.data,
from: getAddress(address),
gas: tx.gas ? numberToHex(tx.gas) : undefined,
+ gasPrice: tx.gasPrice ? numberToHex(tx.gasPrice) : undefined,
+ maxFeePerGas: tx.maxFeePerGas ? numberToHex(tx.maxFeePerGas) : undefined,
+ maxPriorityFeePerGas: tx.maxPriorityFeePerGas ? numberToHex(tx.maxPriorityFeePerGas) : undefined,
to: tx.to as Address,
value: tx.value ? numberToHex(tx.value) : undefined,
},
],🤖 Prompt for AI Agents
In `@packages/thirdweb/src/wallets/base-account/base-account-web.ts` around lines
150 - 178, The sendTransaction method builds the eth_sendTransaction params but
omits EIP-1559 gas fields; inside sendTransaction (the provider.request call
that constructs the tx object) include tx.gasPrice, tx.maxFeePerGas, and
tx.maxPriorityFeePerGas when present (convert numeric values to hex with
numberToHex like gas is handled) so the provider receives explicit gas pricing;
keep getAddress(address), accessList, data, to, value handling unchanged and
continue passing gasPrice through to trackTransaction as before.
| } catch (error) { | ||
| const apiChain = await getChainMetadata(chain); | ||
|
|
||
| // Indicates chain is not added to provider | ||
| // biome-ignore lint/suspicious/noExplicitAny: TODO: fix later | ||
| if ((error as any)?.code === 4902) { | ||
| // try to add the chain | ||
| await provider.request({ | ||
| method: "wallet_addEthereumChain", | ||
| params: [ | ||
| { | ||
| blockExplorerUrls: apiChain.explorers?.map((x) => x.url) || [], | ||
| chainId: chainIdHex, | ||
| chainName: apiChain.name, | ||
| nativeCurrency: apiChain.nativeCurrency, // no client id on purpose here | ||
| rpcUrls: getValidPublicRPCUrl(apiChain), | ||
| }, | ||
| ], | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Errors other than code 4902 are silently swallowed.
When wallet_switchEthereumChain fails with an error that is not code 4902, the error is caught but not re-thrown after the if block (line 447-461). This means chain switch failures due to user rejection (code 4001) or other errors will silently fail without notifying the caller.
Proposed fix to re-throw non-4902 errors
} catch (error) {
const apiChain = await getChainMetadata(chain);
// Indicates chain is not added to provider
// biome-ignore lint/suspicious/noExplicitAny: TODO: fix later
if ((error as any)?.code === 4902) {
// try to add the chain
await provider.request({
method: "wallet_addEthereumChain",
params: [
{
blockExplorerUrls: apiChain.explorers?.map((x) => x.url) || [],
chainId: chainIdHex,
chainName: apiChain.name,
nativeCurrency: apiChain.nativeCurrency, // no client id on purpose here
rpcUrls: getValidPublicRPCUrl(apiChain),
},
],
});
+ } else {
+ throw error;
}
}📝 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.
| } catch (error) { | |
| const apiChain = await getChainMetadata(chain); | |
| // Indicates chain is not added to provider | |
| // biome-ignore lint/suspicious/noExplicitAny: TODO: fix later | |
| if ((error as any)?.code === 4902) { | |
| // try to add the chain | |
| await provider.request({ | |
| method: "wallet_addEthereumChain", | |
| params: [ | |
| { | |
| blockExplorerUrls: apiChain.explorers?.map((x) => x.url) || [], | |
| chainId: chainIdHex, | |
| chainName: apiChain.name, | |
| nativeCurrency: apiChain.nativeCurrency, // no client id on purpose here | |
| rpcUrls: getValidPublicRPCUrl(apiChain), | |
| }, | |
| ], | |
| }); | |
| } | |
| } | |
| } catch (error) { | |
| const apiChain = await getChainMetadata(chain); | |
| // Indicates chain is not added to provider | |
| // biome-ignore lint/suspicious/noExplicitAny: TODO: fix later | |
| if ((error as any)?.code === 4902) { | |
| // try to add the chain | |
| await provider.request({ | |
| method: "wallet_addEthereumChain", | |
| params: [ | |
| { | |
| blockExplorerUrls: apiChain.explorers?.map((x) => x.url) || [], | |
| chainId: chainIdHex, | |
| chainName: apiChain.name, | |
| nativeCurrency: apiChain.nativeCurrency, // no client id on purpose here | |
| rpcUrls: getValidPublicRPCUrl(apiChain), | |
| }, | |
| ], | |
| }); | |
| } else { | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/thirdweb/src/wallets/base-account/base-account-web.ts` around lines
442 - 462, The catch block around the wallet_switchEthereumChain flow swallows
all errors except when error.code === 4902; update the catch in
base-account-web.ts to re-throw any error that is not the "chain not added" case
so callers get notified of failures (e.g., user rejection 4001). Specifically,
inside the catch handling for wallet_switchEthereumChain, keep the existing
handling that calls getChainMetadata and provider.request for the 4902 case but
after that if the caught error's code !== 4902 throw the original error
(preserve the error object) so failures from provider.request or
wallet_switchEthereumChain propagate to callers.
| * const baseWallet = createWallet("org.base.account", { | ||
| * appMetadata: { | ||
| * name: "My App", | ||
| * url: "https://my-app.com", | ||
| * description: "my app description", | ||
| * logoUrl: "https://path/to/my-app/logo.svg", | ||
| * }, | ||
| * testnet: false, // set to true for testnet | ||
| * }); | ||
| * | ||
| * const account = await baseWallet.connect({ | ||
| * client, | ||
| * }); | ||
| * ``` |
There was a problem hiding this comment.
Documentation example contains a non-existent property.
The testnet: false option on line 135 doesn't exist in BaseAccountWalletCreationOptions. Per the type definition in base-account-web.ts, only appMetadata and chains are supported properties.
To specify testnet usage, the chains option should be used instead:
📝 Suggested documentation fix
* const baseWallet = createWallet("org.base.account", {
* appMetadata: {
* name: "My App",
-* url: "https://my-app.com",
-* description: "my app description",
* logoUrl: "https://path/to/my-app/logo.svg",
* },
-* testnet: false, // set to true for testnet
+* chains: [base], // specify supported chains
* });🤖 Prompt for AI Agents
In `@packages/thirdweb/src/wallets/create-wallet.ts` around lines 128 - 141, The
example in createWallet's usage shows a non-existent option `testnet: false`;
update the example to remove `testnet` and demonstrate using the supported
`chains` option from BaseAccountWalletCreationOptions (see base-account-web.ts)
instead — e.g., replace `testnet` with an appropriate `chains` array or comment
explaining how to select testnet chains when calling
createWallet("org.base.account", { appMetadata, chains }) so the example matches
the actual createWallet/BaseAccountWalletCreationOptions API.
2 participants
PR-Codex overview
This PR introduces support for the
Base Account SDKwithin thethirdwebwallet integration, enhancing wallet creation and connection functionalities.Detailed summary
BASE_ACCOUNTconstant inconstants.ts.BaseAccountSDKWalletConnectionOptionsandBaseAccountWalletCreationOptionstypes.baseAccountWalletSDKfunction for wallet connection.Base Account SDK.Base Account SDKfunctionalities.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.