-
Notifications
You must be signed in to change notification settings - Fork 590
feat: superfluid typescript action provider #823
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
feat: superfluid typescript action provider #823
Conversation
✅ Heimdall Review Status
|
|
Hi @gtspencer, thanks for the contribution! Looks pretty good, a few initial comments below before I will do some manual testing |
|
We require all commits to be verified, unfortunately your last one is not signed. Please rebase against main and resign the commit |
|
Could you please add more comprehensive test output to the PR description, ideally prompts for all new actions? |
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "@coinbase/agentkit": minor | |||
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
| export * from "./schemas"; | ||
| export * from "./superfluidStreamActionProvider"; | ||
| export * from "./superfluidPoolActionProvider"; | ||
| export * from "./superfluidQueryActionProvider"; |
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.
might be good to add a 'SuperfluidActionProvider' wrapper combining the 3 action providers for convenience
| export const SuperfluidCreateStreamSchema = z | ||
| .object({ | ||
| erc20TokenAddress: z.string().describe("The ERC20 token to start or update streaming"), | ||
| chainId: z.string().describe("The EVM chain ID on which the ERC20 is deployed"), |
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.
it appears chainId is unused
| ).args; | ||
|
|
||
| if (success) { | ||
| return `Created pool of token ${args.erc20TokenAddress} at ${poolAddress}`; |
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.
please add txHashs in all returns
| * @param network - The network to check. | ||
| * @returns True if the Superfluid action provider supports the network, false otherwise. | ||
| */ | ||
| supportsNetwork = (network: Network) => network.protocolFamily === "evm"; |
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.
should be restricted to base, base-sepolia here
| */ | ||
| export const SuperfluidDeleteStreamSchema = z | ||
| .object({ | ||
| erc20TokenAddress: z.string().describe("The ERC20 token to start streaming"), |
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.
suggest to add .regex(/^0x[a-fA-F0-9]{40}$/, "Invalid Ethereum address format") to all addresses
|
|
||
| return `Updated member units of pool ${args.poolAddress} for member ${args.recipientAddress}, with new member units ${args.units}`; | ||
| } catch (error) { | ||
| return `Error creating Superfluid pool: ${error}`; |
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.
creating -> updating
|
|
||
| return `Current outflows are ${JSON.stringify(activeOutflows)}`; | ||
| } catch (error) { | ||
| return `Error creating Superfluid pool: ${error}`; |
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.
creating -> querying
|
|
||
| return `Updated stream of token ${args.erc20TokenAddress} to ${args.recipientAddress} at a rate of ${args.flowRate}`; | ||
| } catch (error) { | ||
| return `Error creating Superfluid stream: ${error}`; |
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.
creating -> updating
|
|
||
| return `Stopped stream of token ${args.erc20TokenAddress} to ${args.recipientAddress}`; | ||
| } catch (error) { | ||
| return `Error creating Superfluid stream: ${error}`; |
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.
creating -> stopping
|
Main concern is Superfluid wrapped tokens vs erc20. It is my understanding that you cant stream a erc20 token directly but need to supply a superfluid wrapped token. Ideally, the createStream action would first wrap a given erc20 token before calling createFlow or alternatively a new action added that does the wrapping. |
1d03ef4 to
e3f5f84
Compare
|
hey @phdargen thanks for the review! just addressed all your concerns:
Ready for a new review at your leisure! |
7487dfd to
716b63d
Compare
| }, | ||
| ] as const; | ||
|
|
||
| export const SuperTokenFactoryAddress = "0xe20B9a38E0c96F61d1bA6b42a61512D56Fea1Eb3" as const; |
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.
superTokenFactory address is different on base-sepolia (0x7447E94Dfe3d804a9f46Bf12838d467c912C8F6C), see https://docs.superfluid.org/docs/protocol/contract-addresses. Would be good if users could try the workflow on testnet
| export * from "./schemas"; | ||
| export * from "./superfluidStreamActionProvider"; | ||
| export * from "./superfluidPoolActionProvider"; | ||
| export * from "./superfluidQueryActionProvider"; | ||
| export * from "./superfluidWrapperActionProvider"; | ||
| export * from "./superfluidSuperTokenCreatorActionProvider"; |
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.
Please move exports back to index.ts.
Instead add the following here that aggregates all superfluid actions a for a single import in eg chatbot files:
| export * from "./schemas"; | |
| export * from "./superfluidStreamActionProvider"; | |
| export * from "./superfluidPoolActionProvider"; | |
| export * from "./superfluidQueryActionProvider"; | |
| export * from "./superfluidWrapperActionProvider"; | |
| export * from "./superfluidSuperTokenCreatorActionProvider"; | |
| import { ActionProvider } from "../actionProvider"; | |
| import { EvmWalletProvider } from "../../wallet-providers"; | |
| import { Network } from "../../network"; | |
| import { SuperfluidStreamActionProvider } from "./superfluidStreamActionProvider"; | |
| import { SuperfluidPoolActionProvider } from "./superfluidPoolActionProvider"; | |
| import { SuperfluidQueryActionProvider } from "./superfluidQueryActionProvider"; | |
| import { SuperfluidWrapperActionProvider } from "./superfluidWrapperActionProvider"; | |
| import { SuperfluidSuperTokenCreatorActionProvider } from "./superfluidSuperTokenCreatorActionProvider"; | |
| /** | |
| * SuperfluidActionProvider aggregates all Superfluid-related actions into a single provider. | |
| */ | |
| export class SuperfluidActionProvider extends ActionProvider<EvmWalletProvider> { | |
| constructor() { | |
| super("superfluid", [ | |
| new SuperfluidStreamActionProvider(), | |
| new SuperfluidPoolActionProvider(), | |
| new SuperfluidQueryActionProvider(), | |
| new SuperfluidWrapperActionProvider(), | |
| new SuperfluidSuperTokenCreatorActionProvider(), | |
| ]); | |
| } | |
| /** | |
| * Supports Base mainnet and Base sepolia like the underlying Superfluid providers. | |
| */ | |
| supportsNetwork = (network: Network) => | |
| network.networkId === "base-mainnet" || network.networkId === "base-sepolia"; | |
| } | |
| export const superfluidActionProvider = () => new SuperfluidActionProvider(); |
|
|
||
| return `Created super token for ${args.erc20TokenAddress}. Super token address at ${superTokenAddress} Transaction hash: ${createSuperTokenHash}`; | ||
| } catch (error) { | ||
| return `Error creating Superfluid stream: ${error}`; |
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.
stream -> token
| const publicClient = createPublicClient({ | ||
| chain: NETWORK_ID_TO_VIEM_CHAIN[walletProvider.getNetwork().networkId || "base-mainnet"], | ||
| transport: http(), | ||
| }); | ||
|
|
||
| // We don't get contract call results, so we need to simulate to get the pool address | ||
| const { result } = await publicClient.simulateContract({ |
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, please remove createPublicClient and use walletProvider.getPublicClient().simulateContract instead
| @@ -0,0 +1,187 @@ | |||
| # Superfluid Action Provider | |||
|
|
|||
| This directory contains the implementation for Superfluid, broken up by functionality: | |||
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.
Please add link to docs: https://docs.superfluid.org/
|
Please also add an entry in agentkit/README (https://github.com/coinbase/agentkit/blob/main/typescript/agentkit/README.md), should follow alphabetic order |
| name: "wrap_token", | ||
| description: ` | ||
| This tool will directly wrap an amount of ERC20 tokens into its corresponding Super token. | ||
| The user must provide the erc20 address, and the super token address. This will be done on Base Mainnet |
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.
remove "This will be done on Base Mainnet"
| * @returns A JSON string containing the account details or error message | ||
| */ | ||
| @CreateAction({ | ||
| name: "wrap_token", |
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.
wrap_token -> wrap_superfluid_token
| args: [], | ||
| }); | ||
|
|
||
| const amount = parseUnits(String(args.wrapAmount), Number(decimals)); |
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 didnt work well in my tests, the LLM always confused the units.
Try rephrasing the action description: "The user must provide the erc20 address, and the super token address. " -> " It takes the following inputs:
- erc20TokenAddress: The address of the ERC20 token to wrap
- superTokenAddress: The address of the Super token
- wrapAmount: The amount of tokens to wrap in whole units (e.g. 1.5 WETH, 10 USDC)
Use wrapAmount units exactly as provided, do not convert to wei or any other units.
"
and in shema: "The amount of token to wrap. This should be in units of token (and will not take decimals into account)" -> "The amount of tokens to wrap in whole units (e.g. 1.5 WETH, 10 USDC)"
| erc20TokenAddress: z | ||
| .string() | ||
| .regex(/^0x[a-fA-F0-9]{40}$/, "Invalid Ethereum address format") | ||
| .describe("The ERC20 Super token to start or update streaming"), | ||
| recipientAddress: z | ||
| .string() | ||
| .regex(/^0x[a-fA-F0-9]{40}$/, "Invalid Ethereum address format") | ||
| .describe("The EVM address to stream the token to."), | ||
| flowRate: z.string().describe("The rate at which the ERC20 is streamed to the recipient"), | ||
| }) |
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.
nit: erc20TokenAddress -> superTokenAddress, same in SuperfluidDeleteStreamSchema and SuperfluidCreatePoolSchema
| erc20TokenAddress: z | ||
| .string() | ||
| .regex(/^0x[a-fA-F0-9]{40}$/, "Invalid Ethereum address format") | ||
| .describe("The ERC20 token to start streaming"), |
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.
token -> Super token
| .string() | ||
| .regex(/^0x[a-fA-F0-9]{40}$/, "Invalid Ethereum address format") | ||
| .describe("The EVM address to stream the token to."), | ||
| flowRate: z.string().describe("The rate at which the ERC20 is streamed to the recipient"), |
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.
The flow rate needs additional explanation. Is this tokens per sec/min/h/day/...? Does it consider token decimals?
Please also add this info to action description
| const receipt = await walletProvider.waitForTransactionReceipt(createSuperTokenHash); | ||
|
|
||
| const superTokenAddress = extractCreatedSuperTokenAddressAbi(receipt); | ||
| console.log(superTokenAddress); |
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.
please remove all console.log
|
|
||
| const receipt = await walletProvider.waitForTransactionReceipt(createSuperTokenHash); | ||
|
|
||
| const superTokenAddress = extractCreatedSuperTokenAddressAbi(receipt); |
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.
Note: this fails for CdpSmartWallets as receipts doesnt include logs. You can ignore this, will look into it
Hi @gtspencer, thanks for the update! I tested it and it works well except for some confusion with the units. Please have a look at my follow-up comments, then we should be able to get this in |
f5dfe88 to
7a267b3
Compare
|
Hey @phdargen , thanks for the review! Updated everything, made some changes to the prompts, and added more information on flow rate. Let me know if there's anything else outstanding. Thank you! |
| }); | ||
|
|
||
| const createSuperTokenHash = await walletProvider.sendTransaction({ | ||
| to: SuperTokenFactoryAddress as `0x${string}`, |
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.
should be: to: walletProvider.getNetwork().networkId==="base-sepolia" ? SuperTokenFactoryAddress_Base_Sepolia as 0x${string}: SuperTokenFactoryAddress as0x${string},
Hi @gtspencer, looks good! Just one more fix to use the correct contract depending on network |
4513f32 to
86b6ba7
Compare
|
oops! Just updated! @phdargen |
|
This is tested and good to go imo @CarsonRoscoe |
Link to Issue
Description
Superfluid action providers, broken up by functionality (create stream, create pool, update stream, update pool). We experienced need for Superfluid functionality in our agent and decided to include the implementation in the core repo.
Qualified Impact
Very little qualified impact due to the self contained nature of the action. Only dependency is the graphql-request package, which can be omitted in the event of an error.
Pool Actions:
Stream Actions:
Query Actions (easily extensible for other queries):
Create Super Token
Wrap Token
Tests
Create Super Token
Wrap Super Token
Create Stream
Pools
Checklist
A couple of things to include in your PR for completeness: