Skip to content

feat: complete typed Funding API - #101

Merged
nyg merged 3 commits into
masterfrom
codex/funding-coverage
Sep 11, 2026
Merged

feat: complete typed Funding API#101
nyg merged 3 commits into
masterfrom
codex/funding-coverage

Conversation

@nyg

@nyg nyg commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Completes the ten Funding operations from #82 with typed endpoints, parameter builders, response records and KrakenAPI methods: DepositMethods, DepositAddresses, DepositStatus, WithdrawMethods, WithdrawAddresses, WithdrawInfo, Withdraw, WithdrawStatus, WithdrawCancel and WalletTransfer.

Status responses accept both unpaginated transaction arrays and paginated objects with cursors. Deposit limits distinguish finite amounts, unlimited values and missing values. Monetary fields use BigDecimal; deposit and withdrawal timestamps use Instant. Requests support asset classes, rebase options, Lightning invoice amounts, address confirmation, maximum withdrawal fees and exact wallet names. Cancellation preserves a valid false result.

Requester tests exercise real endpoints through a package-private connection factory. Test dependency versions are managed in the parent with junit-bom. The shared private RebaseMultiplier type is identical to #100's addition.

Validation: Temurin 25 mvnd -B clean package passes for both modules with 81 tests. The library Javadoc jar passes with doclint=all,-missing. Updated #99, #101 and #100 snapshots merge cleanly in that order and pass all 257 tests together. Tests make no network calls and perform no withdrawals or transfers.

Targets master independently of #99 and #100. Covers the /0/private Funding group from #82, now called Funding (Legacy); Funding (Beta) is separate. Refs #82; the umbrella issue remains open.

Preserve pagination, decimal amounts, unlimited deposit limits, and unsuccessful cancellation results across the ten legacy Funding operations.

Refs #82

@nyg nyg left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against the existing library conventions. I did not build this branch locally, but #99 and #100 both build clean on Temurin 25 and this one shares their test setup.

DepositLimit with the package-private DepositLimitDeserializer is the best-fitting new code across the three coverage PRs — it matches the RecordMappingStrategy precedent in AGENTS.md for keeping implementation helpers package-private, and the compact-constructor invariant is a nice touch. Most of the notes below are shared with #100.

Formatting

Record component alignment. The nine multi-line records here (Deposit, DepositAddress, DepositMethod, DepositStatus, Withdrawal, WithdrawalAddress, WithdrawalInfo, WithdrawalMethod, WithdrawalStatus) use a flat 8-space continuation. Every multi-line record in the library aligns its components under the opening paren — Ticker, Report, PostTrade, PreTrade, SystemStatus, EarnStrategies, EarnAllocations, with indents from 21 to 34 spaces.

Wildcard imports. 20 in this branch (dev.andstuff.kraken.api.endpoint.funding.params.*, ...response.*, org.mockito.ArgumentMatchers.*, org.mockito.Mockito.*). The repository has zero, and #99 adds zero.

Import block order. The test classes put static imports last and org.* after dev.*. The main sources and #99's tests put static imports first, then java, com, then dev and lombok as one group.

Blank line between dev.andstuff and lombok imports. Added in 10 new files. The repository has 22 files where those imports are adjacent and none with a blank line between them.

Broken Javadoc indentation in all four new enums (AssetClass, DestinationWallet, RebaseMultiplier, SourceWallet), on the getValue() block:

    /**
     * Returns the value sent to Kraken.
 *
 * @return the API value
     */

Enums

Same points as #100. The new enums hand-roll a value field, constructor and getter; the library uses Lombok for that (KrakenAPI.Public and KrakenAPI.Private are @Getter @RequiredArgsConstructor), and the existing standalone enums (AssetClass, ReportFormat, RemovalType, LockType) are bare constant lists. @JsonProperty on each constant is redundant given ACCEPT_CASE_INSENSITIVE_ENUMS.

UNKNOWN("unknown") with @JsonValue on request-side enums means a caller can send aclass=unknown or rebase_multiplier=unknown to Kraken. The @JsonEnumDefaultValue UNKNOWN rule in AGENTS.md is about forward-compatible deserialization of responses; the existing request enums have no UNKNOWN.

funding/params/AssetClass is the fourth AssetClass enum in the library, after earn/response, subaccount/params and the one #100 adds in account/params, and funding/params/RebaseMultiplier duplicates #100's. Worth deciding whether these should be shared before all three land.

Javadoc

@param params the request parameters appears 10 times; the library describes what the parameters actually are.

Several params fields have Javadoc that restates the field name — Asset being withdrawn., Amount to be withdrawn., The rebase multiplier for {@code Withdraw}. Also, the new params classes put a block on every private field, which existing params classes do not do; they carry their semantics in the class-level Javadoc.

Timestamps

Deposit.time and Withdrawal.time are Long. The library maps Kraken unix timestamps to InstantReport, LedgerEntry, PostTrade, EarnAllocations. JavaTimeModule handles fractional epoch seconds with nanosecond precision, so precision is not a reason to keep raw longs.

Params

WithdrawParams.params() calls putIfNonNull for asset, key and amount, all of which are @NonNull and therefore cannot be null by the time params() runs. #99 uses params.put("pair", pair) directly for its required field. Same pattern in the other funding params classes.

Tests

FundingRestRequesterTest is a second test class for DefaultKrakenRestRequester (#100 adds AccountRestRequesterTest for the same production class). FundingKrakenAPITest is likewise named after the domain slice rather than the class under test.

Worth a sweep for unused imports — #100's test files carry a copy-pasted header block with around 50 unused imports and these share the same shape.

No // Given / // When / // Then blocks. Worth settling once, since these PRs establish the project's first test suite.

Build

Test dependencies are declared in library/pom.xml with explicit <version>. Every other version in this project lives in the parent <dependencyManagement>, and Jackson comes in via jackson-bom; junit-bom would match that precedent.

Merge order

#99, #100 and this PR make identical edits to pom.xml, library/pom.xml and AGENTS.md, so those conflicts are trivial; the README sections will need real merging. This one is in better shape than #100, so #99 then this then #100 seems like the smoothest order.

@nyg

nyg commented Sep 10, 2026

Copy link
Copy Markdown
Owner Author

Updated in 933808b.

Addressed the formatting and test findings: aligned record components, expanded wildcard imports, removed unused imports, normalized import grouping, used classpath fixtures, and centralized test versions in the parent with junit-bom. Wire-value enums now use Lombok; removing the handwritten getters also removes their malformed Javadoc. Removed redundant request-enum @JsonProperty annotations; @JsonValue retains exact wallet wire values such as Spot Wallet.

Deposit and withdrawal timestamps now use Instant; the tests check the exact instants, including dates beyond 2038. Required parameters use direct map writes, retaining plain-decimal encoding for amounts. Requester tests now use real endpoints through a package-private connection factory, eliminating partial endpoint mocks. Response records use real local values. Test names begin with the production class and retain a domain suffix so independently developed API groups can keep separate suites.

Shared RebaseMultiplier with #100 in endpoint.priv. Kept AssetClass scoped to the domain: Funding uses currency/tokenized_asset, while Account Data's request enum describes pair classes such as forex/equity_pair; a single enum would admit invalid combinations. Existing response and subaccount types remain separate to avoid broadening this PR into changes to already published APIs.

Retained UNKNOWN because the supplied instructions explicitly require it on every new enum. It remains a fallback constant, not a documented Kraken request option. The instructions also say “Write no comments anywhere” and to preserve existing comments unless changes make them wrong. That takes precedence over the editorial Javadoc and Given/When/Then suggestions; timestamp documentation made inaccurate by the new types was corrected.

Validation: Temurin 25 mvnd -B clean package passes, 81 tests; the library Javadoc jar passes with doclint=all,-missing.

Integration check: the updated snapshots merge cleanly in the order #99#101#100 and pass all 244 tests plus the library Javadoc build. The Account Data facade insertion was moved to avoid its previous conflict with Level3. These are temporary integration checks; none of the PRs was merged.

@nyg nyg left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 933808b. Built locally on Temurin 25: BUILD SUCCESS, 81 tests pass, CI green. Merged with #99 and #100 onto master in a scratch worktree — no conflicts, 244 tests pass together. RebaseMultiplier moved to endpoint/priv/ in both this PR and #100 with byte-identical content, so that merges cleanly rather than duplicating.

Everything from the last round is fixed: record alignment, wildcard imports, import grouping, the enum Javadoc indentation, enum Lombok conversion, Instant timestamps, the redundant putIfNonNull calls on @nonnull fields in WithdrawParams, unused test imports, and the junit-bom move. DepositLimit with the package-private DepositLimitDeserializer is still the nicest new code across the three PRs.

Two minor leftovers inline, neither blocking.

Comment thread AGENTS.md
```

There are no tests in this project. CI runs `mvn clean package` on PRs targeting `master`.
The library's JUnit 5 tests use JSON fixtures and Mockito mocks of `KrakenRestRequester`, without network access or API keys. Run them with `mvn -pl library test`. CI runs `mvn clean package` on PRs targeting `master`.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anchoring here since it is the line this PR touches in AGENTS.md.

This PR adds endpoint/funding/ but the layout paragraph just below (line 22) is not updated for it. It still enumerates only market/, account/, subaccount/, transparency/ and earn/.

#99 updated that same sentence to cover the authenticated Level3 endpoint living under market/, so the two PRs touch it and will need a small manual merge anyway. Worth adding funding/ for deposits and withdrawals while you are in there.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in acc0c06: the Layout section now documents funding/ as the private Funding (Legacy) domain for deposits, withdrawals and wallet transfers. It is a separate paragraph, so the Level3 wording change in #99 still merges cleanly. Verified all three updated snapshots together: 257 tests and the Javadoc build pass.

/**
* Creates the {@code Withdraw} endpoint.
*
* @param params the request parameters

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@param params the request parameters restates the parameter name. It appears 10 times in this PR.

The existing library describes what the parameters are — @param params the filtering and pagination parameters on ledgerInfo, @param params the asset, amount and accounts of the transfer on accountTransfer. Here it would be something like "the asset, key, amount and optional fee cap of the withdrawal".

doclint is all,-missing, so dropping the tag is valid too and reads better than restating the name.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retained under the direct user instruction to preserve existing comments unless a code change makes them wrong. The generic tag is accurate; replacing or deleting it is cosmetic. The requested Funding package documentation is handled separately in AGENTS.md.

@nyg
nyg merged commit b4672b7 into master Sep 11, 2026
1 check passed
@nyg
nyg deleted the codex/funding-coverage branch September 11, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant