-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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(new-plugin): A plugin for the Zilliqa blockchain #2455
base: develop
Are you sure you want to change the base?
Conversation
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.
Hi @rrw-zilliqa! Welcome to the elizaOS community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now an elizaOS contributor!
eb58885
to
48ab933
Compare
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces the Zilliqa plugin to the ElizaOS agent ecosystem. The changes include adding a new package Changes
Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (8)
packages/plugin-zilliqa/src/wallet.ts (3)
33-52
: Ensure consistent error handling ingetZilliqaWalletClient
The function returns
null
when theprivateKey
is missing but throws an error when theprovider
is missing. This inconsistency can lead to confusion and potential runtime errors downstream. Consider throwing an error in both cases or handling both uniformly.Option 1: Throw an error when
privateKey
is missing.export async function getZilliqaWalletClient( getSetting: (key: string) => string | undefined ): Promise<ZilliqaWalletClient | null> { const privateKey = getSetting("EVM_PRIVATE_KEY"); - if (!privateKey) return null; + if (!privateKey) throw new Error("EVM_PRIVATE_KEY not configured"); const provider = getSetting("EVM_PROVIDER_URL"); if (!provider) throw new Error("EVM_PROVIDER_URL not configured"); // ... }Option 2: Return
null
when either is missing.export async function getZilliqaWalletClient( getSetting: (key: string) => string | undefined ): Promise<ZilliqaWalletClient | null> { const privateKey = getSetting("EVM_PRIVATE_KEY"); if (!privateKey) return null; const provider = getSetting("EVM_PROVIDER_URL"); - if (!provider) throw new Error("EVM_PROVIDER_URL not configured"); + if (!provider) return null; // ... }
42-42
: Add error handling forzilliqaChainId
retrievalThe call to
zilliqaChainId(provider)
may fail due to network issues or invalid provider URLs. Consider wrapping this call in atry-catch
block to handle potential exceptions gracefully.Apply this diff:
const provider = getSetting("EVM_PROVIDER_URL"); if (!provider) throw new Error("EVM_PROVIDER_URL not configured"); -const chainId = await zilliqaChainId(provider); +let chainId: number; +try { + chainId = await zilliqaChainId(provider); +} catch (error) { + throw new Error(`Failed to retrieve chain ID: ${error.message}`); +}
54-84
: Specify return types ingetWalletProviders
The
getWalletProviders
function and its innerget
methods lack explicit return types, which can affect readability and maintainability.Add return types to the function and methods:
export function getWalletProviders( walletClient: WalletClientBase, zilliqa: ZilliqaWalletClient -) +): { get(): Promise<string | null> }[] { return [ { async get(): Promise<string | null> { // ... }, }, { async get(): Promise<string | null> { // ... }, }, ]; }packages/plugin-zilliqa/src/actions.ts (1)
155-218
: Add type annotations forgetActionHandler
parameters and return typeExplicit type annotations enhance code readability and maintainability.
Add type annotations:
function getActionHandler( actionName: string, actionDescription: string, tools: Record<string, unknown> -) +): HandlerCallback { return async ( runtime: IAgentRuntime, message: Memory, - state: State | undefined, + state?: State, options?: Record<string, unknown>, callback?: HandlerCallback - ): Promise<boolean> => { + ) => { // ... }; }Ensure that
HandlerCallback
is correctly imported or defined.packages/plugin-zilliqa/src/index.ts (1)
15-15
: EnsurezilliqaWalletClient
is notnull
before passing to providersPassing a potentially
null
zilliqaWalletClient
togetWalletProviders
can lead to unexpected behavior.Since you've added a null check above, this issue would be resolved. Ensure that any functions receiving
zilliqaWalletClient
handle it appropriately.packages/plugin-zilliqa/package.json (1)
2-4
: Consider starting with version 0.1.0-alpha.1.Since this is a new package, it's recommended to start with version 0.1.0-alpha.1 instead of 0.1.7-alpha.2.
🧰 Tools
🪛 GitHub Actions: Integration Tests
[warning] Peer dependency conflict: @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0 but found 19.0.0
[warning] Peer dependency conflict: typedoc requires typescript 4.6.x || 4.7.x || 4.8.x || 4.9.x || 5.0.x || 5.1.x || 5.2.x || 5.3.x || 5.4.x || 5.5.x || 5.6.x but found 5.7.3
[warning] Missing peer dependency: vue@>=3.3.4 <4.0.0-0 is required but not installed
packages/plugin-goat/package.json (1)
24-24
: Align all GOAT SDK package versions.Consider updating other GOAT SDK packages to match the latest versions used in plugin-zilliqa:
- @goat-sdk/adapter-vercel-ai: 0.2.0 → 0.2.7
- @goat-sdk/wallet-evm: 0.2.0 → 0.2.6
- @goat-sdk/wallet-viem: 0.2.0 → 0.2.6
🧰 Tools
🪛 GitHub Actions: Integration Tests
[warning] Peer dependency conflict: @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0 but found 19.0.0
[warning] Peer dependency conflict: typedoc requires typescript 4.6.x || 4.7.x || 4.8.x || 4.9.x || 5.0.x || 5.1.x || 5.2.x || 5.3.x || 5.4.x || 5.5.x || 5.6.x but found 5.7.3
[warning] Missing peer dependency: vue@>=3.3.4 <4.0.0-0 is required but not installed
packages/plugin-zilliqa/README.md (1)
34-35
: Improve text clarity and grammar.Add missing commas and improve sentence structure:
-If you are using Trump as a character it might be tricky to get them to perform any action since the character is full of prompts that aim to change the topic of the conversation. To fix this try using a different character or create your own with prompts that are more suitable to what the agent is supposed to do. +If you are using Trump as a character, it might be tricky to get them to perform any action since the character is full of prompts that aim to change the topic of the conversation. To fix this, try using a different character or create your own with prompts that are more suitable to what the agent is supposed to do.🧰 Tools
🪛 LanguageTool
[style] ~34-~34: Consider using a different verb for a more formal wording.
Context: ... action. Removing the EVM Plugin should fix this issue. There is no need for you to...(FIX_RESOLVE)
[uncategorized] ~35-~35: A comma might be missing here.
Context: ...time. - If you are using Trump as a character it might be tricky to get them to perfo...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~35-~35: A comma might be missing here.
Context: ...e the topic of the conversation. To fix this try using a different character or crea...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
agent/package.json
(1 hunks)agent/src/index.ts
(3 hunks)packages/plugin-goat/package.json
(1 hunks)packages/plugin-zilliqa/README.md
(1 hunks)packages/plugin-zilliqa/package.json
(1 hunks)packages/plugin-zilliqa/src/actions.ts
(1 hunks)packages/plugin-zilliqa/src/index.ts
(1 hunks)packages/plugin-zilliqa/src/wallet.ts
(1 hunks)packages/plugin-zilliqa/tsconfig.json
(1 hunks)packages/plugin-zilliqa/tsup.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/plugin-zilliqa/tsconfig.json
- packages/plugin-zilliqa/tsup.config.ts
🧰 Additional context used
🪛 GitHub Actions: Integration Tests
agent/package.json
[warning] Peer dependency conflict: @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0 but found 19.0.0
[warning] Peer dependency conflict: typedoc requires typescript 4.6.x || 4.7.x || 4.8.x || 4.9.x || 5.0.x || 5.1.x || 5.2.x || 5.3.x || 5.4.x || 5.5.x || 5.6.x but found 5.7.3
[warning] Missing peer dependency: vue@>=3.3.4 <4.0.0-0 is required but not installed
packages/plugin-goat/package.json
[warning] Peer dependency conflict: @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0 but found 19.0.0
[warning] Peer dependency conflict: typedoc requires typescript 4.6.x || 4.7.x || 4.8.x || 4.9.x || 5.0.x || 5.1.x || 5.2.x || 5.3.x || 5.4.x || 5.5.x || 5.6.x but found 5.7.3
[warning] Missing peer dependency: vue@>=3.3.4 <4.0.0-0 is required but not installed
packages/plugin-zilliqa/package.json
[warning] Peer dependency conflict: @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0 but found 19.0.0
[warning] Peer dependency conflict: typedoc requires typescript 4.6.x || 4.7.x || 4.8.x || 4.9.x || 5.0.x || 5.1.x || 5.2.x || 5.3.x || 5.4.x || 5.5.x || 5.6.x but found 5.7.3
[warning] Missing peer dependency: vue@>=3.3.4 <4.0.0-0 is required but not installed
🪛 LanguageTool
packages/plugin-zilliqa/README.md
[style] ~34-~34: Consider using a different verb for a more formal wording.
Context: ... action. Removing the EVM Plugin should fix this issue. There is no need for you to...
(FIX_RESOLVE)
[uncategorized] ~35-~35: A comma might be missing here.
Context: ...time. - If you are using Trump as a character it might be tricky to get them to perfo...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~35-~35: A comma might be missing here.
Context: ...e the topic of the conversation. To fix this try using a different character or crea...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🔇 Additional comments (6)
packages/plugin-zilliqa/src/wallet.ts (2)
63-65
: Verify the balance unit returned bywalletClient.balanceOf
Ensure that
walletClient.balanceOf(address)
returns the balance in ZIL. Since this is an EVM wallet client, the balance might be in a different unit (e.g., ETH or wei).Please confirm the unit of the balance and, if necessary, convert it to ZIL for accurate reporting.
75-76
: Handle potential undefineddefaultAccount
in Zilliqa walletThe expression
zilliqa.getZilliqa().wallet.defaultAccount?.address
may returnundefined
ifdefaultAccount
is not set. Consider adding a check to handle this case.Apply this diff to handle undefined
address
:const address = zilliqa.getZilliqa().wallet.defaultAccount?.address; +if (!address) { + return null; +} return `Zilliqa wallet address: ${address}\n`;agent/src/index.ts (1)
48-48
: LGTM! Plugin initialization follows established patterns.The conditional initialization based on
EVM_PRIVATE_KEY
and secret retrieval is implemented correctly.Also applies to: 842-847
packages/plugin-zilliqa/package.json (1)
8-17
: Verify compatibility between GOAT SDK versions.Ensure that the different GOAT SDK packages (@goat-sdk/*) at different versions (0.1.3, 0.2.6, 0.2.7, 0.4.6) are compatible with each other.
🧰 Tools
🪛 GitHub Actions: Integration Tests
[warning] Peer dependency conflict: @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0 but found 19.0.0
[warning] Peer dependency conflict: typedoc requires typescript 4.6.x || 4.7.x || 4.8.x || 4.9.x || 5.0.x || 5.1.x || 5.2.x || 5.3.x || 5.4.x || 5.5.x || 5.6.x but found 5.7.3
[warning] Missing peer dependency: vue@>=3.3.4 <4.0.0-0 is required but not installed
agent/package.json (2)
114-114
: LGTM! Zilliqa plugin dependency added correctly.The dependency follows the workspace pattern and maintains alphabetical ordering.
🧰 Tools
🪛 GitHub Actions: Integration Tests
[warning] Peer dependency conflict: @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0 but found 19.0.0
[warning] Peer dependency conflict: typedoc requires typescript 4.6.x || 4.7.x || 4.8.x || 4.9.x || 5.0.x || 5.1.x || 5.2.x || 5.3.x || 5.4.x || 5.5.x || 5.6.x but found 5.7.3
[warning] Missing peer dependency: vue@>=3.3.4 <4.0.0-0 is required but not installed
Line range hint
1-124
: Verify peer dependency conflicts.Pipeline shows warnings about peer dependencies. While not directly related to the Zilliqa plugin, these should be addressed:
- @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0
- typedoc requires typescript 4.6.x - 5.6.x
- Missing vue >= 3.3.4
Let's check the dependency tree to identify the source packages:
🧰 Tools
🪛 GitHub Actions: Integration Tests
[warning] Peer dependency conflict: @react-spring/web requires react ^16.8.0 || ^17.0.0 || ^18.0.0 but found 19.0.0
[warning] Peer dependency conflict: typedoc requires typescript 4.6.x || 4.7.x || 4.8.x || 4.9.x || 5.0.x || 5.1.x || 5.2.x || 5.3.x || 5.4.x || 5.5.x || 5.6.x but found 5.7.3
[warning] Missing peer dependency: vue@>=3.3.4 <4.0.0-0 is required but not installed
1f29398
to
961a810
Compare
(fix) Add action to example (feat) Document zilliqa enable secret
961a810
to
a7d3d7c
Compare
@rrw-zilliqa could you allow us to push to this branch!!! |
|
Relates to
No issue yet; can file an issue if you'd like?
Risks
Additional functionality; may affect the workspace package list.
Background
What does this PR do?
This PR adds a plugin to support the Zilliqa blockchain and ecosystem so that developers on the Zilliqa ecosystem can use Eliza's features.
What kind of change is this?
Feature
Documentation changes needed?
My changes do not require a change to the project documentation.
Testing
Where should a reviewer start?
Define
ENABLE_ZILLIQA
,EVM_PROVIDER_URL
andEVM_PRIVATE_KEY
.You should then be able to request the below.
Detailed testing steps