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

Handle includeInactive in ViewService#auctions #1047

Merged
merged 10 commits into from
May 8, 2024
2 changes: 1 addition & 1 deletion apps/extension/.env.testnet
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
CHAIN_ID=penumbra-testnet-deimos-7
IDB_VERSION=37
IDB_VERSION=38
MINIFRONT_URL=https://app.testnet.penumbra.zone
PRAX=lkpmkhpnhknhmibgnmmhdhgdilepfghe
43 changes: 28 additions & 15 deletions packages/query/src/block-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,29 +419,42 @@ export class BlockProcessor implements BlockProcessorInterface {
private async identifyAuctionNfts(action: Action['action']) {
if (action.case === 'actionDutchAuctionSchedule' && action.value.description) {
const auctionId = getAuctionId(action.value.description);
const metadata = getAuctionNftMetadata(
auctionId,
// Always a sequence number of 0 when starting a Dutch auction
0n,
);

// Always a sequence number of 0 when starting a Dutch auction
const seqNum = 0n;

const metadata = getAuctionNftMetadata(auctionId, seqNum);

await Promise.all([
this.indexedDb.saveAssetsMetadata(metadata),
this.indexedDb.upsertAuction(auctionId, {
auction: action.value.description,
seqNum,
}),
]);
} else if (action.case === 'actionDutchAuctionEnd' && action.value.auctionId) {
const metadata = getAuctionNftMetadata(
action.value.auctionId,
// Always a sequence number of 1 when ending a Dutch auction
1n,
);
await this.indexedDb.saveAssetsMetadata(metadata);
// Always a sequence number of 1 when ending a Dutch auction
const seqNum = 1n;

const metadata = getAuctionNftMetadata(action.value.auctionId, seqNum);
Copy link
Contributor

Choose a reason for hiding this comment

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

won't this request metadata for closed auctions since it's passing in seqNum of 1? Shouldn't it pass seqNum of 0 for active auctions, and only after save the metadata and update the auction sequence number? Or does it not matter here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TalDerei

won't this request metadata for closed auctions since it's passing in seqNum of 1?

Correct — although it's not exactly requesting the metadata, but rather generating it. (getAuctioNftMetadata() calls a WASM function under the hood that generates metadata for an asset that doesn't exist yet.)

Shouldn't it pass seqNum of 0 for active auctions, and only after save the metadata and update the auction sequence number?

Hm — we could request the NFT metadata for the open auction, and the modify its sequence number. But that feels a bit messy to me — if the algorithm for generating NFT metadata changes in the future such that closed-auction NFTs have different metadata than open-auction NFTs, this would break. Easier to just generate it from scratch for each auction state.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah was thinking about that, I think generating the metadata is the right design choice here.

Copy link
Contributor

@TalDerei TalDerei May 8, 2024

Choose a reason for hiding this comment

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

requesting

wrong word choice here 😅


await Promise.all([
this.indexedDb.saveAssetsMetadata(metadata),
this.indexedDb.upsertAuction(action.value.auctionId, { seqNum }),
]);
} else if (action.case === 'actionDutchAuctionWithdraw') {
const auctionId = action.value.auctionId;
if (!auctionId) return;

const metadata = getAuctionNftMetadata(auctionId, action.value.seq);

await Promise.all([
this.indexedDb.saveAssetsMetadata(metadata),
this.indexedDb.upsertAuction(auctionId, {
seqNum: action.value.seq,
}),
]);
}
/**
* @todo Handle `actionDutchAuctionWithdraw`, and figure out how to
* determine the sequence number if there have been multiple withdrawals.
*/
}

/**
Expand Down
24 changes: 22 additions & 2 deletions packages/services/src/view-service/auctions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,27 +86,31 @@ const TEST_DATA = [
value: {
auction: MOCK_AUCTION_1,
noteCommitment: new StateCommitment({ inner: new Uint8Array([0, 1, 2, 3]) }),
seqNum: 0n,
},
},
{
id: AUCTION_ID_2,
value: {
auction: MOCK_AUCTION_2,
noteCommitment: new StateCommitment({ inner: new Uint8Array([0, 1, 2, 3]) }),
seqNum: 1n,
},
},
{
id: AUCTION_ID_3,
value: {
auction: MOCK_AUCTION_3,
noteCommitment: new StateCommitment({ inner: new Uint8Array([0, 1, 2, 3]) }),
seqNum: 2n,
},
},
{
id: AUCTION_ID_4,
value: {
auction: MOCK_AUCTION_4,
noteCommitment: new StateCommitment({ inner: new Uint8Array([0, 1, 2, 3]) }),
seqNum: 0n,
},
},
];
Expand Down Expand Up @@ -146,9 +150,9 @@ describe('Auctions request handler', () => {

it('returns auctions', async () => {
const req = new AuctionsRequest();
const result = await Array.fromAsync(auctions(req, mockCtx));
const results = await Array.fromAsync(auctions(req, mockCtx));

expect(result[0]).toEqual(
expect(results[0]).toEqual(
new AuctionsResponse({
id: AUCTION_ID_1,
auction: {
Expand All @@ -174,4 +178,20 @@ describe('Auctions request handler', () => {
);
});
});

it('excludes inactive auctions by default', async () => {
const req = new AuctionsRequest();
const results = await Array.fromAsync(auctions(req, mockCtx));

expect(results.some(result => new AuctionId(result.id).equals(AUCTION_ID_2))).toBe(false);
expect(results.some(result => new AuctionId(result.id).equals(AUCTION_ID_3))).toBe(false);
});

it('includes inactive auctions if `includeInactive` is `true`', async () => {
const req = new AuctionsRequest({ includeInactive: true });
const results = await Array.fromAsync(auctions(req, mockCtx));

expect(results.some(result => new AuctionId(result.id).equals(AUCTION_ID_2))).toBe(true);
expect(results.some(result => new AuctionId(result.id).equals(AUCTION_ID_3))).toBe(true);
});
});
7 changes: 3 additions & 4 deletions packages/services/src/view-service/auctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ const getBech32mAuctionId = (
return captureGroups.auctionId;
};

const isInactive = (seqNum?: bigint) => (seqNum === undefined ? false : seqNum > 0n);
Copy link
Contributor

Choose a reason for hiding this comment

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

why would the sequence number ever be undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it optional since value (used on line 61 below) may have an undefined seqNum property if we haven't saved it yet for some reason.


const iterateAuctionsThisUserControls = async function* (
ctx: HandlerContext,
accountFilter?: AddressIndex,
Expand All @@ -49,17 +51,14 @@ export const auctions: Impl['auctions'] = async function* (req, ctx) {
if (queryLatestState) {
throw new ConnectError('`queryLatestState` not yet implemented', Code.Unimplemented);
}
if (includeInactive) {
throw new ConnectError('`includeInactive` not yet implemented', Code.Unimplemented);
}

const services = await ctx.values.get(servicesCtx)();
const { indexedDb } = await services.getWalletServices();

for await (const auctionId of iterateAuctionsThisUserControls(ctx, accountFilter)) {
/** @todo: if (req.includeInactive && auctionIsInactive()) continue; */
const id = new AuctionId(auctionIdFromBech32(auctionId));
const value = await indexedDb.getAuction(id);
if (!includeInactive && isInactive(value.seqNum)) continue;

let noteRecord: SpendableNoteRecord | undefined;
if (value.noteCommitment) {
Expand Down
5 changes: 5 additions & 0 deletions packages/storage/src/indexed-db/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ export class IndexedDb implements IndexedDbInterface {
value: {
auction?: T;
noteCommitment?: StateCommitment;
seqNum?: bigint;
},
): Promise<void> {
const key = uint8ArrayToBase64(auctionId.inner);
Expand All @@ -700,13 +701,15 @@ export class IndexedDb implements IndexedDbInterface {
const noteCommitment =
(value.noteCommitment?.toJson() as Jsonified<StateCommitment> | undefined) ??
existingRecord?.noteCommitment;
const seqNum = value.seqNum ?? existingRecord?.seqNum;

await this.u.update({
table: 'AUCTIONS',
key,
value: {
auction,
noteCommitment,
seqNum,
},
});
}
Expand All @@ -715,6 +718,7 @@ export class IndexedDb implements IndexedDbInterface {
// Add more auction union types as they are created
auction?: DutchAuctionDescription;
noteCommitment?: StateCommitment;
seqNum?: bigint;
}> {
const result = await this.db.get('AUCTIONS', uint8ArrayToBase64(auctionId.inner));

Expand All @@ -723,6 +727,7 @@ export class IndexedDb implements IndexedDbInterface {
noteCommitment: result?.noteCommitment
? StateCommitment.fromJson(result.noteCommitment)
: undefined,
seqNum: result?.seqNum,
};
}
}
33 changes: 29 additions & 4 deletions packages/storage/src/indexed-db/indexed-db.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -671,10 +671,11 @@ describe('IndexedDb', () => {
});
});

it('inserts an auction and then updates with a note commitment when given the same auction ID', async () => {
it('inserts an auction and sequence number, and then updates with a note commitment when given the same auction ID', async () => {
const auctionId = new AuctionId({ inner: new Uint8Array([0, 1, 2, 3]) });
const auction = new DutchAuctionDescription({ startHeight: 1234n });
await db.upsertAuction(auctionId, { auction });
const seqNum = 0n;
await db.upsertAuction(auctionId, { auction, seqNum });

let fetchedAuction = await db.getAuction(auctionId);
expect(fetchedAuction).toBeTruthy();
Expand All @@ -688,10 +689,11 @@ describe('IndexedDb', () => {
expect(fetchedAuction).toEqual({
auction,
noteCommitment,
seqNum,
});
});

it('inserts a note commitment and then updates with an auction when given the same auction ID', async () => {
it('inserts a note commitment and then updates with an auction and sequence number when given the same auction ID', async () => {
const auctionId = new AuctionId({ inner: new Uint8Array([0, 1, 2, 3]) });
const noteCommitment = new StateCommitment({ inner: new Uint8Array([0, 1, 2, 3]) });
await db.upsertAuction(auctionId, { noteCommitment });
Expand All @@ -700,14 +702,37 @@ describe('IndexedDb', () => {
expect(fetchedAuction).toBeTruthy();

const auction = new DutchAuctionDescription({ startHeight: 1234n });
await db.upsertAuction(auctionId, { auction });
const seqNum = 0n;
await db.upsertAuction(auctionId, { auction, seqNum });

fetchedAuction = await db.getAuction(auctionId);
expect(fetchedAuction).toBeTruthy();

expect(fetchedAuction).toEqual({
auction,
noteCommitment,
seqNum,
});
});

it('inserts all data, and then updates with a sequence number when given the same auction ID', async () => {
const auctionId = new AuctionId({ inner: new Uint8Array([0, 1, 2, 3]) });
const auction = new DutchAuctionDescription({ startHeight: 1234n });
const noteCommitment = new StateCommitment({ inner: new Uint8Array([0, 1, 2, 3]) });
await db.upsertAuction(auctionId, { auction, noteCommitment, seqNum: 0n });

let fetchedAuction = await db.getAuction(auctionId);
expect(fetchedAuction).toBeTruthy();

await db.upsertAuction(auctionId, { seqNum: 1n });

fetchedAuction = await db.getAuction(auctionId);
expect(fetchedAuction).toBeTruthy();

expect(fetchedAuction).toEqual({
auction,
noteCommitment,
seqNum: 1n,
});
});
});
Expand Down
9 changes: 9 additions & 0 deletions packages/types/src/indexed-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,15 @@ export interface IndexedDbInterface {
value: {
auction?: T;
noteCommitment?: StateCommitment;
seqNum?: bigint;
},
): Promise<void>;

getAuction(auctionId: AuctionId): Promise<{
// Add more auction union types as they are created
auction?: DutchAuctionDescription;
noteCommitment?: StateCommitment;
seqNum?: bigint;
}>;
}

Expand Down Expand Up @@ -218,6 +220,13 @@ export interface PenumbraDb extends DBSchema {
noteCommitment?: Jsonified<StateCommitment>;
// Add more types to `auction` as more auction types are created
auction?: Jsonified<DutchAuctionDescription>;
/**
* For Dutch auctions:
* `0n`: auction is active
* `1n`: auction has ended
* `2n`+: the user has withdrawn funds from the auction
*/
seqNum?: bigint;
};
};
}
Expand Down
Loading