feat(coin-ops): coin-ops module that helps to select coins for payment#316
Merged
robert-zaremba merged 2 commits intomasterfrom Jan 22, 2026
Merged
feat(coin-ops): coin-ops module that helps to select coins for payment#316robert-zaremba merged 2 commits intomasterfrom
robert-zaremba merged 2 commits intomasterfrom
Conversation
…or payment Signed-off-by: Robert Zaremba <robert@zaremba.ch>
Contributor
Reviewer's GuideIntroduces a reusable coin-ops utility module for Sui coin selection/merging and refactors IKA coin preparation logic in the Sui indexer to use it, simplifying IkaClient and adjusting configuration naming while relaxing an ESLint rule. Sequence diagram for IKA presign request using coin-opssequenceDiagram
actor Caller
participant SuiClientImp
participant IkaClient
participant CoinOps
participant MystenSuiClient
Caller->>SuiClientImp: requestIkaPresign()
SuiClientImp->>IkaClient: fetchAllIkaCoins(signerAddress)
IkaClient-->>SuiClientImp: CoinStruct[] ikaCoins
SuiClientImp->>CoinOps: prepareCoin(ikaCoins, ikaSignCost, tx)
CoinOps-->>SuiClientImp: preparedCoin
alt encryptionKeyId is null
SuiClientImp->>IkaClient: getLatestNetworkEncryptionKeyId()
IkaClient-->>SuiClientImp: encryptionKeyId
end
SuiClientImp->>IkaClient: requestGlobalPresign(tx, preparedCoin, tx.gas, encryptionKeyId)
IkaClient-->>SuiClientImp: presignCap
SuiClientImp->>MystenSuiClient: signAndExecuteTransaction(tx)
MystenSuiClient-->>SuiClientImp: transactionDigest
SuiClientImp-->>Caller: presignId
Class diagram for coin-ops utilities and Sui IKA integrationclassDiagram
class CoinStruct {
string coinObjectId
string balance
string coinType
string digest
string previousTransaction
string version
}
class HasBalance {
string balance
}
class PrepareCoinResult {
TransactionObjectArgument preparedCoin
CoinStruct[] remaining
}
class CoinOps {
+sortCoinsByBalance(coins CoinStruct[]) CoinStruct[]
+selectBiggestCoins(coins CoinStruct[], target bigint) CoinStruct[]
+selectCoins(allCoins CoinStruct[], target bigint, firstLimit number) CoinStruct[][]
+prepareCoin(allCoins CoinStruct[], target bigint, tx Transaction) PrepareCoinResult
}
HasBalance <|-- CoinStruct
class IkaClient {
<<interface>>
+getLatestNetworkEncryptionKeyId() Promise~string~
+getCoordinatorId() string
+fetchAllIkaCoins(owner string) Promise~CoinStruct[]~
}
class IkaClientImp {
+getLatestNetworkEncryptionKeyId() Promise~string~
+getCoordinatorId() string
+fetchAllIkaCoins(owner string) Promise~CoinStruct[]~
}
IkaClient <|.. IkaClientImp
class SuiClientCfg {
SuiNet network
string signerMnemonic
IkaClient ikaClient
Client client
number ikaSignCost
}
class SuiClientImp {
-Client #sui
-Ed25519Keypair signer
-IkaClient #ika
-string|null encryptionKeyId
-number ikaSignCost
+ikaClient() IkaClient
+requestIkaPresign() Promise~string~
+solveRedeemRequest(...) Promise~string~
}
SuiClientCfg o--> IkaClient
SuiClientCfg o--> Client
SuiClientCfg o--> SuiClientImp
SuiClientImp --> IkaClient : uses
SuiClientImp --> CoinOps : uses
CoinOps --> CoinStruct
CoinOps --> PrepareCoinResult
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
selectCoins, theavailablevalue in the insufficient balance error only reflects the first phase (totalBalance) and ignores any balance that might have been accumulated in the second phase viaselectBiggestCoins, which can make the error message misleading; consider computingavailablefrom all scanned coins instead. selectCoinsandprepareCoinexpose aremainingcoins result but always return an empty array and never use it; either implement the remaining-coin selection logic or simplify the API to omitremainingto avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `selectCoins`, the `available` value in the insufficient balance error only reflects the first phase (`totalBalance`) and ignores any balance that might have been accumulated in the second phase via `selectBiggestCoins`, which can make the error message misleading; consider computing `available` from all scanned coins instead.
- `selectCoins` and `prepareCoin` expose a `remaining` coins result but always return an empty array and never use it; either implement the remaining-coin selection logic or simplify the API to omit `remaining` to avoid confusion.
## Individual Comments
### Comment 1
<location> `packages/lib/src/coin-ops.ts:63-64` </location>
<code_context>
+ if (totalBalance >= target) break;
+ }
+
+ if (totalBalance < target && allCoins.length > selected.length) {
+ const selected2 = selectBiggestCoins(
+ allCoins.slice(selected.length),
+ target - totalBalance,
</code_context>
<issue_to_address>
**suggestion:** Error message on insufficient balance can under-report the actual available balance.
When `selectBiggestCoins` returns `ok: false`, the error message still uses `totalBalance`, which only reflects the first-pass sum and ignores any value accumulated in the second pass. This can under-report the real total of `allCoins`.
Consider having `selectBiggestCoins` return the accumulated total from the second pass and adding it to `totalBalance` before constructing the error message, so the reported "available" amount matches the true balance.
Suggested implementation:
```typescript
if (totalBalance < target && allCoins.length > selected.length) {
const selected2 = selectBiggestCoins(
allCoins.slice(selected.length),
target - totalBalance,
);
if (!selected2.ok) {
const available =
totalBalance +
(selected2.accumulatedTotal !== undefined
? selected2.accumulatedTotal
: BigInt(0));
throw new Error(
`Insufficient coins balance. Required: ${target}, available: ${available}`,
);
}
if (selected2.accumulatedTotal !== undefined) {
totalBalance += selected2.accumulatedTotal;
}
selected = selected.concat(selected2.selected);
} else if (totalBalance < target) {
```
To fully implement this behavior, you also need to update the implementation and return type of `selectBiggestCoins`:
1. Ensure `selectBiggestCoins` returns an object that includes an `accumulatedTotal: bigint` field (or similarly named) representing the sum of balances it considered/selected in its second pass, even when `ok` is `false`.
- For example:
```ts
type SelectBiggestCoinsResult = {
ok: boolean;
selected: CoinStruct[];
accumulatedTotal: bigint;
};
```
2. In the body of `selectBiggestCoins`, maintain a running `accumulatedTotal` as you iterate through coins in the second pass, and always set it on the returned object.
3. If you already have a shared type/interface for this result, extend it with `accumulatedTotal` and update all call sites accordingly (this call site is already adjusted above).
</issue_to_address>
### Comment 2
<location> `packages/sui-indexer/src/ika_client.ts:88-89` </location>
<code_context>
}
- private async fetchAllIkaCoins(owner: string): Promise<CoinStruct[]> {
+ // TODO: we should have max length!
+ async fetchAllIkaCoins(owner: string): Promise<CoinStruct[]> {
const allCoins: CoinStruct[] = [];
let cursor: string | null | undefined = null;
</code_context>
<issue_to_address>
**suggestion (performance):** Public `fetchAllIkaCoins` with no limit can be expensive or problematic for accounts with many coins.
With `fetchAllIkaCoins` now public and used by `redeem-sui-client`, it will always load the entire coin set for a signer, which can be slow, memory-intensive, or hit RPC limits for accounts with many UTXO-like coins.
Since `prepareCoin`/`selectCoins` only need enough to cover `ikaSignCost`, consider adding a configurable max/page size and an early-exit path that stops pagination once the required amount is reached, with callers passing in the target amount as a parameter.
Suggested implementation:
```typescript
fetchAllIkaCoins(
owner: string,
opts?: {
/** Maximum number of coins to fetch before stopping (defensive ceiling). */
maxCoins?: number;
/** Optional target amount; pagination will stop once this balance is reached or exceeded. */
targetAmount?: bigint;
}
): Promise<CoinStruct[]>;
```
```typescript
// Fetch IKA coins for an owner, with defensive limits and optional early-exit
// once a target balance has been reached.
async fetchAllIkaCoins(
owner: string,
opts?: {
/** Maximum number of coins to fetch before stopping (defensive ceiling). */
maxCoins?: number;
/** Optional target amount; pagination will stop once this balance is reached or exceeded. */
targetAmount?: bigint;
}
): Promise<CoinStruct[]> {
const { maxCoins = 1_000, targetAmount } = opts ?? {};
const allCoins: CoinStruct[] = [];
let cursor: string | null | undefined = null;
// Use a conservative page size to avoid large responses and RPC timeouts.
const pageSize = 50;
let accumulated: bigint = 0n;
while (true) {
const page = await this.client.getCoins({
owner,
coinType: this.ikaConfig.objects.ikaCoinType,
cursor: cursor ?? undefined,
limit: pageSize,
});
if (!page.data.length) {
break;
}
for (const coin of page.data) {
allCoins.push(coin);
// Track accumulated balance for early exit when a targetAmount is provided.
if (targetAmount != null) {
accumulated += BigInt(coin.balance);
if (accumulated >= targetAmount) {
return allCoins;
}
}
// Enforce a hard cap on the number of coins fetched to protect against
// pathological accounts with an extremely large number of coins.
if (allCoins.length >= maxCoins) {
return allCoins.slice(0, maxCoins);
}
}
if (!page.hasNextPage) {
break;
}
cursor = page.nextCursor;
}
return allCoins;
}
```
1. Update all call sites of `fetchAllIkaCoins(owner)` (e.g. in `prepareCoin`/`selectCoins` and `redeem-sui-client`) to pass a `targetAmount` equal to the required `ikaSignCost` (or other needed amount), and optionally tighten `maxCoins` if you know a reasonable upper bound for those flows:
- `fetchAllIkaCoins(owner, { targetAmount: ikaSignCost })`
2. Ensure `this.client` is the same Sui client used elsewhere in this file and that `this.ikaConfig.objects.ikaCoinType` is the correct coin type string for IKA coins. If the file uses a different helper/wrapper for pagination (e.g. a shared `getAllCoins` utility), refactor the loop to use that helper instead of calling `this.client.getCoins` directly to stay consistent with the rest of the codebase.
3. If `CoinStruct.balance` is not a string but already a bigint/number, adjust the `BigInt(coin.balance)` conversion accordingly (e.g. `accumulated += coin.balance`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new reusable coin-ops module for selecting and preparing coins for payments on Sui, refactoring the existing coin selection logic out of the IkaClient implementation. The module provides functions to sort, select, and merge coins to meet target payment amounts.
Changes:
- Added new
coin-opsmodule with functions for coin selection, sorting, and transaction preparation - Refactored
IkaClientto exposefetchAllIkaCoinsas a public method instead of handling coin preparation internally - Updated
SuiClientto use the newprepareCoinfunction fromcoin-opsinstead of the oldprepareIkaCoinmethod
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/lib/src/coin-ops.ts | New module implementing coin selection algorithms (sortCoinsByBalance, selectBiggestCoins, selectCoins, prepareCoin) |
| packages/lib/src/coin-ops.test.ts | Unit tests for the selectCoins function covering various scenarios |
| packages/sui-indexer/src/ika_client.ts | Removed prepareIkaCoin method and made fetchAllIkaCoins public; deleted private coin selection helper methods |
| packages/sui-indexer/src/ika_client.test.ts | Removed tests for the deleted prepareIkaCoin method |
| packages/sui-indexer/src/redeem-sui-client.ts | Updated to use prepareCoin from coin-ops module; renamed ikaUpperLimit parameter to ikaSignCost for clarity |
| eslint.config.mjs | Disabled no-non-null-assertion rule globally to allow non-null assertions in the new code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+87
to
+108
| export function prepareCoin( | ||
| allCoins: CoinStruct[], | ||
| target: bigint, | ||
| tx: Transaction, | ||
| ): PrepareCoinResult { | ||
| const [selected, remaining] = selectCoins(allCoins, target); | ||
|
|
||
| if (selected.length === 1) { | ||
| const coin = selected[0]!; | ||
| return { preparedCoin: tx.object(coin.coinObjectId), remaining }; | ||
| } | ||
|
|
||
| const [firstCoin, ...coinsToMerge] = selected; | ||
| if (!firstCoin) { | ||
| throw new Error("No primary coin available"); | ||
| } | ||
| const preparedCoin = tx.object(firstCoin.coinObjectId); | ||
| const coinToMergeArgs = coinsToMerge.map((c) => tx.object(c.coinObjectId)); | ||
|
|
||
| tx.mergeCoins(preparedCoin, coinToMergeArgs); | ||
| return { preparedCoin, remaining }; | ||
| } |
There was a problem hiding this comment.
The prepareCoin function is not tested. While selectCoins is tested, prepareCoin adds additional logic for creating transaction arguments and merging coins that should be verified with tests. Consider adding tests for the prepareCoin function to ensure the transaction building logic works correctly.
packages/lib/src/coin-ops.test.ts
Outdated
| const createCoins = (amounts: number[]) => | ||
| amounts.map((amount, index) => createMockCoin(`coin${index}`, amount)); | ||
|
|
||
| describe("prepareCoin", () => { |
There was a problem hiding this comment.
The describe block is named "prepareCoin" but the tests are actually testing the "selectCoins" function. The describe block name should match the function being tested for clarity.
Suggested change
| describe("prepareCoin", () => { | |
| describe("selectCoins", () => { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce a reusable coin operations helper for Sui and migrate IKA coin selection to use it.
New Features:
Enhancements:
Tests:
Chores: