-
Notifications
You must be signed in to change notification settings - Fork 622
[SDK] Feature: Makes chain optional on smart wallet config #5988
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
Changes from 3 commits
cadb36f
d1e7560
cbdb7cd
0783395
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| --- | ||
| "thirdweb": minor | ||
| --- | ||
|
|
||
| Feature: Chain is no longer required for smart accounts | ||
|
|
||
| ```ts | ||
| import { smartWallet } from "thirdweb"; | ||
|
|
||
| const wallet = smartWallet({ | ||
| sponsorGas: true, // enable sponsored transactions | ||
| }); | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,7 +43,6 @@ import type { SmartWalletOptions } from "./types.js"; | |
| * import { sendTransaction } from "thirdweb"; | ||
| * | ||
| * const wallet = smartWallet({ | ||
| * chain: sepolia, | ||
| * sponsorGas: true, // enable sponsored transactions | ||
| * }); | ||
| * | ||
|
|
@@ -77,7 +76,7 @@ import type { SmartWalletOptions } from "./types.js"; | |
| * import { sepolia } from "thirdweb/chains"; | ||
| * | ||
| * const wallet = smartWallet({ | ||
| * chain: sepolia, | ||
| * chain: sepolia, // specify a chain if your factory only exists on one chain | ||
| * sponsorGas: true, // enable sponsored transactions | ||
| * factoryAddress: "0x...", // custom factory address | ||
| * }); | ||
|
|
@@ -94,7 +93,6 @@ import type { SmartWalletOptions } from "./types.js"; | |
| * import { sepolia } from "thirdweb/chains"; | ||
| * | ||
| * const wallet = smartWallet({ | ||
| * chain: sepolia, | ||
| * sponsorGas: true, // enable sponsored transactions | ||
| * factoryAddress: DEFAULT_ACCOUNT_FACTORY_V0_7, // 0.7 factory address | ||
| * }); | ||
|
|
@@ -131,6 +129,12 @@ import type { SmartWalletOptions } from "./types.js"; | |
| export function smartWallet( | ||
| createOptions: SmartWalletOptions, | ||
| ): Wallet<"smart"> { | ||
| if ( | ||
| typeof createOptions.factoryAddress === "undefined" && | ||
| typeof createOptions.chain !== "undefined" | ||
| ) { | ||
| throw new Error("You must provide a chain if factory address is specified"); | ||
| } | ||
|
Comment on lines
+132
to
+137
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The condition appears to be checking the wrong logic - it currently throws when if (typeof createOptions.factoryAddress !== 'undefined' && typeof createOptions.chain === 'undefined')This ensures that a chain is specified whenever a custom factory address is used. Spotted by Graphite Reviewer
Comment on lines
+132
to
+137
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The validation logic appears to be inverted. The current check allows a if (typeof createOptions.factoryAddress !== 'undefined' && typeof createOptions.chain === 'undefined')This ensures that smart wallets with custom factories have the required chain configuration. Spotted by Graphite Reviewer |
||
| const emitter = createWalletEmitter<"smart">(); | ||
| let account: Account | undefined = undefined; | ||
| let adminAccount: Account | undefined = undefined; | ||
|
|
||
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.
One issue I see is that below this we get the factory and account contracts based on this chain variable.
It works with our default factory because it's deterministic (though need to double check it's on mainnet) but might error if you pass another factory address and no chain.
Also would double check we don't use this chain car for other assumptions in this connect flow