Skip to content
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.

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

feat: #1989: subaccount filter in ownedPositionIds #2006

Merged
merged 9 commits into from
Jan 29, 2025
11 changes: 11 additions & 0 deletions .changeset/lucky-snails-clean.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@penumbra-zone/storage': major
'@penumbra-zone/protobuf': minor
'@penumbra-zone/services': minor
'@penumbra-zone/types': minor
---

- storage: add subaccount filter to `getOwnedPositionIds` method
- protobuf: sync latest changes in penumbra protobufs
- services: add subaccount filter to `ownedPositionIds` method in ViewService
- types: update indexedDB schema
2 changes: 1 addition & 1 deletion packages/protobuf/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"gen:ibc": "buf generate buf.build/cosmos/ibc:7ab44ae956a0488ea04e04511efa5f70",
"gen:ics23": "buf generate buf.build/cosmos/ics23:55085f7c710a45f58fa09947208eb70b",
"gen:noble": "buf generate buf.build/noble-assets/forwarding:5a8609a6772d417584a9c60cd8b80881",
"gen:penumbra": "buf generate buf.build/penumbra-zone/penumbra:0a56a4f32c244e7eb277e02f6e85afbd",
"gen:penumbra": "buf generate buf.build/penumbra-zone/penumbra:649f1d61327144cb9a7e15c7ad210dcb",
"lint": "eslint src",
"lint:fix": "eslint src --fix",
"lint:strict": "tsc --noEmit && eslint src --max-warnings 0",
Expand Down
3 changes: 2 additions & 1 deletion packages/services/src/view-service/owned-position-ids.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ export const ownedPositionIds: Impl['ownedPositionIds'] = async function* (req,
for await (const positionId of indexedDb.getOwnedPositionIds(
req.positionState,
req.tradingPair,
req.subaccount,
)) {
yield { positionId: positionId };
yield { positionId: positionId, subaccount: req.subaccount };
}
};
20 changes: 17 additions & 3 deletions packages/storage/src/indexed-db/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ export class IndexedDb implements IndexedDbInterface {
async *getOwnedPositionIds(
positionState: PositionState | undefined,
tradingPair: TradingPair | undefined,
subaccount: AddressIndex | undefined,
) {
yield* new ReadableStream({
start: async cont => {
Expand All @@ -578,7 +579,10 @@ export class IndexedDb implements IndexedDbInterface {
const position = Position.fromJson(cursor.value.position);
if (
(!positionState || positionState.equals(position.state)) &&
(!tradingPair || tradingPair.equals(position.phi?.pair))
(!tradingPair || tradingPair.equals(position.phi?.pair)) &&
(!subaccount ||
(cursor.value.subaccount &&
subaccount.equals(AddressIndex.fromJson(cursor.value.subaccount))))
) {
cont.enqueue(PositionId.fromJson(cursor.value.id));
}
Expand All @@ -589,16 +593,25 @@ export class IndexedDb implements IndexedDbInterface {
});
}

async addPosition(positionId: PositionId, position: Position): Promise<void> {
async addPosition(
positionId: PositionId,
position: Position,
subaccount?: AddressIndex,
): Promise<void> {
assertPositionId(positionId);
const positionRecord = {
id: positionId.toJson() as Jsonified<PositionId>,
position: position.toJson() as Jsonified<Position>,
subaccount: subaccount && (subaccount.toJson() as Jsonified<AddressIndex>),
};
await this.u.update({ table: 'POSITIONS', value: positionRecord });
}

async updatePosition(positionId: PositionId, newState: PositionState): Promise<void> {
async updatePosition(
positionId: PositionId,
newState: PositionState,
subaccount?: AddressIndex,
): Promise<void> {
assertPositionId(positionId);
const key = uint8ArrayToBase64(positionId.inner);
const positionRecord = await this.db.get('POSITIONS', key);
Expand All @@ -615,6 +628,7 @@ export class IndexedDb implements IndexedDbInterface {
value: {
id: positionId.toJson() as Jsonified<PositionId>,
position: position.toJson() as Jsonified<Position>,
subaccount: subaccount ? (subaccount.toJson() as Jsonified<AddressIndex>) : undefined,
},
});
}
Expand Down
4 changes: 4 additions & 0 deletions packages/storage/src/indexed-db/indexed-db.test-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Transaction } from '@penumbra-zone/protobuf/penumbra/core/transaction/v
import type { ScanBlockResult } from '@penumbra-zone/types/state-commitment-tree';
import { base64ToUint8Array } from '@penumbra-zone/types/base64';
import { StateCommitment } from '@penumbra-zone/protobuf/penumbra/crypto/tct/v1/tct_pb';
import { AddressIndex } from '@penumbra-zone/protobuf/penumbra/core/keys/v1/keys_pb';

const hash3312332298 = base64ToUint8Array('JbOzRkf0VKm4eIM0DS27N5igX8jxvPhAMpBWSr2bj/Q=');

Expand Down Expand Up @@ -717,3 +718,6 @@ export const epoch3 = new Epoch({
index: 3n,
startHeight: 300n,
});

export const mainAccount = new AddressIndex({ account: 0 });
export const firstSubaccount = new AddressIndex({ account: 1 });
75 changes: 62 additions & 13 deletions packages/storage/src/indexed-db/indexed-db.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import {
tradingPairGmGn,
transaction,
transactionId,
mainAccount,
firstSubaccount,
} from './indexed-db.test-data.js';
import { AddressIndex, WalletId } from '@penumbra-zone/protobuf/penumbra/core/keys/v1/keys_pb';
import {
Expand Down Expand Up @@ -493,16 +495,26 @@ describe('IndexedDb', () => {
});

describe('positions', () => {
it('returns empty array for zero positions', async () => {
const db = await IndexedDb.initialize({ ...generateInitialProps() });

const ownedPositions: PositionId[] = [];
for await (const positionId of db.getOwnedPositionIds(undefined, undefined, undefined)) {
ownedPositions.push(positionId as PositionId);
}
expect(ownedPositions.length).toBe(0);
});

it('position should be added and their state should change', async () => {
Copy link
Contributor

@TalDerei TalDerei Jan 28, 2025

Choose a reason for hiding this comment

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

suggestion: could we consider adding a few additional tests?

  1. call getOwnedPositionIds with no positions in the database and check it returns an empty array,
  2. check that combining all filters (PositionState, tradingPair, subaccount) works and returns the expected position
  3. check that subaccount filtering mechanism works when multiple subaccounts exist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added 1 and 2, number 3 is sufficiently covered by 'should get all position with given subaccount index' test

const db = await IndexedDb.initialize({ ...generateInitialProps() });

await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy);
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy, mainAccount);
await db.updatePosition(
positionIdGmPenumbraBuy,
new PositionState({ state: PositionState_PositionStateEnum.CLOSED }),
);
const ownedPositions: PositionId[] = [];
for await (const positionId of db.getOwnedPositionIds(undefined, undefined)) {
for await (const positionId of db.getOwnedPositionIds(undefined, undefined, undefined)) {
ownedPositions.push(positionId as PositionId);
}
expect(ownedPositions.length).toBe(1);
Expand All @@ -521,27 +533,28 @@ describe('IndexedDb', () => {

it('should get all position ids', async () => {
const db = await IndexedDb.initialize({ ...generateInitialProps() });
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy);
await db.addPosition(positionIdGnPenumbraSell, positionGnPenumbraSell);
await db.addPosition(positionIdGmGnSell, positionGmGnSell);
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy, mainAccount);
await db.addPosition(positionIdGnPenumbraSell, positionGnPenumbraSell, mainAccount);
await db.addPosition(positionIdGmGnSell, positionGmGnSell, firstSubaccount);

const ownedPositions: PositionId[] = [];
for await (const positionId of db.getOwnedPositionIds(undefined, undefined)) {
for await (const positionId of db.getOwnedPositionIds(undefined, undefined, undefined)) {
ownedPositions.push(positionId as PositionId);
}
expect(ownedPositions.length).toBe(3);
});

it('should get all position with given position state', async () => {
const db = await IndexedDb.initialize({ ...generateInitialProps() });
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy);
await db.addPosition(positionIdGnPenumbraSell, positionGnPenumbraSell);
await db.addPosition(positionIdGmGnSell, positionGmGnSell);
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy, mainAccount);
await db.addPosition(positionIdGnPenumbraSell, positionGnPenumbraSell, mainAccount);
await db.addPosition(positionIdGmGnSell, positionGmGnSell, firstSubaccount);

const ownedPositions: PositionId[] = [];
for await (const positionId of db.getOwnedPositionIds(
new PositionState({ state: PositionState_PositionStateEnum.CLOSED }),
undefined,
undefined,
)) {
ownedPositions.push(positionId as PositionId);
}
Expand All @@ -550,16 +563,52 @@ describe('IndexedDb', () => {

it('should get all position with given trading pair', async () => {
const db = await IndexedDb.initialize({ ...generateInitialProps() });
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy);
await db.addPosition(positionIdGnPenumbraSell, positionGnPenumbraSell);
await db.addPosition(positionIdGmGnSell, positionGmGnSell);
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy, mainAccount);
await db.addPosition(positionIdGnPenumbraSell, positionGnPenumbraSell, mainAccount);
await db.addPosition(positionIdGmGnSell, positionGmGnSell, firstSubaccount);

const ownedPositions: PositionId[] = [];
for await (const positionId of db.getOwnedPositionIds(undefined, tradingPairGmGn)) {
for await (const positionId of db.getOwnedPositionIds(
undefined,
tradingPairGmGn,
undefined,
)) {
ownedPositions.push(positionId as PositionId);
}
expect(ownedPositions.length).toBe(1);
});

it('should get all position with given subaccount index', async () => {
const db = await IndexedDb.initialize({ ...generateInitialProps() });
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy, mainAccount);
await db.addPosition(positionIdGnPenumbraSell, positionGnPenumbraSell, mainAccount);
await db.addPosition(positionIdGmGnSell, positionGmGnSell, firstSubaccount);

const ownedPositions: PositionId[] = [];
for await (const positionId of db.getOwnedPositionIds(undefined, undefined, mainAccount)) {
ownedPositions.push(positionId as PositionId);
}
expect(ownedPositions.length).toBe(2);
});

it('should filter positions correctly when all filters applied together', async () => {
const db = await IndexedDb.initialize({ ...generateInitialProps() });
await db.addPosition(positionIdGmPenumbraBuy, positionGmPenumbraBuy, mainAccount);
await db.addPosition(positionIdGnPenumbraSell, positionGnPenumbraSell, mainAccount);
await db.addPosition(positionIdGmGnSell, positionGmGnSell, firstSubaccount);

const ownedPositions: PositionId[] = [];
for await (const positionId of db.getOwnedPositionIds(
new PositionState({ state: PositionState_PositionStateEnum.CLOSED }),
tradingPairGmGn,
firstSubaccount,
)) {
ownedPositions.push(positionId as PositionId);
}

expect(ownedPositions.length).toBe(1);
expect(ownedPositions[0]?.equals(positionIdGmGnSell)).toBeTruthy();
});
});

describe('prices', () => {
Expand Down
10 changes: 8 additions & 2 deletions packages/types/src/indexed-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,14 @@ export interface IndexedDbInterface {
getOwnedPositionIds(
positionState: PositionState | undefined,
tradingPair: TradingPair | undefined,
subaccount: AddressIndex | undefined,
): AsyncGenerator<PositionId, void>;
addPosition(positionId: PositionId, position: Position): Promise<void>;
updatePosition(positionId: PositionId, newState: PositionState): Promise<void>;
addPosition(positionId: PositionId, position: Position, subaccount?: AddressIndex): Promise<void>;
Copy link
Contributor

Choose a reason for hiding this comment

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

ACK subaccount in POSITIONS table

Screenshot 2025-01-28 at 7 55 29 PM

updatePosition(
positionId: PositionId,
newState: PositionState,
subaccount?: AddressIndex,
): Promise<void>;
addEpoch(startHeight: bigint): Promise<void>;
getEpochByHeight(height: bigint): Promise<Epoch | undefined>;
upsertValidatorInfo(validatorInfo: ValidatorInfo): Promise<void>;
Expand Down Expand Up @@ -297,6 +302,7 @@ export interface PenumbraDb extends DBSchema {
export interface PositionRecord {
id: Jsonified<PositionId>; // PositionId (must be JsonValue because ['id']['inner'] is a key )
position: Jsonified<Position>; // Position
subaccount?: Jsonified<AddressIndex>; // Position AddressIndex
}

export type Tables = Record<string, StoreNames<PenumbraDb>>;
Expand Down