Skip to content

skip balance tracking for vaultless vaults orderbook v6 #432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rouzwelt wants to merge 3 commits into
base: master
Choose a base branch
from
+155 −33
Open

skip balance tracking for vaultless vaults orderbook v6 #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9eac8e
init
rouzwelt Feb 4, 2026
c922e21
Update round.ts
rouzwelt Feb 4, 2026
f4ada03
Merge branch 'master' into 2026-02-04-skip-vaultless-balance-tracker
rouzwelt Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/core/process/round.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { RainSolver } from "..";
import { Pair } from "../../order";
import { Token } from "sushi/currency";
import { iterRandom, Result } from "../../common";
import { SpanStatusCode } from "@opentelemetry/api";
import { PreAssembledSpan, SpanWithContext } from "../../logger";
import { OrderbookVersions, Pair, ZERO_BYTES_32 } from "../../order";
import { ErrorSeverity, errorSnapshot, isTimeout, KnownErrors } from "../../error";
import {
ProcessOrderStatus,
Expand Down Expand Up @@ -181,8 +181,17 @@ export async function processOrderInit(
),
)?.balance ?? orderDetails.buyTokenVaultBalance;

// skip if the output vault is empty
if (orderDetails.sellTokenVaultBalance <= 0n) {
// skip if the output vault is empty for
// non v6 orderbook or non vaultless v6 orderbook
if (
(orderDetails.orderbookVersion !== OrderbookVersions.V6 &&
orderDetails.sellTokenVaultBalance <= 0n) ||
(orderDetails.orderbookVersion === OrderbookVersions.V6 &&
orderDetails.takeOrder.struct.order.validOutputs[
orderDetails.takeOrder.struct.outputIOIndex
].vaultId !== ZERO_BYTES_32 &&
orderDetails.sellTokenVaultBalance <= 0n)
) {
Comment on lines +186 to +194
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Minor: the condition can be simplified by factoring out the common <= 0n check.

Both branches share the sellTokenVaultBalance <= 0n guard.

Simplified condition 
-    if (
-        (orderDetails.orderbookVersion !== OrderbookVersions.V6 &&
-            orderDetails.sellTokenVaultBalance <= 0n) ||
-        (orderDetails.orderbookVersion === OrderbookVersions.V6 &&
-            orderDetails.takeOrder.struct.order.validOutputs[
-                orderDetails.takeOrder.struct.outputIOIndex
-            ].vaultId !== ZERO_BYTES_32 &&
-            orderDetails.sellTokenVaultBalance <= 0n)
-    ) {
+    const isVaultless =
+        orderDetails.orderbookVersion === OrderbookVersions.V6 &&
+        orderDetails.takeOrder.struct.order.validOutputs[
+            orderDetails.takeOrder.struct.outputIOIndex
+        ].vaultId === ZERO_BYTES_32;
+    if (!isVaultless && orderDetails.sellTokenVaultBalance <= 0n) {
🤖 Prompt for AI Agents 
In `@src/core/process/round.ts` around lines 186 - 194, The condition redundantly
checks orderDetails.sellTokenVaultBalance <= 0n in both branches; refactor the
if in round.ts to first check sellTokenVaultBalance <= 0n and then, if needed,
evaluate the V6-specific vaultId condition
(orderDetails.takeOrder.struct.outputIOIndex and its validOutputs[].vaultId
compared to ZERO_BYTES_32) only when orderDetails.orderbookVersion ===
OrderbookVersions.V6, so the common guard is factored out and the logic remains
equivalent.

const endTime = performance.now();
const settlement: Settlement = {
pair,
Expand Down
91 changes: 81 additions & 10 deletions src/order/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1188,7 +1188,14 @@ describe("Test OrderManager", () => {
const balance = "0x1234";
const spy = vi.spyOn(common, "normalizeFloat");

const result = orderManager.updateVault(orderbook, owner, token, vaultId, balance);
const result = orderManager.updateVault(
orderbook,
owner,
token,
vaultId,
balance,
OrderbookVersions.V5,
);
expect(result).toBeUndefined();
expect(spy).toHaveBeenCalledWith(balance, token.decimals);

Expand All @@ -1206,7 +1213,7 @@ describe("Test OrderManager", () => {
const vaultId = 123n;
const balance = 1000000000000000000n;

orderManager.updateVault(orderbook, owner, token, vaultId, balance);
orderManager.updateVault(orderbook, owner, token, vaultId, balance, OrderbookVersions.V4);

const orderbookMap = orderManager.ownerTokenVaultMap.get(orderbook);
expect(orderbookMap).toBeDefined();
Expand Down Expand Up @@ -1236,7 +1243,7 @@ describe("Test OrderManager", () => {
const balance = "0xffffffee00000000000000000000000000000000000000000000000000000001";
const spy = vi.spyOn(common, "normalizeFloat");

orderManager.updateVault(orderbook, owner, token, vaultId, balance);
orderManager.updateVault(orderbook, owner, token, vaultId, balance, OrderbookVersions.V5);

expect(spy).toHaveBeenCalledWith(balance, token.decimals);

Expand All @@ -1258,6 +1265,40 @@ describe("Test OrderManager", () => {
spy.mockRestore();
});

it("should set 0 vault balance for vaultless orderbook v6", () => {
const orderbook = "0xorderbook1";
const owner = "0xowner1";
const token = {
address: "0xtoken1",
symbol: "TOKEN1",
decimals: 18,
};
const vaultId = 0n;
const balance = "0xffffffee00000000000000000000000000000000000000000000000000000001"; // ignored
const spy = vi.spyOn(common, "normalizeFloat");

orderManager.updateVault(orderbook, owner, token, vaultId, balance, OrderbookVersions.V6);

expect(spy).not.toHaveBeenCalledWith();

const orderbookMap = orderManager.ownerTokenVaultMap.get(orderbook);
expect(orderbookMap).toBeDefined();

const ownerMap = orderbookMap?.get(owner);
expect(ownerMap).toBeDefined();

const tokenMap = ownerMap?.get(token.address);
expect(tokenMap).toBeDefined();

const vault = tokenMap?.get(vaultId);
expect(vault).toBeDefined();
expect(vault?.id).toBe(vaultId);
expect(vault?.balance).toBe(0n); // skip tracking, always 0
expect(vault?.token).toEqual(token);

spy.mockRestore();
});
Comment on lines +1268 to +1300
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Bug: expect(spy).not.toHaveBeenCalledWith() does not assert the spy was never called.

On line 1282, .not.toHaveBeenCalledWith() with no arguments only asserts the spy was not called with zero arguments — it does not assert that normalizeFloat was never invoked. The intent here is to verify that balance normalization is skipped entirely for vaultless V6 vaults.

Fix the assertion 
-        expect(spy).not.toHaveBeenCalledWith();
+        expect(spy).not.toHaveBeenCalled();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("should set 0 vault balance for vaultless orderbook v6", () => {
const orderbook = "0xorderbook1";
const owner = "0xowner1";
const token = {
address: "0xtoken1",
symbol: "TOKEN1",
decimals: 18,
};
const vaultId = 0n;
const balance = "0xffffffee00000000000000000000000000000000000000000000000000000001"; // ignored
const spy = vi.spyOn(common, "normalizeFloat");
orderManager.updateVault(orderbook, owner, token, vaultId, balance, OrderbookVersions.V6);
expect(spy).not.toHaveBeenCalledWith();
const orderbookMap = orderManager.ownerTokenVaultMap.get(orderbook);
expect(orderbookMap).toBeDefined();
const ownerMap = orderbookMap?.get(owner);
expect(ownerMap).toBeDefined();
const tokenMap = ownerMap?.get(token.address);
expect(tokenMap).toBeDefined();
const vault = tokenMap?.get(vaultId);
expect(vault).toBeDefined();
expect(vault?.id).toBe(vaultId);
expect(vault?.balance).toBe(0n); // skip tracking, always 0
expect(vault?.token).toEqual(token);
spy.mockRestore();
});
it("should set 0 vault balance for vaultless orderbook v6", () => {
const orderbook = "0xorderbook1";
const owner = "0xowner1";
const token = {
address: "0xtoken1",
symbol: "TOKEN1",
decimals: 18,
};
const vaultId = 0n;
const balance = "0xffffffee00000000000000000000000000000000000000000000000000000001"; // ignored
const spy = vi.spyOn(common, "normalizeFloat");
orderManager.updateVault(orderbook, owner, token, vaultId, balance, OrderbookVersions.V6);
expect(spy).not.toHaveBeenCalled();
const orderbookMap = orderManager.ownerTokenVaultMap.get(orderbook);
expect(orderbookMap).toBeDefined();
const ownerMap = orderbookMap?.get(owner);
expect(ownerMap).toBeDefined();
const tokenMap = ownerMap?.get(token.address);
expect(tokenMap).toBeDefined();
const vault = tokenMap?.get(vaultId);
expect(vault).toBeDefined();
expect(vault?.id).toBe(vaultId);
expect(vault?.balance).toBe(0n); // skip tracking, always 0
expect(vault?.token).toEqual(token);
spy.mockRestore();
});
🤖 Prompt for AI Agents 
In `@src/order/index.test.ts` around lines 1268 - 1300, The test currently checks
the spy with expect(spy).not.toHaveBeenCalledWith(), which only asserts no call
with zero args; change this to assert the spy was never called by replacing that
line with expect(spy).not.toHaveBeenCalled() (or
expect(spy).toHaveBeenCalledTimes(0)) so the test truly verifies
common.normalizeFloat was not invoked when calling orderManager.updateVault(...,
OrderbookVersions.V6); keep the spy setup and spy.mockRestore() as-is.


it("should update vault correctly when balance is decimal string", () => {
const orderbook = "0xorderbook1";
const owner = "0xowner1";
Expand All @@ -1270,7 +1311,7 @@ describe("Test OrderManager", () => {
const balance = "1000000000000000000";
const spy = vi.spyOn(common, "normalizeFloat");

orderManager.updateVault(orderbook, owner, token, vaultId, balance);
orderManager.updateVault(orderbook, owner, token, vaultId, balance, OrderbookVersions.V4);

expect(spy).not.toHaveBeenCalled();

Expand Down Expand Up @@ -1305,7 +1346,14 @@ describe("Test OrderManager", () => {
const newBalance = 2000000000000000000n;

// First update - create vault
orderManager.updateVault(orderbook, owner, token, vaultId, initialBalance);
orderManager.updateVault(
orderbook,
owner,
token,
vaultId,
initialBalance,
OrderbookVersions.V4,
);

// verify initial state
const vault = orderManager.ownerTokenVaultMap
Expand All @@ -1316,7 +1364,14 @@ describe("Test OrderManager", () => {
expect(vault?.balance).toBe(initialBalance);

// second update - update balance
orderManager.updateVault(orderbook, owner, token, vaultId, newBalance);
orderManager.updateVault(
orderbook,
owner,
token,
vaultId,
newBalance,
OrderbookVersions.V4,
);

// verify updated balance
const updatedVault = orderManager.ownerTokenVaultMap
Expand All @@ -1342,8 +1397,8 @@ describe("Test OrderManager", () => {
const balance1 = 1000000000000000000n;
const balance2 = 2000000000000000000n;

orderManager.updateVault(orderbook, owner, token, vaultId1, balance1);
orderManager.updateVault(orderbook, owner, token, vaultId2, balance2);
orderManager.updateVault(orderbook, owner, token, vaultId1, balance1, OrderbookVersions.V4);
orderManager.updateVault(orderbook, owner, token, vaultId2, balance2, OrderbookVersions.V4);

const tokenMap = orderManager.ownerTokenVaultMap
.get(orderbook)
Expand Down Expand Up @@ -1374,10 +1429,24 @@ describe("Test OrderManager", () => {
const balance2 = 500000000n;

// Add first vault
orderManager.updateVault(orderbook, owner, token1, vaultId1, balance1);
orderManager.updateVault(
orderbook,
owner,
token1,
vaultId1,
balance1,
OrderbookVersions.V4,
);

// Add second vault with different token
orderManager.updateVault(orderbook, owner, token2, vaultId2, balance2);
orderManager.updateVault(
orderbook,
owner,
token2,
vaultId2,
balance2,
OrderbookVersions.V4,
);

// verify both vaults exist
const ownerMap = orderManager.ownerTokenVaultMap.get(orderbook)?.get(owner);
Expand Down Expand Up @@ -1442,6 +1511,7 @@ describe("Test OrderManager", () => {
},
20n, // From validOutputs[1].vaultId
2000n, // sellTokenVaultBalance
OrderbookVersions.V5,
);

// should use inputIOIndex: 0 (first input)
Expand All @@ -1456,6 +1526,7 @@ describe("Test OrderManager", () => {
},
30n, // From validInputs[0].vaultId
1000n, // buyTokenVaultBalance
OrderbookVersions.V5,
);

updateVaultSpy.mockRestore();
Expand Down
17 changes: 13 additions & 4 deletions src/order/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
CounterpartySource,
OrderbooksOwnersProfileMap,
OrderbookOwnerTokenVaultsMap,
OrderbookVersions,
} from "./types";

export * from "./types";
Expand Down Expand Up @@ -229,6 +230,7 @@ export class OrderManager {
},
BigInt(outputVault.vaultId),
pair.sellTokenVaultBalance,
pair.orderbookVersion,
);
this.updateVault(
orderbook,
Expand All @@ -240,6 +242,7 @@ export class OrderManager {
},
BigInt(inputVault.vaultId),
pair.buyTokenVaultBalance,
pair.orderbookVersion,
);
}

Expand All @@ -250,21 +253,27 @@ export class OrderManager {
* @param token - The token details
* @param vaultId - The vault id
* @param balance - The new vault balance
* @param orderbookVersion - Specifies the orderbook version
*/
updateVault(
orderbook: string,
owner: string,
token: TokenDetails,
vaultId: bigint,
balance: string | bigint,
orderbookVersion: OrderbookVersions,
) {
// normalize balance based on vault type
let normalizedBalance: bigint;
if (typeof balance === "string") {
if (balance.startsWith("0x")) {
const normalized = normalizeFloat(balance, token.decimals);
if (normalized.isErr()) return;
normalizedBalance = normalized.value;
if (orderbookVersion === OrderbookVersions.V6 && vaultId === 0n) {
normalizedBalance = 0n;
} else {
const normalized = normalizeFloat(balance, token.decimals);
if (normalized.isErr()) return;
normalizedBalance = normalized.value;
}
} else {
normalizedBalance = BigInt(balance);
}
Expand Down Expand Up @@ -413,7 +422,7 @@ export class OrderManager {
new OrderManagerError(
"Failed to create order pair from args",
OrderManagerErrorType.WasmEncodedError,
pairResult.error,
pairResult.error.readableMsg,
),
);
}
Expand Down
6 changes: 6 additions & 0 deletions src/order/sync.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ describe("Test syncOrders", () => {
},
123n,
"1000000000000000000",
"",
);
expect(mockUpdateVault).toHaveBeenCalledTimes(1);
});
Comment on lines 86 to 91
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Missing test coverage for the V6 subgraph version path in vault-related events.

All Deposit/Withdrawal/Clear/TakeOrder tests pass "" as the expected orderbookVersion since none of the mock transactions set __version to SubgraphVersions.V6. Consider adding at least one test that sets res.__version = SubgraphVersions.V6 and asserts that mockUpdateVault is called with OrderbookVersions.V6 as the last argument.

🤖 Prompt for AI Agents 
In `@src/order/sync.test.ts` around lines 86 - 91, Add a test that covers the V6
subgraph branch by creating a mock transaction result where you set
res.__version = SubgraphVersions.V6 (reuse the existing
Deposit/Withdrawal/Clear/TakeOrder test pattern), run the same sync logic that
triggers mockUpdateVault, and assert mockUpdateVault was called with
OrderbookVersions.V6 as the final argument; reference the existing
mockUpdateVault, SubgraphVersions, and OrderbookVersions symbols to
modify/extend the test so the last parameter expectation checks for
OrderbookVersions.V6.

Expand Down Expand Up @@ -129,6 +130,7 @@ describe("Test syncOrders", () => {
},
456n,
"500000000",
"",
);
expect(mockUpdateVault).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -194,6 +196,7 @@ describe("Test syncOrders", () => {
},
100n,
"2000000000000000000",
"",
);
expect(mockUpdateVault).toHaveBeenNthCalledWith(
2,
Expand All @@ -206,6 +209,7 @@ describe("Test syncOrders", () => {
},
200n,
"1000000000",
"",
);
});

Expand Down Expand Up @@ -270,6 +274,7 @@ describe("Test syncOrders", () => {
},
789n,
"50000000",
"",
);
expect(mockUpdateVault).toHaveBeenNthCalledWith(
2,
Expand All @@ -282,6 +287,7 @@ describe("Test syncOrders", () => {
},
101112n,
"3000000000000000000",
"",
);
});

Expand Down
7 changes: 5 additions & 2 deletions src/order/sync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { OrderManager } from ".";
import { OrderbookVersions, OrderManager } from ".";
import { errorSnapshot } from "../error";
import { PreAssembledSpan } from "../logger";
import { SgTransaction } from "../subgraph/types";
import { SgTransaction, SubgraphVersions } from "../subgraph/types";
import { applyFilters } from "../subgraph/filter";

/** Syncs orders and vaults to upstream changes since the last fetch */
Expand Down Expand Up @@ -35,6 +35,7 @@ export async function syncOrders(this: OrderManager) {
},
BigInt(event.vault.vaultId),
event.vault.balance,
version === SubgraphVersions.V6 ? OrderbookVersions.V6 : ("" as any),
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

"" as any bypasses type safety and is fragile.

The updateVault signature declares orderbookVersion: OrderbookVersions, but the empty string "" is not a valid enum member. This compiles only because of the as any cast. If future code adds version-specific branches (e.g., for V4 or V5), the empty string will silently fall through.

Consider either making the parameter optional (orderbookVersion?: OrderbookVersions) or mapping non-V6 versions to a concrete enum value. This same pattern repeats on lines 54 and 66.

Option A: Make the parameter optional in updateVault

In src/order/index.ts:

 updateVault(
     orderbook: string,
     owner: string,
     token: TokenDetails,
     vaultId: bigint,
     balance: string | bigint,
-    orderbookVersion: OrderbookVersions,
+    orderbookVersion?: OrderbookVersions,
 ) {

Then in src/order/sync.ts:

-                version === SubgraphVersions.V6 ? OrderbookVersions.V6 : ("" as any),
+                version === SubgraphVersions.V6 ? OrderbookVersions.V6 : undefined,
🤖 Prompt for AI Agents 
In `@src/order/sync.ts` at line 38, The code passes an invalid "" cast as any into
updateVault's orderbookVersion parameter (seen where it computes version ===
SubgraphVersions.V6 ? OrderbookVersions.V6 : ("" as any)), which bypasses type
safety; update updateVault(orderbookVersion: OrderbookVersions) usage by either
making orderbookVersion optional in updateVault's declaration
(orderbookVersion?: OrderbookVersions) and handle undefined inside updateVault,
or replace the fallback with a concrete enum member (e.g., OrderbookVersions.V1
or a clearly named OrderbookVersions.Unknown) and handle that case; apply the
same fix to the other occurrences (the branches at the other two calls in
sync.ts).

);
}
if (event.__typename === "Clear" || event.__typename === "TakeOrder") {
Expand All @@ -50,6 +51,7 @@ export async function syncOrders(this: OrderManager) {
},
BigInt(trade.inputVaultBalanceChange.vault.vaultId),
trade.inputVaultBalanceChange.vault.balance,
version === SubgraphVersions.V6 ? OrderbookVersions.V6 : ("" as any),
);
this.updateVault(
trade.outputVaultBalanceChange.orderbook.id,
Expand All @@ -61,6 +63,7 @@ export async function syncOrders(this: OrderManager) {
},
BigInt(trade.outputVaultBalanceChange.vault.vaultId),
trade.outputVaultBalanceChange.vault.balance,
version === SubgraphVersions.V6 ? OrderbookVersions.V6 : ("" as any),
);
});
}
Expand Down
8 changes: 5 additions & 3 deletions src/order/types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
TakeOrdersConfigTypeV5,
} from "./v4";

export const ZERO_BYTES_32 = "0x0000000000000000000000000000000000000000000000000000000000000000";

// Re-export types from v3 and v4
export { OrderV3, OrderProfileV3, PairV3, TakeOrderDetailsV3, TakeOrdersConfigTypeV3, TakeOrderV3 };
export {
Expand Down Expand Up @@ -128,9 +130,9 @@ export type BundledOrders = {
};

export enum OrderbookVersions {
V4,
V5,
V6,
V4 = "v4",
V5 = "v5",
V6 = "v6",
}

export type PairBase = {
Expand Down
Loading
Loading