fix(bug): address ?at bug and endingOffset conditionals (#824)

* fix: current auction endingOffset bug * fix: header bug * update tests to accurately test the updates * add vrfDelay to endingOffset * refactor logic to make it cleaner * Update src/services/paras/ParasService.ts Co-authored-by: Zeke Mostov <z.mostov@gmail.com> * Update src/services/paras/ParasService.ts Co-authored-by: Zeke Mostov <z.mostov@gmail.com> * remove vrfDelay from offset calculation * update docs with vrfDelay info * add tests for startPeriod, endPeriod and vrfDelay * inline docs * fix some grumbles for vrfDelay, and endPeriod sample * Update src/services/paras/ParasService.ts * Update docs * Update src/services/paras/ParasService.ts Co-authored-by: James Wilson <james@jsdw.me> * fix naming in test * set the vrfDelay test to use the finishEnd block * update docs Co-authored-by: Zeke Mostov <z.mostov@gmail.com> Co-authored-by: James Wilson <james@jsdw.me>
paritytech · Feb 1, 2022 · 1774e90 · 1774e90
1 parent 23d4cec
commit 1774e90
Show file tree

Hide file tree

Showing 4 changed files with 150 additions and 51 deletions.
diff --git a/docs/dist/app.bundle.js b/docs/dist/app.bundle.js
diff --git a/docs/src/openapi-v1.yaml b/docs/src/openapi-v1.yaml
@@ -1044,8 +1044,12 @@ paths:
       summary: |
         Get the status of the current auction.
       description: |
-        Returns an overview of the current of auction. There is only one auction
-        at a time. If there is no auction most fields will be `null`.
+        Returns an overview of the current auction. There is only one auction
+        at a time. If there is no auction most fields will be `null`. If the current
+        auction phase is in `vrfDelay` and you are looking to retrieve the latest winning
+        bids, it is advised to query one block before `finishEnd` in the `endingPeriod` phase
+        for that auction as there technically are no winners during the `vrfDelay` and thus 
+        the field is `null`.
       parameters:
       - name: at
         in: query

diff --git a/src/services/paras/ParasService.spec.ts b/src/services/paras/ParasService.spec.ts
@@ -17,7 +17,6 @@ import {
 	TypeFactory,
 } from '../../test-helpers/typeFactory';
 import {
-	blockHash20000,
 	blockHash789629,
 	defaultMockApi,
 	mockBlock789629,
@@ -201,11 +200,11 @@ const slotsLeasesEntriesAt = () =>
  */
 export const auctionsInfoAt = (): Promise<Option<Vec<BlockNumber>>> =>
 	Promise.resolve().then(() => {
-		const beingEnd = rococoRegistry.createType('BlockNumber', 1000);
+		const beginEnd = rococoRegistry.createType('BlockNumber', 780000);
 		const leasePeriodIndex = rococoRegistry.createType('BlockNumber', 39);
 		const vectorAuctions = rococoTypeFactory.vecOf([
 			leasePeriodIndex,
-			beingEnd,
+			beginEnd,
 		]);
 		const optionAuctions = rococoTypeFactory.optionOf(vectorAuctions);
 
@@ -469,7 +468,30 @@ describe('ParasService', () => {
 	});
 
 	describe('ParasService.auctionsCurrent', () => {
-		it('Should return the correct data during an ongoing auction', async () => {
+		/**
+		 * Helper function to generate a a header to use to override the current one.
+		 * This allows us to change the expected block we are using here as our head
+		 * to test for a specific `phase` in an auction.
+		 *
+		 * @param blockNumber Current block head returned by header
+		 * @returns
+		 */
+		const generateOverrideHeader = (
+			blockNumber: number
+		): Record<string, unknown> => {
+			return {
+				parentHash:
+					'0x205da5dba43bbecae52b44912249480aa9f751630872b6b6ba1a9d2aeabf0177',
+				number: blockNumber,
+				stateRoot:
+					'0x023b5bb1bc10a1a91a9ef683f46a8bb09666c50476d5592bd6575a73777eb173',
+				extrinsicsRoot:
+					'0x4c1d65bf6b57086f00d5df40aa0686ffbc581ef60878645613b1fc3303de5030',
+				digest: {},
+			};
+		};
+
+		it('Should return the correct data during an ongoing endPeriod phase', async () => {
 			const leasePeriodIndex = new BN(39);
 			const leaseIndexArray = parasService['enumerateLeaseSets'](
 				historicApi,
@@ -485,9 +507,9 @@ describe('ParasService', () => {
 
 			const expectedResponse = {
 				at: expectedAt,
-				beginEnd: '1000',
-				finishEnd: '21000',
-				phase: 'vrfDelay',
+				beginEnd: '780000',
+				finishEnd: '800000',
+				phase: 'endPeriod',
 				auctionIndex: '4',
 				leasePeriods: ['39', '40', '41', '42'],
 				winning: [
@@ -516,33 +538,60 @@ describe('ParasService', () => {
 			expect(sanitizeNumbers(response)).toMatchObject(expectedResponse);
 		});
 
-		/**
-		 * The goal of this test is to manipulate the number of the finalized block so that it is less than
-		 * the expected `finishHead`, but higher the `beginEnd` which would denote we are in the `endPeriod` phase
-		 * of the current auction.
-		 */
-		it('Should return the correct `ending` phase', async () => {
-			const overrideHeader = {
-				parentHash:
-					'0x3d489d71f8fd2e15259df5059a1497436e6b73497500a303b1a705993e25cb27',
-				number: 20000,
-				stateRoot:
-					'0xa0089595e48850a8a00081dd987a4735d0e8f94ac98af89030521f23f6cb8e31',
-				extrinsicsRoot:
-					'0x2d5d3fdb96b487d480b08b64ed69a65433c1713ae3579dd23704cb790aa3b2ae',
-				digest: {},
+		it('Should return the correct data during a startPeriod phase', async () => {
+			const overrideHeader = generateOverrideHeader(770000);
+			const header = polkadotRegistry.createType('Header', overrideHeader);
+
+			// Override the mockApi
+			(mockApi.rpc.chain.getHeader as unknown) = () =>
+				Promise.resolve().then(() => header);
+
+			const expectedResponse = {
+				at: {
+					hash: '0x7b713de604a99857f6c25eacc115a4f28d2611a23d9ddff99ab0e4f1c17a8578',
+					height: '770000',
+				},
+				beginEnd: '780000',
+				finishEnd: '800000',
+				phase: 'startPeriod',
+				auctionIndex: '4',
+				leasePeriods: ['39', '40', '41', '42'],
+				winning: null,
 			};
+
+			const response = await parasService.auctionsCurrent(blockHash789629);
+
+			expect(sanitizeNumbers(response)).toStrictEqual(expectedResponse);
+
+			// Set the MockApi back to its original self
+			(mockApi.rpc.chain.getHeader as unknown) = () =>
+				Promise.resolve().then(() => mockBlock789629.header);
+		});
+
+		it('Should return the correct data during a vrfDelay phase', async () => {
+			const overrideHeader = generateOverrideHeader(800000);
 			const header = polkadotRegistry.createType('Header', overrideHeader);
 
 			// Override the mockApi
 			(mockApi.rpc.chain.getHeader as unknown) = () =>
 				Promise.resolve().then(() => header);
 
-			const expectedResponse = 'endPeriod';
+			const expectedResponse = {
+				at: {
+					hash: '0x7b713de604a99857f6c25eacc115a4f28d2611a23d9ddff99ab0e4f1c17a8578',
+					height: '800000',
+				},
+				beginEnd: '780000',
+				finishEnd: '800000',
+				phase: 'vrfDelay',
+				auctionIndex: '4',
+				leasePeriods: ['39', '40', '41', '42'],
+				winning: null,
+			};
 
-			const response = await parasService.auctionsCurrent(blockHash20000);
+			const response = await parasService.auctionsCurrent(blockHash789629);
 
-			expect(response.phase).toBe(expectedResponse);
+			expect(sanitizeNumbers(response)).toStrictEqual(expectedResponse);
 
 			// Set the MockApi back to its original self
 			(mockApi.rpc.chain.getHeader as unknown) = () =>

diff --git a/src/services/paras/ParasService.ts b/src/services/paras/ParasService.ts
@@ -13,7 +13,7 @@ import {
 	WinningData,
 } from '@polkadot/types/interfaces';
 import { ITuple } from '@polkadot/types/types';
-import { BN_ONE, BN_ZERO } from '@polkadot/util';
+import { BN_ZERO } from '@polkadot/util';
 import BN from 'bn.js';
 import { InternalServerError } from 'http-errors';
 
@@ -223,21 +223,47 @@ export class ParasService extends AbstractService {
 			phase: IOption<AuctionPhase>,
 			winning;
 		if (auctionInfoOpt.isSome) {
+			/**
+			 * If `AuctionInfo:::<T>:get()` is `Some`, it returns a tuple where the first item is the
+			 * lease period index that the first of the four contiguous lease periods
+			 * of an auction is for. The second is the block number when the auction will
+			 * 'being to end', i.e. the first block of the Ending Period of the auction
+			 */
 			[leasePeriodIndex, beginEnd] = auctionInfoOpt.unwrap();
-			const endingOffset = this.endingOffset(blockNumber, beginEnd);
-
-			const winningOpt = endingOffset
-				? await historicApi.query.auctions.winning<Option<WinningData>>(
-						endingOffset
-				  )
-				: await historicApi.query.auctions.winning<Option<WinningData>>(
-						// when we are not in the ending phase of the auction winning bids are stored at 0
-						0
-				  );
-
-			if (winningOpt.isSome) {
+
+			/**
+			 * End of current auctions endPeriod.
+			 */
+			finishEnd = beginEnd.add(endingPeriod);
+			/**
+			 * We determine what our phase is so we can decide how to calculate our
+			 * ending offset.
+			 */
+			if (finishEnd.lte(blockNumber)) {
+				phase = 'vrfDelay';
+			} else {
+				phase = beginEnd.gt(blockNumber) ? 'startPeriod' : 'endPeriod';
+			}
+
+			/**
+			 * The endingOffset according to polkadot has two potential phases
+			 * where this will be a valid block number. Both `startPeriod`, and `endPeriod`
+			 * have valid offsets.
+			 */
+			const endingOffset = this.endingOffset(
+				blockNumber,
+				beginEnd,
+				phase,
+				historicApi
+			);
+
+			if (endingOffset) {
 				const ranges = this.enumerateLeaseSets(historicApi, leasePeriodIndex);
 
+				const winningOpt = await historicApi.query.auctions.winning<
+					Option<WinningData>
+				>(endingOffset);
+
 				// zip the winning bids together with their enumerated `SlotRange` (aka `leaseSet`)
 				winning = winningOpt.unwrap().map((bid, idx) => {
 					const leaseSet = ranges[idx];
@@ -255,14 +281,6 @@ export class ParasService extends AbstractService {
 			} else {
 				winning = null;
 			}
-
-			finishEnd = beginEnd.add(endingPeriod);
-
-			if (finishEnd.lt(blockNumber)) {
-				phase = 'vrfDelay';
-			} else {
-				phase = beginEnd.gt(blockNumber) ? 'startPeriod' : 'endPeriod';
-			}
 		} else {
 			leasePeriodIndex = null;
 			beginEnd = null;
@@ -426,8 +444,15 @@ export class ParasService extends AbstractService {
 	 *
 	 * @param now current block number
 	 * @param beginEnd block number of the start of the auction's ending period
+	 * @param phase Current phase the auction is in
+	 * @param historicApi api specific to the queried blocks runtime
 	 */
-	private endingOffset(now: BN, beginEnd: IOption<BN>): IOption<BN> {
+	private endingOffset(
+		now: BN,
+		beginEnd: IOption<BN>,
+		phase: string,
+		historicApi: ApiDecoration<'promise'>
+	): IOption<BN> {
 		if (isNull(beginEnd)) {
 			return null;
 		}
@@ -437,7 +462,28 @@ export class ParasService extends AbstractService {
 			return null;
 		}
 
-		return now.sub(new BN(beginEnd)).iadd(BN_ONE);
+		/**
+		 * The length of each sample to take during the ending period.
+		 *
+		 * A sample can be represented by `afterEarlyEnd` / `sampleLength`.
+		 * When we are in the endingPeriod, the offset is represented by:
+		 * `sample`.
+		 *
+		 * If the current phase is in `vrfDelay`, and you are interested in
+		 * querying the winners of the auction that just finished, it is advised
+		 * to query the last block of the `endingPeriod`.
+		 */
+		const sampleLength = historicApi.consts.auctions
+			.sampleLength as BlockNumber;
+
+		switch (phase) {
+			case 'startPeriod':
+				return BN_ZERO;
+			case 'endPeriod':
+				return afterEarlyEnd.div(sampleLength);
+			default:
+				return null;
+		}
 	}
 
 	/**