diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..ebf194f --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,53 @@ +# Copilot Instructions + +## Build + +```sh +mvn clean install # build and install both modules +mvn clean package # build without install +mvn -pl library package # build only the library module +mvn -pl examples package # build only the examples module +``` + +There are no tests in this project. CI runs `mvn clean package` on PRs targeting `master`. + +Java 25 with Temurin is required (configured via `maven-compiler-plugin` with `25`). + +## Architecture + +This is a multi-module Maven project (`library` + `examples`) providing a Java client for the [Kraken REST API](https://docs.kraken.com/rest/). + +### Endpoint type hierarchy + +All Kraken endpoints extend `Endpoint` and split into two branches: + +- **`PublicEndpoint`** — GET requests to `/0/public/{path}`. Uses `QueryParams` to build URL query strings. +- **`PrivateEndpoint`** — POST requests to `/0/private/{path}`. Uses `PostParams` for form-encoded body with nonce-based HMAC signing. + +Concrete endpoints live in domain packages under `endpoint/`: +- `endpoint/market/` — public market data (assets, pairs, ticker, server time) +- `endpoint/account/` — private account data (ledgers, reports) + +Each endpoint package follows: `{Name}Endpoint`, `params/{Name}Params`, `response/{ResponseType}`. + +### KrakenAPI facade + +`KrakenAPI` is the main entry point. It provides: +1. **Typed methods** for implemented endpoints (e.g., `assetInfo()`, `ledgerInfo()`) +2. **Generic `query()` methods** returning `JsonNode` for unimplemented endpoints, using the `Public`/`Private` enums +3. **Raw `queryPublic()`/`queryPrivate()`** accepting path strings for endpoints not yet in the enums + +### REST requester + +`KrakenRestRequester` is the HTTP abstraction. `DefaultKrakenRestRequester` uses `HttpsURLConnection`. The interface is designed for alternative HTTP client implementations (Spring RestTemplate, OkHttp, etc.). + +JSON responses are deserialized through `KrakenResponse`, which unwraps the Kraken `{error, result}` envelope. ZIP responses (report exports) go through `Endpoint.processZipResponse()`. + +## Conventions + +- **Lombok** is used throughout: `@Getter`, `@RequiredArgsConstructor`, `@Builder`, `@Slf4j`, `@With`, `@Setter`. Annotation processing is configured in the parent POM. +- **Java records** for response types and `KrakenResponse`. Records use `@JsonProperty` for Kraken's naming conventions and `@JsonEnumDefaultValue` for forward-compatible enum deserialization. +- **OpenCSV** annotations (`@CsvBindByName`) on `LedgerEntry` enable both JSON API and CSV file parsing with the same record. +- Jackson is configured with `ACCEPT_CASE_INSENSITIVE_ENUMS`, `READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE`, and `FAIL_ON_UNKNOWN_PROPERTIES` disabled — always include `@JsonEnumDefaultValue UNKNOWN` on new enums. +- Commit messages follow [Conventional Commits](https://www.conventionalcommits.org/). Release commits use `chore(release):` prefix. +- The `examples` module is excluded from Maven Central publishing. API keys go in `examples/src/main/resources/api-keys.properties` (not committed). diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md new file mode 100644 index 0000000..86281c7 --- /dev/null +++ b/docs/ARCHITECTURE.md @@ -0,0 +1,180 @@ +# Library Architecture + +## Overview + +The library is a Java client for the [Kraken REST API](https://docs.kraken.com/rest/). It is organized around four core concepts: + +- **`KrakenAPI`** — The main entry point. A facade that exposes typed methods for implemented endpoints and generic methods for any endpoint. +- **`Endpoint`** — Represents a single API call. Knows its HTTP method, URL path, parameters, and response type. Splits into `PublicEndpoint` (GET) and `PrivateEndpoint` (POST with HMAC signing). +- **`KrakenRestRequester`** — Interface that performs the actual HTTP request and response parsing. `DefaultKrakenRestRequester` is the built-in implementation using `HttpsURLConnection`. +- **Params / Response types** — Each endpoint has dedicated parameter objects (`QueryParams` for public, `PostParams` for private) and response records deserialized via Jackson. + +## Request Flow + +### Public Endpoint + +```mermaid +sequenceDiagram + participant User + participant KrakenAPI + participant PublicEndpoint + participant QueryParams + participant RestRequester as DefaultKrakenRestRequester + participant Kraken as Kraken API + + User->>KrakenAPI: assetInfo(["BTC", "ETH"]) + KrakenAPI->>PublicEndpoint: new AssetInfoEndpoint(assets) + activate PublicEndpoint + PublicEndpoint->>QueryParams: new AssetInfoParams(assets, "currency") + deactivate PublicEndpoint + + KrakenAPI->>RestRequester: execute(endpoint) + RestRequester->>PublicEndpoint: buildURL() + PublicEndpoint->>QueryParams: toMap() + QueryParams-->>PublicEndpoint: {asset: "BTC,ETH", aclass: "currency"} + PublicEndpoint-->>RestRequester: https://api.kraken.com/0/public/Assets?asset=BTC,ETH&aclass=currency + + RestRequester->>Kraken: GET + Kraken-->>RestRequester: {"error": [], "result": {…}} + + RestRequester->>RestRequester: deserialize into KrakenResponse> + RestRequester->>RestRequester: unwrap result or throw KrakenException + RestRequester-->>KrakenAPI: Map + KrakenAPI-->>User: Map +``` + +### Private Endpoint + +```mermaid +sequenceDiagram + participant User + participant KrakenAPI + participant PrivateEndpoint + participant PostParams + participant NonceGen as KrakenNonceGenerator + participant Credentials as KrakenCredentials + participant RestRequester as DefaultKrakenRestRequester + participant Kraken as Kraken API + + User->>KrakenAPI: ledgerInfo(params) + KrakenAPI->>PrivateEndpoint: new LedgerInfoEndpoint(params) + KrakenAPI->>RestRequester: execute(endpoint, credentials, nonceGenerator) + + RestRequester->>NonceGen: generate() + NonceGen-->>RestRequester: "1712750400000" + + RestRequester->>PrivateEndpoint: encodedParamsWith(nonce) + PrivateEndpoint->>PostParams: setNonce(nonce) + PrivateEndpoint->>PostParams: encoded() + PostParams-->>RestRequester: "nonce=1712750400000&asset=BTC,ETH&…" + + RestRequester->>PrivateEndpoint: buildURL() + PrivateEndpoint-->>RestRequester: https://api.kraken.com/0/private/Ledgers + + RestRequester->>Credentials: sign(url, nonce, encodedParams) + Note over Credentials: SHA-256(nonce + params)
HMAC-SHA512(base64(secret), path + sha256) + Credentials-->>RestRequester: Base64 signature + + RestRequester->>Kraken: POST with API-Key + API-Sign headers + Kraken-->>RestRequester: {"error": [], "result": {…}} + + RestRequester->>RestRequester: deserialize into KrakenResponse + RestRequester->>RestRequester: unwrap result or throw KrakenException + RestRequester-->>KrakenAPI: LedgerInfo + KrakenAPI-->>User: LedgerInfo +``` + +## Component Diagram + +```mermaid +classDiagram + class KrakenAPI { + -KrakenCredentials credentials + -KrakenNonceGenerator nonceGenerator + -KrakenRestRequester restRequester + +serverTime() ServerTime + +assetInfo(assets) Map~String, AssetInfo~ + +ticker(pairs) Map~String, Ticker~ + +ledgerInfo(params) LedgerInfo + +query(endpoint) JsonNode + +queryPublic(path) JsonNode + +queryPrivate(path) JsonNode + } + + class Endpoint~T~ { + <> + #String path + -String httpMethod + -TypeReference~T~ responseType + +buildURL() URL + +wrappedResponseType() JavaType + } + + class PublicEndpoint~T~ { + -QueryParams queryParams + +buildURL() URL + } + + class PrivateEndpoint~T~ { + -PostParams postParams + +encodedParamsWith(nonce) String + +buildURL() URL + } + + class KrakenRestRequester { + <> + +execute(PublicEndpoint~T~) T + +execute(PrivateEndpoint~T~, credentials, nonceGenerator) T + } + + class DefaultKrakenRestRequester { + -ObjectMapper OBJECT_MAPPER + } + + class KrakenResponse~T~ { + <> + +List~String~ error + +Optional~T~ result + } + + class KrakenException { + +List~String~ errors + } + + class KrakenCredentials { + +sign(url, nonce, params) String + } + + class KrakenNonceGenerator { + <> + +generate() String + } + + Endpoint <|-- PublicEndpoint + Endpoint <|-- PrivateEndpoint + KrakenRestRequester <|.. DefaultKrakenRestRequester + KrakenAPI --> KrakenRestRequester + KrakenAPI --> KrakenCredentials + KrakenAPI --> KrakenNonceGenerator + DefaultKrakenRestRequester ..> KrakenResponse : deserializes + KrakenResponse ..> KrakenException : throws on error +``` + +## Three Access Tiers + +`KrakenAPI` provides three levels of access, from most to least typed: + +| Tier | Methods | Return type | When to use | +|------|---------|-------------|-------------| +| **Typed** | `assetInfo()`, `ledgerInfo()`, etc. | Domain records | Endpoint has a dedicated implementation | +| **Enum-based** | `query(Public.TICKER, params)` | `JsonNode` | Endpoint is in the `Public`/`Private` enum but not yet typed | +| **Raw path** | `queryPublic("Trades", params)` | `JsonNode` | Endpoint isn't in the enum yet (e.g., newly added by Kraken) | + +## Extending the Library + +To add a new typed endpoint: + +1. Create a response record in the appropriate `response/` package +2. Create a params class implementing `QueryParams` (public) or extending `PostParams` (private) +3. Create an endpoint class extending `PublicEndpoint` or `PrivateEndpoint` +4. Add a convenience method to `KrakenAPI` diff --git a/docs/CLAUDE_REVIEW.md b/docs/CLAUDE_REVIEW.md new file mode 100644 index 0000000..63ddff8 --- /dev/null +++ b/docs/CLAUDE_REVIEW.md @@ -0,0 +1,127 @@ +# Code Review — `library` Module + +**Date:** 2026-04-10 +**Reviewed by:** Claude Opus 4.6 + +--- + +## 1. Architecture + +The library follows a clean, layered design: + +- **`KrakenAPI`** — Facade class that exposes typed convenience methods for implemented endpoints, generic `query()` methods for unimplemented endpoints, and raw `queryPublic()`/`queryPrivate()` methods for fully custom paths. This three-tier access pattern provides good ergonomics while remaining extensible. +- **`Endpoint` hierarchy** — `PublicEndpoint` (GET) and `PrivateEndpoint` (POST) handle URL construction, parameter encoding, and response type resolution. Each concrete endpoint is a small, focused class. +- **`KrakenRestRequester`** — Interface that decouples HTTP transport from the API logic, allowing users to swap `HttpsURLConnection` for other HTTP clients. +- **Params / Response separation** — Each domain package (`market/`, `account/`) cleanly separates endpoint definitions, request parameters, and response types. + +**Strengths:** +- The endpoint-per-class pattern makes adding new Kraken endpoints straightforward and low-risk. +- The `KrakenResponse` envelope unwrapping centralizes error handling. +- The generic `query()` escape hatch ensures the library is immediately usable for any Kraken endpoint, even before a typed implementation exists. +- Use of Java records for response types is idiomatic and eliminates boilerplate. + +**Suggestions:** +- The `Public` and `Private` enums in `KrakenAPI` partially duplicate the typed endpoint classes. As more endpoints get typed implementations, consider whether these enums still serve a purpose or whether they add maintenance burden. +- `KrakenAPI` has many constructor overloads. The existing `@Builder` annotation could replace most of them, simplifying the API surface. + +--- + +## 2. Code Quality + +### Effective use of language features +- Java records, sealed-style enums with `@JsonEnumDefaultValue`, pattern matching in `LedgerEntry.underlyingAsset()`, and text blocks/formatted strings are used well throughout. +- Lombok is applied consistently (`@Getter`, `@RequiredArgsConstructor`, `@Builder`, `@Slf4j`, `@With`). + +### Areas for improvement + +**`PostParams.encoded()` readability** (`PostParams.java:17-31`) +The `reduce` with `StringBuilder` is harder to read than it needs to be. A simpler approach: +```java +return params.entrySet().stream() + .map(e -> e.getKey() + "=" + URLEncoder.encode(e.getValue(), StandardCharsets.UTF_8)) + .collect(Collectors.joining("&")); +``` +This also avoids the `replaceFirst("&$", "")` trailing-ampersand cleanup. + +**`KrakenCredentials` helper methods visibility** (`KrakenCredentials.java:38-43`) +`concat()` and `hmacSha512()` are `public static` but are implementation details of the signing process. They should be `private` (or package-private at most) to avoid polluting the public API. + +**`AssetInfoParams.toMap()` uses `Map.of()`** (`AssetInfoParams.java:15-19`) +This always includes both `asset` and `aclass` parameters, even when they could be null. If `assets` is null, `String.join(",", null)` will throw a `NullPointerException`. Contrast with `AssetPairParams` which correctly uses `putIfNonNull`. + +--- + +## 3. Potential Bugs + +### `LedgerInfo.hasNext()` hardcodes page size (`LedgerInfo.java:22`) +```java +public boolean hasNext() { + return entries.size() == 50; +} +``` +This assumes the Kraken API always returns exactly 50 entries per page. If Kraken changes the page size, or if the last page happens to contain exactly 50 entries, this heuristic will break. Consider using the `count` field (which the record already contains) to determine whether more entries exist: `return resultOffset + entries.size() < count`. + +### `RecordMappingStrategy` line length assertion (`RecordMappingStrategy.java:33-35`) +```java +if (recordComponents.length != line.length) { + throw new CsvRuntimeException("Mismatch between line values and record components"); +} +``` +This will fail if the CSV contains extra columns not mapped to record components. OpenCSV's standard behavior is to silently ignore extra columns, but this strict check breaks that contract. If Kraken adds a new column to their CSV export, this will throw an exception rather than gracefully ignoring the extra data. + +### `KrakenInstantConverter` fragile date parsing (`RecordMappingStrategy.java:73-75`) +```java +String[] dateTime = value.split(" "); +return Instant.parse("%sT%sZ".formatted(dateTime[0], dateTime[1])); +``` +No bounds checking on the `dateTime` array. If the input is empty, contains no space, or has an unexpected format, this will throw an `ArrayIndexOutOfBoundsException` with no helpful message. Consider adding validation or using a `DateTimeFormatter`. + +### Nonce collision risk with `EpochBasedNonceGenerator` (`EpochBasedNonceGenerator.java:6-8`) +Using `System.currentTimeMillis()` means two rapid sequential calls within the same millisecond produce the same nonce, which Kraken will reject. This is documented as customizable, but the default should be safer — for example, using `System.nanoTime()` or an `AtomicLong` counter seeded from the epoch. + +### `PostParams.encoded()` iteration order (`PostParams.java:21`) +`params.keySet().stream()` iterates over a `HashMap`, which has no guaranteed order. While this doesn't affect correctness for the Kraken API (which doesn't care about parameter order), the nonce signature is computed over the encoded params string, meaning the signing in `KrakenCredentials.sign()` receives `urlEncodedParams` — if the `params()` method returns a different iteration order than `encoded()` produces, signing would fail. Currently this isn't a problem because `encoded()` builds the string itself, but it's a fragile coupling. Using a `LinkedHashMap` or `TreeMap` would make this deterministic. + +--- + +## 4. Error Handling + +### `KrakenException` missing message (`KrakenException.java`) +`KrakenException` extends `RuntimeException` but never calls `super(message)`. When this exception is caught and logged, `getMessage()` returns `null`, which makes log output less useful. Consider: +```java +public KrakenException(List errors) { + super(String.join(", ", errors)); + this.errors = errors; +} +``` +The `@ToString` annotation helps in some contexts but won't be invoked by default logging frameworks. + +### `IllegalStateException` used for multiple failure modes +`DefaultKrakenRestRequester` throws `IllegalStateException` for HTTP errors, URL construction errors, and unsupported content types. These are all different failure modes that callers may want to handle differently. Consider a dedicated exception hierarchy or at minimum distinct messages that allow programmatic differentiation. + +### No HTTP status code checking (`DefaultKrakenRestRequester.java:83-98`) +`parseResponse()` reads from `connection.getInputStream()` without checking the HTTP status code. If the Kraken API returns a 4xx or 5xx status, `getInputStream()` will throw an `IOException`, and the actual error body (available via `getErrorStream()`) is lost. This makes debugging API errors harder than necessary. + +### Missing `null` check on `credentials` for private endpoints (`KrakenAPI.java:146-148`) +```java +private T executePrivate(PrivateEndpoint endpoint) { + return restRequester.execute(endpoint, credentials, nonceGenerator); +} +``` +If `KrakenAPI` was constructed without credentials (using the no-arg constructor), calling any private endpoint method will pass `null` credentials to the requester, eventually causing a `NullPointerException` deep in the signing code. A clear error like `"Credentials required for private endpoints"` thrown early would be more helpful. + +--- + +## 5. Suggestions for Improvement + +1. **Add tests.** The library has zero test coverage. The endpoint classes, parameter encoding, response deserialization, nonce generation, and HMAC signing are all highly testable in isolation. The `KrakenRestRequester` interface makes it straightforward to mock the HTTP layer. + +2. **Rate limiting.** The Kraken API has rate limits. Callers currently have no protection against hitting them. Consider adding optional rate-limiting support in `DefaultKrakenRestRequester` or documenting the limitation clearly. + +3. **Retry logic.** Network failures and transient Kraken errors (e.g., `EService:Unavailable`) are not retried. An optional retry policy would improve robustness for production use. + +4. **Resource cleanup in `DefaultKrakenRestRequester`.** `HttpsURLConnection` responses are not explicitly disconnected after reading. While not strictly required, calling `connection.disconnect()` in a `finally` block would release resources promptly. + +5. **Thread safety of `PostParams`.** The `setNonce()` method mutates state on the `PostParams` instance, which means the same endpoint object cannot safely be reused across threads. Since endpoints are created per-call in `KrakenAPI`, this isn't currently a problem, but it's a subtle trap if the usage pattern changes. Making `encodedParamsWith()` return the encoded string without mutating the object would be cleaner. + +6. **Consider `java.net.http.HttpClient`** (available since Java 11) as a replacement for `HttpsURLConnection`. It provides a more modern API with built-in async support and is the recommended approach for new Java HTTP code. diff --git a/RELEASE.md b/docs/RELEASE.md similarity index 100% rename from RELEASE.md rename to docs/RELEASE.md