Refactor spot-exchange Commands for improved Marshallable code readability and consistency#25
Conversation
…loc wire reads/writes - Implemented explicit `readMarshallable` and `writeMarshallable` methods on `AuthCommand`, `DepositCommand`, `OrderCancelCommand`, `OrderCreateCommand`, and `SnapshotCommand`. - Refactored `OrderCreateCommand` to carry the SBE payload as a `PointerBytesStore`. - Updated `WsCommandInboundHandler` to use `.marshallable(cmd)` with `ChronicleWireKey.payload` to write domain commands directly to the WAL. - Refactored `AeronSender` (gateway) and `AeronReceiver` (core) to cleanly serialize/deserialize the commands directly from the payload. - Refactored `Engine` core to retrieve properties cleanly without duplicated raw wire manipulations. - Fixed direct buffer `byte[]` allocation issues on hot-paths inside `AeronSender` by properly reading into a thread-local `scratchBuffer` directly. Co-authored-by: vincentf13 <13410214+vincentf13@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25a19eb474
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| wire.read(ChronicleWireKey.payload).marshallable(cmd); | ||
|
|
||
| final int payloadLength = (int) cmd.getSbePayload().capacity(); | ||
| cmd.getSbePayload().bytesForRead().read(scratchBuffer.buffer().byteArray(), 0, payloadLength); |
There was a problem hiding this comment.
After scratchBuffer.clear(), this code copies into scratchBuffer.buffer().byteArray() without first wrapping the buffer to valid writable storage, and it never advances scratchBuffer.bytes() positions. In the ORDER_CREATE path this can produce a null/invalid destination or leave wrapForRead() with zero readable bytes, so the gateway can send malformed or empty create-order payloads to Aeron.
Useful? React with 👍 / 👎.
| wire.read(ChronicleWireKey.data).bytes(scratchBuffer.bytes()); | ||
| // Copy SBE payload to scratchBuffer for decoding | ||
| long bytesLen = cmd.getSbePayload().capacity(); | ||
| cmd.getSbePayload().bytesForRead().read(scratchBuffer.buffer().byteArray(), 0, (int) bytesLen); |
There was a problem hiding this comment.
The engine repeats the same copy pattern by reading into scratchBuffer.buffer().byteArray() right after scratchBuffer.clear() and before preparing the scratch buffer state. Since wrapForRead() uses scratchBuffer.bytes().readRemaining(), ORDER_CREATE can be decoded from an empty/invalid buffer, leading to failed or corrupted create-order processing during WAL replay/live consumption.
Useful? React with 👍 / 👎.
1 participant
This PR refactors how
spot-exchangeusesnet.openhft.chronicle.wireinside Aeron integration logic.Instead of parsing objects manually inside network handlers and routing orchestrators,
spot-infracommands (AuthCommand,DepositCommand,OrderCreateCommand, etc.) now strictly implement theMarshallableinterface. Senders and receivers simply deserialize/serialize entire objects using.marshallable()viaChronicleWireKey.payload.Additionally, we use direct copying onto thread-local off-heap structures inside Aeron loops to prevent hot-path allocations and Garbage Collection pressure.
PR created automatically by Jules for task 5331915758396860001 started by @vincentf13