-
Notifications
You must be signed in to change notification settings - Fork 66
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: hyperliquid #71
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces a comprehensive implementation of a Hyperliquid trading bot, adding support for Hyperliquid trading functionality across multiple files. The changes include creating a new Hyperliquid client, adding environment variables for Hyperliquid credentials, implementing a trading example script, and defining a new trading character. The implementation provides capabilities for placing limit and market orders, retrieving account balances, and managing trading operations through an interactive console interface. Changes
Assessment against linked issues
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 3
🧹 Nitpick comments (7)
packages/core/src/core/io/hyperliquid.ts (3)
35-41
: Good initialization sequence, but consider environment-based defaults fortestnet
.Currently,
testnet
defaults tofalse
. If you regularly work in a test/staging environment, you may want to derive this from an environment variable or a configuration setting to prevent accidental production orders.
272-299
: Consider makingreduce_only
configurable.The
reduce_only: false
parameter is hard-coded for all orders. If you need partial closing of positions (especially for risk management or exit strategies), exposing areduce_only
option to your placeOrder method may offer more flexibility.
141-158
: Potential partial-failure handling for concurrent sells.When calling
marketSellPositions
with multiple tickers, a single failure will throw an error and halt the entirePromise.all()
operation. If partial success is acceptable, consider adding a structured approach to handle partial failures (e.g., continuing to process remaining tickers even if one fails).packages/core/src/core/env.ts (1)
15-17
: Be mindful of storing private keys in environment variables.Storing private keys in plain text env variables might pose a security risk if logs or stack traces leak. Consider using a secure vault or key management service. If environment variables are the only option, ensure that logs and error messages never expose them.
packages/core/src/core/character_trading_sage.ts (2)
110-110
: Fix capitalization in contextRules.The word "emphasize" should be capitalized for consistency with other rules.
- "emphasize risk management in volatile markets", + "Emphasize risk management in volatile markets",
115-130
: Enhance tweet template with market risk disclaimers.The tweet template should include guidance for adding appropriate risk disclaimers.
tweetTemplate: ` <thinking id="tweet_template"> As {{name}}, share a practical trading insight or reminder that promotes thoughtful trading. Rules: - Keep it actionable but not specific financial advice - Use at most one emoji - Focus on risk management or psychology + - Include appropriate risk disclaimers + - Avoid mentioning specific assets or trading pairs Context: {{context}} Market condition: {{market_context}} Key focus: {{trading_topic}} Aim to promote disciplined trading while maintaining a supportive tone. </thinking>package.json (1)
10-10
: Group related scripts together.Consider grouping the hyperliquid script with other trading-related scripts for better organization.
"scripts": { "goal": "bun run examples/example-goal.ts", "task": "bun run examples/example-basic.ts", "twitter": "bun run examples/example-twitter.ts", "server": "bun run examples/example-server.ts", "discord": "bun run examples/example-discord.ts", - "hyperliquid": "bun run examples/example-hyperliquid.ts", "api": "bun run examples/example-api.ts", + + "# Trading scripts" + "hyperliquid": "bun run examples/example-hyperliquid.ts", + "ui": "pnpm --dir clients/example-ui run dev",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.env.example
(1 hunks)examples/example-hyperliquid.ts
(1 hunks)package.json
(2 hunks)packages/core/src/core/character_trading_sage.ts
(1 hunks)packages/core/src/core/env.ts
(1 hunks)packages/core/src/core/io/hyperliquid.ts
(1 hunks)
🔇 Additional comments (2)
packages/core/src/core/io/hyperliquid.ts (1)
215-217
: Verify correct orderbook indexing for best bid/ask.You're accessing
orderbook.levels[1]
for the ask price whenis_buy
is true andorderbook.levels[0]
otherwise. Verify that the Hyperliquid orderbook data is ordered such that:
levels[0]
is indeed the best price for one side (usually the best ask or bid),levels[1]
is the next best price, etc.If the indexing is reversed or differs, this might lead to unexpected slippage or missed fills.
package.json (1)
27-27
: Verify Hyperliquid package version compatibility.The hyperliquid package version 1.5.8 should be verified for:
- Compatibility with the current Node.js version
- Known security vulnerabilities
- Latest available version
✅ Verification successful
Hyperliquid package version is optimal and secure ✅
The package is using the latest available version (1.5.8) and has no known security vulnerabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check hyperliquid package version and security advisories # Check latest version npm view hyperliquid versions --json | jq -r '.[-1]' # Check for security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "hyperliquid") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 405
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: 7
🧹 Nitpick comments (1)
examples/example-hyperliquid.ts (1)
112-114
: Use decimal.js for price calculations.Using regular floating-point arithmetic for price calculations can lead to precision issues. Consider using a decimal arithmetic library for accurate calculations.
+import { Decimal } from 'decimal.js'; // Example for one of the log messages: chalk.blue( `🔍 ${message.is_buy ? "Buying" : "Selling"} ${message.sz}x${message.ticker} at ${message.limit_px} (total $${ + new Decimal(message.limit_px).mul(message.sz).toFixed(2) - message.limit_px * message.sz })...` )Also applies to: 147-149, 221-224, 258-260
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example
(1 hunks)examples/example-hyperliquid.ts
(1 hunks)packages/core/src/core/character_trading_sage.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/core/character_trading_sage.ts
- .env.example
🔇 Additional comments (1)
examples/example-hyperliquid.ts (1)
38-38
: Review database purging strategy.Purging databases on startup (
vectorDb.purge()
andscheduledTaskDb.deleteAll()
) might lead to data loss in production. Consider making this behavior configurable based on the environment.Would you like me to suggest an implementation that makes database purging configurable?
Also applies to: 65-65
execute: async (data: unknown) => { | ||
const message = data as { | ||
ticker: string; | ||
sz: number; | ||
limit_px: number; | ||
is_buy: boolean; | ||
}; | ||
console.log( | ||
chalk.blue( | ||
`🔍 ${message.is_buy ? "Buying" : "Selling"} ${message.sz}x${message.ticker} at ${message.limit_px} (total $${message.limit_px * message.sz})...` | ||
) | ||
); | ||
return await hyperliquid.placeLimitOrderInstantOrCancel( | ||
message.ticker, | ||
message.sz, | ||
message.limit_px, | ||
message.is_buy | ||
); | ||
}, |
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.
🛠️ Refactor suggestion
Standardize error handling across handlers.
Error handling is inconsistent across handlers. Some handlers have try-catch blocks (e.g., hyperliquid_place_market_order
), while others don't (e.g., hyperliquid_place_limit_order_instantorcancel
). Standardize error handling to ensure consistent behavior.
Here's an example implementation for one handler:
execute: async (data: unknown) => {
const message = data as {
ticker: string;
sz: number;
limit_px: number;
is_buy: boolean;
};
console.log(
chalk.blue(
`🔍 ${message.is_buy ? "Buying" : "Selling"} ${message.sz}x${message.ticker} at ${message.limit_px} (total $${message.limit_px * message.sz})...`
)
);
+ try {
return await hyperliquid.placeLimitOrderInstantOrCancel(
message.ticker,
message.sz,
message.limit_px,
message.is_buy
);
+ } catch (err) {
+ console.error(chalk.red("Error placing limit order:"), err);
+ throw err;
+ }
}
Apply similar error handling to all handlers.
Also applies to: 139-157, 167-177, 185-189, 196-200, 215-235, 251-271, 286-293
ticker: z | ||
.string() | ||
.describe( | ||
"Ticker must be only the letter of the ticker in uppercase without the -PERP or -SPOT suffix" | ||
), |
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.
🛠️ Refactor suggestion
Enhance ticker validation in schema.
The current schema only describes the ticker format but doesn't enforce it. Add pattern validation to ensure tickers follow the required format.
ticker: z
.string()
+ .regex(/^[A-Z]+$/, "Ticker must contain only uppercase letters")
.describe(
"Ticker must be only the letter of the ticker in uppercase without the -PERP or -SPOT suffix"
)
Also applies to: 130-134, 207-211, 279-283
}); | ||
|
||
// Start the prompt loop | ||
console.log(chalk.cyan("🤖 Bot is now running and monitoring Discord...")); |
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.
Update console message to remove Discord reference.
The console message mentions Discord monitoring, but this appears to be a standalone console application.
-console.log(chalk.cyan("🤖 Bot is now running and monitoring Discord..."));
+console.log(chalk.cyan("🤖 Bot is now running..."));
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
console.log(chalk.cyan("🤖 Bot is now running and monitoring Discord...")); | |
console.log(chalk.cyan("🤖 Bot is now running...")); |
const hyperliquid = new HyperliquidClient( | ||
{ | ||
mainAddress: env.HYPERLIQUID_MAIN_ADDRESS, | ||
walletAddress: env.HYPERLIQUID_WALLET_ADDRESS, | ||
privateKey: env.HYPERLIQUID_PRIVATE_KEY, | ||
}, | ||
loglevel | ||
); |
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.
🛠️ Refactor suggestion
Add validation for required environment variables.
The Hyperliquid client initialization uses environment variables without validation. Add checks to ensure these required credentials are present before initialization.
+ // Validate required environment variables
+ if (!env.HYPERLIQUID_MAIN_ADDRESS || !env.HYPERLIQUID_WALLET_ADDRESS || !env.HYPERLIQUID_PRIVATE_KEY) {
+ throw new Error(
+ "Missing required environment variables: HYPERLIQUID_MAIN_ADDRESS, HYPERLIQUID_WALLET_ADDRESS, HYPERLIQUID_PRIVATE_KEY"
+ );
+ }
const hyperliquid = new HyperliquidClient(
{
mainAddress: env.HYPERLIQUID_MAIN_ADDRESS,
walletAddress: env.HYPERLIQUID_WALLET_ADDRESS,
privateKey: env.HYPERLIQUID_PRIVATE_KEY,
},
loglevel
);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const hyperliquid = new HyperliquidClient( | |
{ | |
mainAddress: env.HYPERLIQUID_MAIN_ADDRESS, | |
walletAddress: env.HYPERLIQUID_WALLET_ADDRESS, | |
privateKey: env.HYPERLIQUID_PRIVATE_KEY, | |
}, | |
loglevel | |
); | |
// Validate required environment variables | |
if (!env.HYPERLIQUID_MAIN_ADDRESS || !env.HYPERLIQUID_WALLET_ADDRESS || !env.HYPERLIQUID_PRIVATE_KEY) { | |
throw new Error( | |
"Missing required environment variables: HYPERLIQUID_MAIN_ADDRESS, HYPERLIQUID_WALLET_ADDRESS, HYPERLIQUID_PRIVATE_KEY" | |
); | |
} | |
const hyperliquid = new HyperliquidClient( | |
{ | |
mainAddress: env.HYPERLIQUID_MAIN_ADDRESS, | |
walletAddress: env.HYPERLIQUID_WALLET_ADDRESS, | |
privateKey: env.HYPERLIQUID_PRIVATE_KEY, | |
}, | |
loglevel | |
); |
const scheduledTaskDb = new MongoDb( | ||
"mongodb://localhost:27017", | ||
"myApp", | ||
"scheduled_tasks" | ||
); |
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.
🛠️ Refactor suggestion
Move MongoDB connection URL to environment variables.
Hardcoding the MongoDB connection URL reduces configurability and poses security risks.
const scheduledTaskDb = new MongoDb(
- "mongodb://localhost:27017",
+ env.MONGODB_URL || "mongodb://localhost:27017",
"myApp",
"scheduled_tasks"
);
Committable suggestion skipped: line range outside the PR's diff.
execute: async (payload) => { | ||
return payload; | ||
}, |
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.
🛠️ Refactor suggestion
Add input validation to chat handler.
The user_chat
handler forwards the payload without validation. Add input validation to ensure message format consistency.
execute: async (payload) => {
+ const schema = z.object({
+ content: z.string().min(1),
+ userId: z.string()
+ });
+ const validated = schema.parse(payload);
- return payload;
+ return validated;
}
Committable suggestion skipped: line range outside the PR's diff.
main().catch((error) => { | ||
console.error(chalk.red("Fatal error:"), error); | ||
process.exit(1); | ||
}); |
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.
🛠️ Refactor suggestion
Enhance error handling and cleanup in main function.
The error handling could be more informative and should include cleanup operations before exit.
main().catch(async (error) => {
console.error(chalk.red("Fatal error:"), error);
+ // Attempt cleanup
+ try {
+ if (scheduledTaskDb) {
+ await scheduledTaskDb.disconnect();
+ }
+ // Add any other cleanup operations
+ console.log(chalk.yellow("Cleanup completed"));
+ } catch (cleanupError) {
+ console.error(chalk.red("Error during cleanup:"), cleanupError);
+ }
process.exit(1);
});
Committable suggestion skipped: line range outside the PR's diff.
fixes #44
Summary by CodeRabbit
Release Notes
New Features
Configuration
Development