feat(connectors-web): add alchemySmartWallet connector #2115
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🌿 Documentation Preview
|
| */ | ||
| const loggedOutProvider: Provider = { | ||
| on: () => {}, | ||
| removeListener: () => {}, |
There was a problem hiding this comment.
Not sure if this is overkill/needed, do you think we should have some code to remove the listeners?
const listeners = new Map();
const loggedOutProvider: Provider = {
on: (event, listener) => {
if (!listeners.has(event)) listeners.set(event, new Set());
listeners.get(event)?.add(listener);
return () => listeners.get(event)?.delete(listener);
},
removeListener: (event, listener) => {
listeners.get(event)?.delete(listener);
},
};
There was a problem hiding this comment.
Since right now this provider never emits events, what I wrote should be equivalent behavior and I think there's no reason to store listeners that won't be called.
(We could forward listeners to the inner provider, but that feels like it could have unpredictable consequences.)
There was a problem hiding this comment.
not sure if this is applicable here, but in alchemyAuth we used a resumePromise (link) as a standard promise that we wait for on reload.
does the getProvider() only get called on reload, which is the case that the logged out provider is supposed to be for?
There was a problem hiding this comment.
Oh that's really interesting, I assumed that wagmi wouldn't accept us until we produced a provider, but it seems like just a promise of a provider is good enough for wagmi. Let me try that, and also see what built-in connectors are doing.
There was a problem hiding this comment.
There was a problem hiding this comment.
the only time we would need the dummy provider during this implementation is during reconnect, right?
when getProvider is called here, could we await the inner connector being ready, then await the smart wallet client creation so that we have the provider here?
There was a problem hiding this comment.
Thinking about it more, I'm not sure I like the idea of returning a suspended promise that resolves once the user logs in. That seems like it could be unexpected behavior for callers, who might call await connector.getProvider() and then be surprised that their program hangs forever. I think the expected behavior would be to return a provider that reflects the logged out state. What do you think?
| */ | ||
| const loggedOutProvider: Provider = { | ||
| on: () => {}, | ||
| removeListener: () => {}, |
There was a problem hiding this comment.
not sure if this is applicable here, but in alchemyAuth we used a resumePromise (link) as a standard promise that we wait for on reload.
does the getProvider() only get called on reload, which is the case that the logged out provider is supposed to be for?
There was a problem hiding this comment.
left a couple of questions. good work so far! cool to see this coming along.
could i request that we update the "send call via smart wallet client" button in the connector-web-example to just be "send call" and use the wagmi action to send a call via the connector?
| */ | ||
| const loggedOutProvider: Provider = { | ||
| on: () => {}, | ||
| removeListener: () => {}, |
There was a problem hiding this comment.
| */ | ||
| const loggedOutProvider: Provider = { | ||
| on: () => {}, | ||
| removeListener: () => {}, |
There was a problem hiding this comment.
the only time we would need the dummy provider during this implementation is during reconnect, right?
when getProvider is called here, could we await the inner connector being ready, then await the smart wallet client creation so that we have the provider here?
| async connect(parameters) { | ||
| await baseConnector.connect(parameters); | ||
| throw new Error("Not implemented: connect"); | ||
| async connect(params): Promise<{ accounts: Address[]; chainId: number }> { |
There was a problem hiding this comment.
not sure if this would change anything for your implementation, but just want to flag that connect has an { isReconnecting: true } param whenever it's called via wagmi's reconnect action.
dphilipson
force-pushed
the
dp/smart-wallet-connector
branch
from
b13c98a to
2429bd5
Compare
Basic implementation of a connector which uses the Wallet API to perform actions using an Alchemy smart account, wrapping another connector to perform signing. This version is tested to work when wrapping both the Metamask connector and the Alchemy Auth connector, including many edge cases such as changing accounts or configs in the Metamask UI after connecting.
dphilipson
force-pushed
the
dp/smart-wallet-connector
branch
from
2429bd5 to
8592acc
Compare
dphilipson
requested review from
dancoombs,
linnall,
adamegyed,
Zer0dot,
noam-alchemy,
avarobinson,
florrdv,
andysim3d,
0xfourzerofour,
niveda-krish,
alex-miao,
joshzhang5,
AlvaroLuken,
Dan-Nolan,
SahilAujla and
Richard-Dang
as code owners
Basic implementation of a connector which uses the Wallet API to perform actions using an Alchemy smart account, wrapping another connector to perform signing.
This version is tested to work with the Metamask connector, although there is additional work that can done to clean up code and polish edge cases, as marked with TODO's.
Pull Request Checklist
yarn test)sitefolder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change)yarn lint:check) and fix any issues? (yarn lint:write)PR-Codex overview
This PR primarily focuses on enhancing the
@alchemy/connectors-weblibrary by introducing thealchemySmartWalletconnector, updating existing connectors, and refining the integration with Alchemy's authentication and wallet APIs.Detailed summary
resolveConnector.tsandresolveAlchemyAuthConnector.mdx.package.jsonfiles to include new dependencies.alchemySmartWalletconnector with detailed options and parameters.resolveAlchemyAuthConnectorto improve connector resolution.alchemySmartWalletfunctionality.