-
Notifications
You must be signed in to change notification settings - Fork 528
Feat/dynamic server wallets #738
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
base: main
Are you sure you want to change the base?
Feat/dynamic server wallets #738
Conversation
🟡 Heimdall Review Status
|
Hi @murrlincoln, let me know if you'd like anything changed here! |
Hi @matthew1809, thanks for your contribution! Could you please rebase against main and bump the dynamic packages to the latest versions (if appropriate), then I will do some testing. A few initial comments below |
Please make sure to run pnpm run format and pnpm run lint before you commit, there are several unused imports/vars that would lead to failed checks |
"@coinbase/agentkit": minor | ||
--- | ||
|
||
Adds Dynamic as wallet provider |
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.
minor -> patch
Adds -> Added
DYNAMIC_BASE_API_URL=https://app.dynamicauth.com | ||
DYNAMIC_BASE_MPC_RELAY_API_URL=https://relay.dynamicauth.com | ||
|
||
# Optional Network ID. If you'd like to use a Dynamic Solana wallet, set to "solana-devnet". Otherwise, defaults to "base-sepolia" |
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.
defaults to "mainnet-beta" according to chatbot code
"dependencies": { | ||
"@coinbase/agentkit": "workspace:*", | ||
"@coinbase/agentkit-langchain": "^0.3.0", | ||
"@dynamic-labs-wallet/core": "^0.0.71", |
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.
Are the dynamic dependencies needed here? If so, should use the same versions as in typescript/agentkit/package.json.
Please update other dep to be consistent with https://github.com/coinbase/agentkit/blob/main/typescript/examples/langchain-cdp-chatbot/package.json
pythActionProvider(), | ||
walletActionProvider(), | ||
erc20ActionProvider(), | ||
cdpApiActionProvider({ |
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.
Following #785, cdpApiActionProvider needs to be renamed to legacyCdpApiActionProvider
apiKeyName: process.env.CDP_API_KEY_NAME as string, | ||
apiKeyPrivateKey: process.env.CDP_API_KEY_PRIVATE_KEY as string, | ||
}), | ||
splActionProvider(), |
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.
This is mixing up evm-only and svm-only actions, while the wallet setup is only for svm.
I'd suggest to check from config vars wether evm or svm wallet should be initialized and then add actions accordingly. Have a look at https://github.com/coinbase/agentkit/blob/main/typescript/examples/langchain-cdp-chatbot/chatbot.ts for inspiration
It would be great if you could include the result of some manual end-to-end testing in the PR description including prompt + reply for both evm and svm wallets |
type: "json-rpc", | ||
}, | ||
chain, | ||
transport: http(), |
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.
Following #824, this should include an optional config to set custom RPC_URL
throw new Error(`Chain with ID ${chainId} not found`); | ||
} | ||
|
||
const publicClient = (dynamic as DynamicEvmWalletClient).createViemPublicClient({ |
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.
publicClient appears to be unused, please remove
55c414c
to
12b91f3
Compare
Description
Add Dynamic as a wallet provider (PR for app generation to follow)
Tests
I tested with the example chatbot.
Other