feat: add Bitcoin plugin with Taproot and Ark #1553
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @tiero! Welcome to the ai16z community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now a ai16z contributor!
|
Hey @odilitime let me know what's may be needed to progress on this! I see the GH actions have not been approved for run, on my side all the test pass |
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive Bitcoin plugin for the Ark protocol, adding configuration options, a new package, and multiple actions for Bitcoin wallet interactions. The changes include environment configuration, a new Changes
Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (16)
packages/plugin-bitcoin/src/providers/bitcoin.ts (2)
48-59: Simplify error handling ingetBitcoinPriceRethrowing the error after logging leads to redundant error handling.
Consider removing the
console.errorto prevent duplicate logs:} catch (error) { - console.error("Error fetching Bitcoin price:", error); throw error; }
118-121: Improve error message clarityThe error message when the private key is missing can be more descriptive.
Update the message for better clarity:
if (!privateKey) { throw new Error( - "BITCOIN_PRIVATE_KEY environment variable is missing" + "Bitcoin provider initialization failed: BITCOIN_PRIVATE_KEY is missing." ); }packages/plugin-bitcoin/src/actions/send.ts (2)
22-22: Remove console.log from production codeLogging content may expose sensitive information.
Remove the
console.logstatement:- console.log("Content for bitcoin transfer:", content);
178-179: Safely access error messagesDirectly accessing
error.messagemay fail iferroris not anErrorinstance.Safely retrieve the error message:
text: `Error sending Bitcoin: ${error instanceof Error ? error.message : String(error)}`,packages/plugin-bitcoin/src/sdk.ts (1)
5-13: Use static imports instead of dynamic importsDynamic imports can complicate module resolution.
Simplify by using static imports:
-async function initSDK() { - const sdk = await import('@arklabs/wallet-sdk'); - InMemoryKey = sdk.InMemoryKey; - Wallet = sdk.Wallet; -} - -// Initialize the SDK -initSDK().catch(console.error); +// SDK initialized via static importpackages/plugin-bitcoin/src/types.ts (1)
15-23: Make BitcoinPrice interface more flexible.The interface only supports USD. Consider making it more generic to support multiple currencies.
-export interface BitcoinPrice { - USD: { +export interface BitcoinPrice<T extends string = string> { + [currency: T]: {packages/plugin-bitcoin/src/index.ts (1)
13-26: Add version information to plugin metadata.Include version information in the plugin metadata for better tracking and compatibility checks.
export const bitcoinPlugin: Plugin = { name: "bitcoin", + version: "1.0.0", description:packages/plugin-bitcoin/src/test-setup.ts (1)
17-30: Make mock values configurable.Replace hardcoded balance values with configurable ones to support different test scenarios.
+const DEFAULT_BALANCE = { + total: 150000000, + onchain: { + total: 130000000, + confirmed: 100000000, + unconfirmed: 30000000 + }, + offchain: { + total: 20000000, + settled: 20000000, + pending: 0, + swept: 0 + } +}; -getBalance: vi.fn().mockResolvedValue({ - total: 150000000, +getBalance: vi.fn().mockResolvedValue(config.balance || DEFAULT_BALANCE)packages/plugin-bitcoin/src/actions/balance.ts (1)
51-54: Consider handling Promise.all rejectionThe parallel execution of
getWalletBalanceandgetBitcoinPricecould fail independently. Consider handling partial failures gracefully.- const [balance, price] = await Promise.all([ - provider.getWalletBalance(), - provider.getBitcoinPrice(), - ]); + const [balanceResult, priceResult] = await Promise.allSettled([ + provider.getWalletBalance(), + provider.getBitcoinPrice(), + ]); + + if (balanceResult.status === 'rejected') { + throw new Error('Failed to fetch wallet balance'); + } + if (priceResult.status === 'rejected') { + console.warn('Price fetch failed, using fallback price'); + return { text: `Bitcoin Balance: ${balanceResult.value.total.toLocaleString()} sats` }; + } + + const balance = balanceResult.value; + const price = priceResult.value;packages/plugin-bitcoin/src/tests/balance.test.ts (1)
21-45: Add edge case tests for extreme valuesThe mock data uses "safe" values. Consider adding tests for edge cases:
- Maximum possible balance
- Zero balance
- Negative price movements
packages/plugin-bitcoin/src/tests/bitcoin.test.ts (1)
91-98: Add test for maximum amountWhile testing dust amount, also test the maximum allowed amount to ensure proper bounds checking.
+ it("should handle maximum amount", async () => { + const recipientAddress = "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx"; + const MAX_BITCOIN = 21_000_000n * 100_000_000n; + + await expect(provider.sendBitcoin({ + address: recipientAddress, + amount: MAX_BITCOIN + 1n + })).rejects.toThrow("Amount exceeds maximum possible Bitcoin supply"); + });packages/plugin-bitcoin/src/actions/addresses.ts (1)
20-54: Well-structured error handling and address display!The implementation properly handles both onchain and offchain scenarios with appropriate error handling.
Consider adding address validation before display to ensure the addresses are in the correct format for the selected network (mainnet/testnet).
.env.example (1)
363-364: Add validation hints for Ark server configuration.Consider adding format examples and validation requirements for the server URL and public key.
-ARK_SERVER_URL= # Ark protocol server URL -ARK_SERVER_PUBLIC_KEY= # Ark server public key for L2 operations +ARK_SERVER_URL= # Ark protocol server URL (e.g., https://ark.example.com) +ARK_SERVER_PUBLIC_KEY= # Ark server public key for L2 operations (33 or 65 bytes hex string)packages/plugin-bitcoin/src/tests/coins.test.ts (3)
16-39: Consider improving mock data readability and type safety.The mock provider implementation could be enhanced:
- Use constants for UTXO values to clarify their meaning
- Consider using a proper type-safe mock implementation instead of type casting
const mockProvider = { wallet: { getCoins: async () => [ { - txid: "tx1", - vout: 0, - value: 1000, + txid: "tx1", // Consider using TEST_TXID_1 + vout: 0, + value: 1000, // Consider using SATS_PER_TEST_UTXO status: { confirmed: true }, }, // ... similar changes for other mock UTXOs ], }, -} as unknown as BitcoinTaprootProvider; +} satisfies Partial<BitcoinTaprootProvider>;
57-73: Enhance test coverage for confirmed UTXOs.Consider adding assertions for:
- Total number of confirmed UTXOs
- Format of displayed values (e.g., decimal places)
- Error cases with malformed UTXO data
109-147: Well-implemented error handling test cases!The error scenarios are thoroughly covered. Consider adding specific error type checks for better error handling validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.env.example(1 hunks)agent/package.json(1 hunks)agent/src/index.ts(2 hunks)packages/plugin-bitcoin/package.json(1 hunks)packages/plugin-bitcoin/src/actions/addresses.ts(1 hunks)packages/plugin-bitcoin/src/actions/balance.ts(1 hunks)packages/plugin-bitcoin/src/actions/coins.ts(1 hunks)packages/plugin-bitcoin/src/actions/send.ts(1 hunks)packages/plugin-bitcoin/src/index.ts(1 hunks)packages/plugin-bitcoin/src/providers/bitcoin.ts(1 hunks)packages/plugin-bitcoin/src/sdk.ts(1 hunks)packages/plugin-bitcoin/src/test-setup.ts(1 hunks)packages/plugin-bitcoin/src/tests/balance.test.ts(1 hunks)packages/plugin-bitcoin/src/tests/bitcoin.test.ts(1 hunks)packages/plugin-bitcoin/src/tests/coins.test.ts(1 hunks)packages/plugin-bitcoin/src/types.ts(1 hunks)packages/plugin-bitcoin/src/types/index.ts(1 hunks)packages/plugin-bitcoin/tsconfig.json(1 hunks)packages/plugin-bitcoin/tsup.config.ts(1 hunks)packages/plugin-bitcoin/vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/plugin-bitcoin/vitest.config.ts
- packages/plugin-bitcoin/tsconfig.json
- packages/plugin-bitcoin/tsup.config.ts
🧰 Additional context used
🪛 Gitleaks (8.21.2)
packages/plugin-bitcoin/src/tests/bitcoin.test.ts
9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (4)
packages/plugin-bitcoin/src/actions/balance.ts (1)
60-62: 🛠️ Refactor suggestionPrevent potential numeric overflow
The calculations involving price conversion should handle potential numeric overflow for large amounts.
- const onchainUSD = (onchainTotal * price.USD.last) / 100_000_000; - const offchainUSD = (offchainTotal * price.USD.last) / 100_000_000; - const totalUSD = (total * price.USD.last) / 100_000_000; + const convertToUSD = (sats: number) => { + const BTC = Number(sats) / 100_000_000; + return BTC * price.USD.last; + }; + const onchainUSD = convertToUSD(onchainTotal); + const offchainUSD = convertToUSD(offchainTotal); + const totalUSD = convertToUSD(total);agent/package.json (1)
62-62: LGTM!The Bitcoin plugin dependency is correctly added and follows the workspace dependency pattern.
agent/src/index.ts (1)
604-604: Clean integration of Bitcoin plugin!The implementation follows the established pattern for blockchain plugins, maintaining consistency with the codebase.
packages/plugin-bitcoin/package.json (1)
18-19: Verify dependency versions for security and stability.
@arklabs/[email protected]is an early version that might be unstable[email protected]should be checked for vulnerabilities
| async sendBitcoin(params: { | ||
| address: string; | ||
| amount: bigint; | ||
| message?: string; | ||
| }) { | ||
| return this.wallet.sendBitcoin({ | ||
| address: params.address, | ||
| amount: Number(params.amount), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Avoid precision loss when converting bigint to number
In the sendBitcoin method, converting amount from bigint to number may cause precision loss for large amounts.
Ensure the amount retains its precision by keeping it as a bigint if the underlying SDK supports it, or validate that the amount is within the safe number range.
| // Convert amounts if needed | ||
| let amount: bigint | undefined; | ||
| if (content.amount) { | ||
| // Convert BTC to satoshis | ||
| amount = BigInt( | ||
| Math.floor(Number(content.amount) * 100_000_000) | ||
| ); | ||
| } else if (content.amountSats) { | ||
| // Already in satoshis | ||
| amount = BigInt(Math.floor(Number(content.amountSats))); | ||
| } else if (content.amountUSD) { | ||
| // Get current BTC price and convert USD to satoshis | ||
| const price = await provider.getBitcoinPrice(); | ||
| const btcAmount = Number(content.amountUSD) / price.USD.last; | ||
| amount = BigInt(Math.floor(btcAmount * 100_000_000)); | ||
| } |
There was a problem hiding this comment.
Validate amount inputs to prevent errors
Inputs for amounts are not validated and may cause runtime errors if invalid.
Add validation to ensure amounts are valid numbers before processing.
| const content = await generateObjectDeprecated({ | ||
| runtime, | ||
| context: transferContext, | ||
| modelClass: ModelClass.SMALL, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update deprecated function usage
The function generateObjectDeprecated is deprecated.
Replace with the updated function, such as generateObject:
const content = await generateObject({
runtime,
context: transferContext,
modelClass: ModelClass.SMALL,
});Committable suggestion skipped: line range outside the PR's diff.
| // @ts-ignore | ||
| let InMemoryKey: any; | ||
| let Wallet: any; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid suppressing TypeScript checks
Using @ts-ignore and any types reduces type safety.
Import types directly to maintain type safety:
-// @ts-ignore
-let InMemoryKey: any;
-let Wallet: any;
+import { InMemoryKey, Wallet } from '@arklabs/wallet-sdk';📝 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.
| // @ts-ignore | |
| let InMemoryKey: any; | |
| let Wallet: any; | |
| import { InMemoryKey, Wallet } from '@arklabs/wallet-sdk'; | |
| import { IAgentRuntime, Memory, State } from "@elizaos/core"; | ||
|
|
||
| export interface BitcoinConfig { | ||
| privateKey: string; | ||
| network: "bitcoin" | "testnet" | "mutinynet" | "regtest" | "signet"; | ||
| arkServerPublicKey?: string; | ||
| arkServerUrl?: string; | ||
| } | ||
|
|
||
| export interface SendBitcoinParams { | ||
| address: string; | ||
| amount?: bigint; // in satoshis, optional if sendAll is true | ||
| amountUSD?: number; // amount in USD | ||
| sendAll?: boolean; // if true, sends all available funds minus fee | ||
| feeRate?: number; // sats/vbyte, optional | ||
| } | ||
|
|
||
| export interface BitcoinPrice { | ||
| [currency: string]: { | ||
| "15m": number; | ||
| last: number; | ||
| buy: number; | ||
| sell: number; | ||
| symbol: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Remove duplicate type definitions.
This file duplicates interfaces from types.ts. Consolidate type definitions into a single location to prevent inconsistencies.
Additionally, there's a type inconsistency for the amount field:
types/index.ts:amount?: biginttypes.ts:amount?: number
This could lead to runtime errors when handling Bitcoin amounts.
| if (amount < 546n) { | ||
| throw new Error('Amount must be at least 546 sats (dust limit)'); | ||
| } | ||
| if (!address.startsWith('tb1') && !address.startsWith('tark1')) { | ||
| throw new Error('Invalid Bitcoin address'); | ||
| } | ||
| if (amount > 1000000000n) { | ||
| throw new Error('Insufficient funds'); | ||
| } | ||
| return 'txid'; | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling in sendBitcoin mock.
The mock implementation needs more comprehensive error cases:
- Network errors
- Rate limiting
- Invalid fee rates
- Double-spend attempts
sendBitcoin: vi.fn()
.mockImplementation(({ address, amount, feeRate }) => {
if (amount < 546n) {
throw new Error('Amount must be at least 546 sats (dust limit)');
}
if (!address.startsWith('tb1') && !address.startsWith('tark1')) {
throw new Error('Invalid Bitcoin address');
}
if (amount > 1000000000n) {
throw new Error('Insufficient funds');
}
+ if (feeRate && (feeRate < 1 || feeRate > 500)) {
+ throw new Error('Invalid fee rate');
+ }
+ if (Math.random() < 0.1) { // Simulate network errors
+ throw new Error('Network error');
+ }
return 'txid';
}),📝 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.
| if (amount < 546n) { | |
| throw new Error('Amount must be at least 546 sats (dust limit)'); | |
| } | |
| if (!address.startsWith('tb1') && !address.startsWith('tark1')) { | |
| throw new Error('Invalid Bitcoin address'); | |
| } | |
| if (amount > 1000000000n) { | |
| throw new Error('Insufficient funds'); | |
| } | |
| return 'txid'; | |
| }), | |
| if (amount < 546n) { | |
| throw new Error('Amount must be at least 546 sats (dust limit)'); | |
| } | |
| if (!address.startsWith('tb1') && !address.startsWith('tark1')) { | |
| throw new Error('Invalid Bitcoin address'); | |
| } | |
| if (amount > 1000000000n) { | |
| throw new Error('Insufficient funds'); | |
| } | |
| if (feeRate && (feeRate < 1 || feeRate > 500)) { | |
| throw new Error('Invalid fee rate'); | |
| } | |
| if (Math.random() < 0.1) { // Simulate network errors | |
| throw new Error('Network error'); | |
| } | |
| return 'txid'; | |
| }), |
| const confirmedTotal = confirmedCoins.reduce( | ||
| (sum, coin) => sum + coin.value, | ||
| 0 | ||
| ); | ||
| const unconfirmedTotal = unconfirmedCoins.reduce( | ||
| (sum, coin) => sum + coin.value, | ||
| 0 | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using BigInt for sats calculations
Bitcoin amounts should be handled as BigInt to prevent precision loss with large numbers.
- const confirmedTotal = confirmedCoins.reduce(
- (sum, coin) => sum + coin.value,
- 0
- );
- const unconfirmedTotal = unconfirmedCoins.reduce(
- (sum, coin) => sum + coin.value,
- 0
- );
+ const confirmedTotal = confirmedCoins.reduce(
+ (sum, coin) => sum + BigInt(coin.value),
+ 0n
+ );
+ const unconfirmedTotal = unconfirmedCoins.reduce(
+ (sum, coin) => sum + BigInt(coin.value),
+ 0n
+ );📝 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 confirmedTotal = confirmedCoins.reduce( | |
| (sum, coin) => sum + coin.value, | |
| 0 | |
| ); | |
| const unconfirmedTotal = unconfirmedCoins.reduce( | |
| (sum, coin) => sum + coin.value, | |
| 0 | |
| ); | |
| const confirmedTotal = confirmedCoins.reduce( | |
| (sum, coin) => sum + BigInt(coin.value), | |
| 0n | |
| ); | |
| const unconfirmedTotal = unconfirmedCoins.reduce( | |
| (sum, coin) => sum + BigInt(coin.value), | |
| 0n | |
| ); |
| const provider = runtime.providers.find( | ||
| (p) => p instanceof BitcoinTaprootProvider | ||
| ) as BitcoinTaprootProvider; |
There was a problem hiding this comment.
Unsafe type assertion
The type assertion to BitcoinTaprootProvider could fail at runtime. Consider using type guard pattern.
- const provider = runtime.providers.find(
- (p) => p instanceof BitcoinTaprootProvider
- ) as BitcoinTaprootProvider;
+ const provider = runtime.providers.find(
+ (p): p is BitcoinTaprootProvider => p instanceof BitcoinTaprootProvider
+ );📝 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 provider = runtime.providers.find( | |
| (p) => p instanceof BitcoinTaprootProvider | |
| ) as BitcoinTaprootProvider; | |
| const provider = runtime.providers.find( | |
| (p): p is BitcoinTaprootProvider => p instanceof BitcoinTaprootProvider | |
| ); |
| // Test private key (DO NOT USE IN PRODUCTION) | ||
| const testPrivateKey = "ce66c68f8875c0c98a502c666303dc183a21600130013c06f9d1edf60207abf2"; | ||
| const testServerPubKey = "02eec7245d6b7d2ccb30380bfbe2a3648cd7a942653f5aa340edcea1f283686619"; |
There was a problem hiding this comment.
Security: Remove hardcoded test keys
Even test private keys should not be committed to the repository. Consider using environment variables for test keys.
- const testPrivateKey = "ce66c68f8875c0c98a502c666303dc183a21600130013c06f9d1edf60207abf2";
- const testServerPubKey = "02eec7245d6b7d2ccb30380bfbe2a3648cd7a942653f5aa340edcea1f283686619";
+ const testPrivateKey = process.env.TEST_BITCOIN_PRIVATE_KEY ?? "mock_key";
+ const testServerPubKey = process.env.TEST_ARK_SERVER_PUBLIC_KEY ?? "mock_pubkey";📝 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.
| // Test private key (DO NOT USE IN PRODUCTION) | |
| const testPrivateKey = "ce66c68f8875c0c98a502c666303dc183a21600130013c06f9d1edf60207abf2"; | |
| const testServerPubKey = "02eec7245d6b7d2ccb30380bfbe2a3648cd7a942653f5aa340edcea1f283686619"; | |
| // Test private key (DO NOT USE IN PRODUCTION) | |
| const testPrivateKey = process.env.TEST_BITCOIN_PRIVATE_KEY ?? "mock_key"; | |
| const testServerPubKey = process.env.TEST_ARK_SERVER_PUBLIC_KEY ?? "mock_pubkey"; |
🧰 Tools
🪛 Gitleaks (8.21.2)
9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| @@ -356,3 +356,9 @@ CRONOSZKEVM_PRIVATE_KEY= | |||
|
|
|||
| # Fuel Ecosystem (FuelVM) | |||
| FUEL_WALLET_PRIVATE_KEY= | |||
|
|
|||
| # Bitcoin Configuration | |||
| BITCOIN_PRIVATE_KEY= # Your Bitcoin private key in WIF format | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add security warning for private key storage.
The private key should be stored securely and never committed to version control.
-BITCOIN_PRIVATE_KEY= # Your Bitcoin private key in WIF format
+BITCOIN_PRIVATE_KEY= # Your Bitcoin private key in WIF format (IMPORTANT: Store securely, never commit to version control)📝 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.
| BITCOIN_PRIVATE_KEY= # Your Bitcoin private key in WIF format | |
| BITCOIN_PRIVATE_KEY= # Your Bitcoin private key in WIF format (IMPORTANT: Store securely, never commit to version control) |
🤖 Bitcoin for AI Agents
This PR enables Eliza agents to handle Bitcoin transactions autonomously through @arklabs/wallet-sdk integration. Agents can now receive payments for services, pay other agents, and manage their Bitcoin wallets with both on-chain and Layer 2 (Ark protocol) support.
🚨 WARNING
Caution: Until an external audit is completed, it is strongly recommended not to use this integration on the main Bitcoin network with more than a negligible amount of BTC.
🔑 Configuration
The plugin requires the following environment variables:
🚀 Available Actions
🔍 Key Benefits for Agents
Autonomous Financial Operations
Flexible Payment Options
Enhanced Security
📋 Implementation Status
🧪 Testing
📚 Example Usage
Here's how to integrate the Bitcoin plugin into your agent:
Basic Transaction Flow