added a new bridge so the cb can use the low lateny trading platform …#17
added a new bridge so the cb can use the low lateny trading platform …#17
Conversation
…(rust) as a tool, lets make some bread boiz
There was a problem hiding this comment.
Pull request overview
This PR adds a low-latency Rust trading bridge for OpenClaw that enables communication between TypeScript-based tools and a high-performance Rust trading daemon via Unix domain socket (UDS). The bridge establishes a foundation for integrating low-latency trading capabilities while maintaining the existing OpenClaw tool ecosystem.
Changes:
- Adds Rust workspace with three crates: trading_protocol (shared protocol), trading_daemon (UDS server), and tradingctl (CLI client)
- Creates OpenClaw extension (.openclaw/extensions/trading-bridge) exposing trading_status and trading_control tools
- Integrates trading-daemon service into docker-compose.yml with shared socket volume
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/Cargo.toml | Defines Rust workspace with trading_protocol, trading_daemon, and tradingctl members |
| crates/Cargo.lock | Auto-generated dependency lockfile for Rust workspace |
| crates/trading_protocol/Cargo.toml | Protocol library dependencies (serde, tokio-util, uuid, chrono) |
| crates/trading_protocol/src/lib.rs | Shared protocol definitions: 4-byte length-prefixed JSON codec, Envelope struct, ControlCommand enum |
| crates/trading_daemon/Cargo.toml | Daemon dependencies including tokio, tracing, fs2 for locking |
| crates/trading_daemon/src/main.rs | UDS server implementation with single-instance locking, connection handling, and basic control commands |
| crates/trading_daemon/Dockerfile | Multi-stage Docker build from rust:1-slim-bookworm to debian:bookworm-slim, runs as non-root user 1000:1000 |
| crates/tradingctl/Cargo.toml | CLI tool dependencies (clap, futures, tokio) |
| crates/tradingctl/src/main.rs | Command-line client for sending control commands to daemon |
| crates/README.md | Comprehensive documentation covering architecture, Docker deployment, local development, and troubleshooting |
| .openclaw/extensions/trading-bridge/package.json | Extension package metadata with TypeScript build scripts |
| .openclaw/extensions/trading-bridge/tsconfig.json | TypeScript compiler configuration for ES2022/NodeNext |
| .openclaw/extensions/trading-bridge/openclaw.plugin.json | Plugin metadata defining socketPath configuration schema |
| .openclaw/extensions/trading-bridge/src/index.ts | TradingClient implementation with reconnection logic, frame parsing, and tool registration |
| clawdbot-workflows/docker-compose.yml | Adds trading-daemon service with shared openclaw-socket-vol volume and openclaw-gateway build context |
| pnpm-workspace.yaml | Adds '.openclaw/extensions/trading-bridge' to workspace packages |
| README.md | Documents new Rust Trading Bridge section with quick start and architecture reference |
| .dockerignore | Configures root-context Docker builds to include only crates/ and required non-agent-workflows/libs/ |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn create_codec() -> JsonCodec { | ||
| LengthDelimitedCodec::builder() | ||
| .length_field_length(4) | ||
| .new_codec() | ||
| } |
There was a problem hiding this comment.
The LengthDelimitedCodec is created without setting a maximum frame length. This could allow denial-of-service attacks where a malicious client sends extremely large frame length values, causing memory exhaustion. Consider adding .max_frame_length() to limit the maximum message size.
crates/README.md
Outdated
| docker exec trading-trading-daemon-1 tradingctl ping | ||
| docker exec trading-trading-daemon-1 tradingctl status | ||
| docker exec trading-trading-daemon-1 tradingctl start | ||
| docker exec trading-trading-daemon-1 tradingctl stop |
There was a problem hiding this comment.
The docker exec examples use 'trading-trading-daemon-1' as the container name, but the service is named 'trading-daemon' in docker-compose.yml. Docker Compose naming convention would typically produce 'clawdbot-workflows-trading-daemon-1' or a similar format based on the project directory name. Consider verifying and correcting the container name in the examples.
| docker exec trading-trading-daemon-1 tradingctl ping | |
| docker exec trading-trading-daemon-1 tradingctl status | |
| docker exec trading-trading-daemon-1 tradingctl start | |
| docker exec trading-trading-daemon-1 tradingctl stop | |
| docker compose -f clawdbot-workflows/docker-compose.yml --project-directory . exec trading-daemon tradingctl ping | |
| docker compose -f clawdbot-workflows/docker-compose.yml --project-directory . exec trading-daemon tradingctl status | |
| docker compose -f clawdbot-workflows/docker-compose.yml --project-directory . exec trading-daemon tradingctl start | |
| docker compose -f clawdbot-workflows/docker-compose.yml --project-directory . exec trading-daemon tradingctl stop |
| build: | ||
| context: ./clawdbot-workflows | ||
| dockerfile: Dockerfile |
There was a problem hiding this comment.
The openclaw-gateway service now specifies a build context pointing to ./clawdbot-workflows with a Dockerfile, but this appears to contradict the existing pattern where the image is specified by the OPENCLAW_IMAGE environment variable. If the intention is to use a pre-built image specified by the environment variable, the build section should be removed. If the intention is to always build locally, this is fine but represents a change in deployment strategy.
| build: | |
| context: ./clawdbot-workflows | |
| dockerfile: Dockerfile |
| serde_json = "1.0" | ||
| anyhow = "1.0" | ||
| tracing = "0.1" | ||
| tracing-subscriber = "0.3" |
There was a problem hiding this comment.
The trading_daemon declares kalshi_client as a dependency but doesn't appear to use it in the main.rs file. The daemon currently only implements a basic control protocol stub without actual trading functionality. Consider removing this dependency if it's not needed yet, or document why it's included for future implementation.
| tracing-subscriber = "0.3" | |
| tracing-subscriber = "0.3" | |
| # kalshi_client is pre-declared for upcoming trading functionality against Kalshi markets, | |
| # even though the current daemon only implements a basic control protocol stub. |
| - 'non-agent-workflows' | ||
| - 'clawdbot-workflows' | ||
| - 'llm-workflows' | ||
| - '.openclaw/extensions/trading-bridge' |
There was a problem hiding this comment.
The trading-bridge extension is added to the root pnpm-workspace.yaml at '.openclaw/extensions/trading-bridge', but the clawdbot-workflows workspace already includes 'extensions/*' which should match extensions in its own directory. This appears to be adding an extension from outside the clawdbot-workflows directory. Consider whether this extension should be placed in clawdbot-workflows/extensions/ instead to follow the existing pattern, or if this unusual location is intentional.
| - '.openclaw/extensions/trading-bridge' |
| let listener = UnixListener::bind(socket_path) | ||
| .with_context(|| format!("Failed to bind UDS socket {}", socket_path))?; | ||
| #[cfg(unix)] | ||
| std::fs::set_permissions(socket_path, std::fs::Permissions::from_mode(0o660)) |
There was a problem hiding this comment.
The socket permissions are set to 0o660 (read-write for user and group only), but the daemon runs as user 1000:1000, and the openclaw-gateway likely runs as a different user (node). Unless both containers share the same group ownership or use the shared volume with appropriate permissions, the gateway may not be able to connect to the socket. Consider setting permissions to 0o666 for broader access, or ensure proper group ownership is configured via the Docker volume mount.
| std::fs::set_permissions(socket_path, std::fs::Permissions::from_mode(0o660)) | |
| std::fs::set_permissions(socket_path, std::fs::Permissions::from_mode(0o666)) |
| - openclaw-socket-vol:/var/run/openclaw | ||
| env_file: | ||
| - .env | ||
| user: "1000:1000" |
There was a problem hiding this comment.
The trading-daemon service is missing the 'init: true' setting which is present in other services. Without this, zombie processes may not be properly reaped in the container. Consider adding 'init: true' for consistency and proper process management.
| user: "1000:1000" | |
| user: "1000:1000" | |
| init: true |
| private handleData(data: Buffer): void { | ||
| this.buffer = Buffer.concat([this.buffer, data]); | ||
|
|
||
| while (this.buffer.length >= 4) { | ||
| const length = this.buffer.readUInt32BE(0); | ||
| if (this.buffer.length < 4 + length) { | ||
| break; | ||
| } | ||
|
|
||
| const frame = this.buffer.subarray(4, 4 + length); | ||
| this.buffer = this.buffer.subarray(4 + length); | ||
|
|
||
| let envelope: Envelope; | ||
| try { | ||
| envelope = JSON.parse(frame.toString("utf8")) as Envelope; | ||
| } catch (error) { | ||
| console.error("Failed to parse trading frame:", toErrorMessage(error)); | ||
| continue; | ||
| } | ||
|
|
||
| this.state.lastEnvelope = envelope; | ||
| const pending = this.pending.get(envelope.id); | ||
| if (pending) { | ||
| clearTimeout(pending.timer); | ||
| this.pending.delete(envelope.id); | ||
| pending.resolve(envelope); | ||
| continue; | ||
| } | ||
|
|
||
| this.emit("envelope", envelope); | ||
| } |
There was a problem hiding this comment.
The handleData method doesn't validate the frame length before reading. A malicious or corrupted message could specify an extremely large length value, potentially causing memory exhaustion. Consider adding a maximum frame size check before allocating or processing the frame.
| execute: async ({ command }: { command: TradingCommand }) => { | ||
| const response = await client.request(toControlKind(command), {}); | ||
| return { | ||
| sent: true, | ||
| command, | ||
| response: response.payload, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
The trading_control tool's execute function doesn't check if the client is connected before sending requests. If the daemon is disconnected, the request will throw an error that's not handled. Consider checking client.state.connected and returning a helpful error response before attempting to send the request.
…(rust) as a tool, lets make some bread boiz