refactor: throw in coputeRoundTiming, convert to null when called

Agoric · May 15, 2023 · b578f74 · b578f74
1 parent 041fe10
commit b578f74
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 112 deletions.
diff --git a/packages/inter-protocol/src/auction/scheduleMath.js b/packages/inter-protocol/src/auction/scheduleMath.js
@@ -5,7 +5,7 @@ import { natSafeMath } from '@agoric/zoe/src/contractSupport/index.js';
 import { assertAllDefined, makeTracer } from '@agoric/internal';
 
 const { subtract, multiply, floorDivide } = natSafeMath;
-const { details: X, Fail } = assert;
+const { Fail } = assert;
 
 const trace = makeTracer('SMath', false);
 
@@ -24,48 +24,25 @@ export const computeRoundTiming = (params, baseTime) => {
   /** @type {NatValue} */
   const lowestRate = params.getLowestRate();
 
-  const noNextAuction = msg => {
-    console.error(assert.error(msg));
-    return undefined;
-  };
-
   /** @type {RelativeTime} */
   const startDelay = params.getAuctionStartDelay();
-  if (TimeMath.compareRel(freq, startDelay) <= 0) {
-    return noNextAuction(
-      X`startFrequency must exceed startDelay, ${freq}, ${startDelay}`,
-    );
-  }
-  if (TimeMath.compareRel(freq, lockPeriod) <= 0) {
-    return noNextAuction(
-      X`startFrequency must exceed lock period, ${freq}, ${lockPeriod}`,
-    );
-  }
-
-  if (startingRate <= lowestRate) {
-    return noNextAuction(
-      X`startingRate ${startingRate} must be more than lowest: ${lowestRate}`,
-    );
-  }
+  TimeMath.compareRel(freq, startDelay) > 0 ||
+    Fail`startFrequency must exceed startDelay, ${freq}, ${startDelay}`;
+  TimeMath.compareRel(freq, lockPeriod) > 0 ||
+    Fail`startFrequency must exceed lock period, ${freq}, ${lockPeriod}`;
 
+  startingRate > lowestRate ||
+    Fail`startingRate ${startingRate} must be more than lowest: ${lowestRate}`;
   const rateChange = subtract(startingRate, lowestRate);
   const requestedSteps = floorDivide(rateChange, discountStep);
-
-  if (requestedSteps <= 0n) {
-    return noNextAuction(
-      X`discountStep ${discountStep} too large for requested rates`,
-    );
-  }
-
-  if (TimeMath.compareRel(freq, clockStep) < 0) {
-    return noNextAuction(
-      X`clockStep ${TimeMath.relValue(
-        clockStep,
-      )} must be shorter than startFrequency ${TimeMath.relValue(
-        freq,
-      )} to allow at least one step down`,
-    );
-  }
+  requestedSteps > 0n ||
+    Fail`discountStep ${discountStep} too large for requested rates`;
+  TimeMath.compareRel(freq, clockStep) >= 0 ||
+    Fail`clockStep ${TimeMath.relValue(
+      clockStep,
+    )} must be shorter than startFrequency ${TimeMath.relValue(
+      freq,
+    )} to allow at least one step down`;
 
   const requestedDuration = TimeMath.multiplyRelNat(clockStep, requestedSteps);
   const targetDuration =
@@ -75,12 +52,8 @@ export const computeRoundTiming = (params, baseTime) => {
   const steps = TimeMath.divideRelRel(targetDuration, clockStep);
   const duration = TimeMath.multiplyRelNat(clockStep, steps);
 
-  if (steps <= 0n) {
-    return noNextAuction(
-      X`clockStep ${clockStep} too long for auction duration ${duration}`,
-    );
-  }
-
+  steps > 0n ||
+    Fail`clockStep ${clockStep} too long for auction duration ${duration}`;
   const endRate = subtract(startingRate, multiply(steps, discountStep));
 
   const actualDuration = TimeMath.multiplyRelNat(clockStep, steps);

diff --git a/packages/inter-protocol/src/auction/scheduler.js b/packages/inter-protocol/src/auction/scheduler.js
@@ -57,6 +57,15 @@ const makeCancelToken = makeCancelTokenMaker('scheduler');
  *    will take place
  */
 
+const safelyComputeRoundTiming = (params, baseTime) => {
+  try {
+    return computeRoundTiming(params, baseTime);
+  } catch (e) {
+    console.error(assert.error(e));
+    return null;
+  }
+};
+
 /**
  * @param {AuctionDriver} auctionDriver
  * @param {import('@agoric/time/src/types').TimerService} timer
@@ -86,7 +95,7 @@ export const makeScheduler = async (
       // XXX manualTimer returns a bigint, not a timeRecord.
       E(timer).getCurrentTimestamp(),
       now => {
-        const nextSchedule = computeRoundTiming(
+        const nextSchedule = safelyComputeRoundTiming(
           params,
           TimeMath.coerceTimestampRecord(now, timerBrand),
         );
@@ -145,7 +154,7 @@ export const makeScheduler = async (
           now,
           TimeMath.coerceRelativeTimeRecord(1n, timerBrand),
         );
-        nextSchedule = computeRoundTiming(params, afterNow);
+        nextSchedule = safelyComputeRoundTiming(params, afterNow);
       }
 
       liveSchedule = undefined;
@@ -285,7 +294,7 @@ export const makeScheduler = async (
             nextSchedule.startTime,
           )}. Skipping that round.`,
         );
-        nextSchedule = computeRoundTiming(params, now);
+        nextSchedule = safelyComputeRoundTiming(params, now);
       } else {
         console.warn(`Auction started late by ${q(late)}. Starting ${q(now)}`);
       }
@@ -296,7 +305,7 @@ export const makeScheduler = async (
     }
 
     const after = TimeMath.addAbsRel(liveSchedule.endTime, relativeTime(1n));
-    nextSchedule = computeRoundTiming(params, after);
+    nextSchedule = safelyComputeRoundTiming(params, after);
     if (!nextSchedule) {
       return;
     }

diff --git a/packages/inter-protocol/test/auction/test-scheduleMath.js b/packages/inter-protocol/test/auction/test-scheduleMath.js
@@ -61,10 +61,10 @@ const checkSchedule = (t, params, baseTime, rawExpect) => {
  * @param {ReturnType<makeDefaultParams>} params
  * @param {number} baseTime
  */
-const checkScheduleNull = (t, params, baseTime) => {
-  const schedule = computeRoundTiming(params, coerceAbs(baseTime));
-
-  t.is(schedule, undefined);
+const checkScheduleThrows = async (t, params, baseTime, message) => {
+  t.throws(() => computeRoundTiming(params, coerceAbs(baseTime)), {
+    message,
+  });
 };
 
 // Hourly starts. 4 steps down, 5 price levels. discount steps of 10%.
@@ -116,30 +116,34 @@ test(
 // lock Period too Long
 test(
   'long lock period',
-  checkScheduleNull,
+  checkScheduleThrows,
   makeDefaultParams({ lock: 3600 }),
   100,
+  /startFrequency must exceed lock period, .*3600n.*3600n/,
 );
 
 test(
   'longer auction than freq',
-  checkScheduleNull,
+  checkScheduleThrows,
   makeDefaultParams({ freq: 500, lock: 300 }),
   100,
+  /clockStep "\[600n]" must be shorter than startFrequency "\[500n]" to allow at least one step down/,
 );
 
 test(
   'startDelay too long',
-  checkScheduleNull,
+  checkScheduleThrows,
   makeDefaultParams({ delay: 5000 }),
   100,
+  /startFrequency must exceed startDelay, .*3600n.*5000n/,
 );
 
 test(
   'large discount step',
-  checkScheduleNull,
+  checkScheduleThrows,
   makeDefaultParams({ discount: 5000n }),
   100,
+  /discountStep "\[5000n]" too large for requested rates/,
 );
 
 test(
@@ -160,9 +164,10 @@ test(
 
 test(
   'lowest rate higher than start',
-  checkScheduleNull,
+  checkScheduleThrows,
   makeDefaultParams({ lowest: 10_600n }),
   100,
+  /startingRate "\[10500n]" must be more than lowest: "\[10600n]"/,
 );
 
 // If the steps are small enough that we can't get to the end_rate, we'll cut

diff --git a/packages/inter-protocol/test/auction/test-scheduler.js b/packages/inter-protocol/test/auction/test-scheduler.js
@@ -283,16 +283,22 @@ test('lowest >= starting', async t => {
   );
 
   const { subscriber } = makePublishKit();
-  const scheduler = await makeScheduler(
-    fakeAuctioneer,
-    timer,
-    paramManager,
-    timer.getTimerBrand(),
-    recorderKit.recorder,
-    // @ts-expect-error Oops. wrong kind of subscriber.
-    subscriber,
+  await t.throwsAsync(
+    () =>
+      makeScheduler(
+        fakeAuctioneer,
+        timer,
+        paramManager,
+        timer.getTimerBrand(),
+        recorderKit.recorder,
+        // @ts-expect-error Oops. wrong kind of subscriber.
+        subscriber,
+      ),
+    {
+      message:
+        /startingRate \\"\[105n]\\" must be more than lowest: \\"\[110n]\\"/,
+    },
   );
-  t.is(scheduler.getSchedule().nextAuctionSchedule, undefined);
 });
 
 test('zero time for auction', async t => {
@@ -332,16 +338,22 @@ test('zero time for auction', async t => {
   );
 
   const { subscriber } = makePublishKit();
-  const scheduler = await makeScheduler(
-    fakeAuctioneer,
-    timer,
-    paramManager,
-    timer.getTimerBrand(),
-    recorderKit.recorder,
-    // @ts-expect-error Oops. wrong kind of subscriber.
-    subscriber,
+  await t.throwsAsync(
+    () =>
+      makeScheduler(
+        fakeAuctioneer,
+        timer,
+        paramManager,
+        timer.getTimerBrand(),
+        recorderKit.recorder,
+        // @ts-expect-error Oops. wrong kind of subscriber.
+        subscriber,
+      ),
+    {
+      message:
+        /clockStep \\"\[3n]\\" must be shorter than startFrequency \\"\[2n]\\" to allow at least one step down/,
+    },
   );
-  t.is(scheduler.getSchedule().nextAuctionSchedule, undefined);
 });
 
 test('discountStep 0', async t => {
@@ -389,7 +401,7 @@ test('discountStep 0', async t => {
         // @ts-expect-error Oops. wrong kind of subscriber.
         subscriber,
       ),
-    { message: 'Division by zero' },
+    { message: /Division by zero/ },
   );
 });
 
@@ -428,16 +440,19 @@ test('discountStep larger than starting rate', async t => {
   );
 
   const { subscriber } = makePublishKit();
-  const scheduler = await makeScheduler(
-    fakeAuctioneer,
-    timer,
-    paramManager,
-    timer.getTimerBrand(),
-    recorderKit.recorder,
-    // @ts-expect-error Oops. wrong kind of subscriber.
-    subscriber,
+  await t.throwsAsync(
+    () =>
+      makeScheduler(
+        fakeAuctioneer,
+        timer,
+        paramManager,
+        timer.getTimerBrand(),
+        recorderKit.recorder,
+        // @ts-expect-error Oops. wrong kind of subscriber.
+        subscriber,
+      ),
+    { message: /discountStep .* too large for requested rates/ },
   );
-  t.is(scheduler.getSchedule().nextAuctionSchedule, undefined);
 });
 
 test('start Freq 0', async t => {
@@ -474,16 +489,19 @@ test('start Freq 0', async t => {
   );
 
   const { subscriber } = makePublishKit();
-  const scheduler = await makeScheduler(
-    fakeAuctioneer,
-    timer,
-    paramManager,
-    timer.getTimerBrand(),
-    recorderKit.recorder,
-    // @ts-expect-error Oops. wrong kind of subscriber.
-    subscriber,
+  await t.throwsAsync(
+    () =>
+      makeScheduler(
+        fakeAuctioneer,
+        timer,
+        paramManager,
+        timer.getTimerBrand(),
+        recorderKit.recorder,
+        // @ts-expect-error Oops. wrong kind of subscriber.
+        subscriber,
+      ),
+    { message: /startFrequency must exceed startDelay.*0n.*10n.*/ },
   );
-  t.is(scheduler.getSchedule().nextAuctionSchedule, undefined);
 });
 
 test('delay > freq', async t => {
@@ -521,16 +539,19 @@ test('delay > freq', async t => {
   );
 
   const { subscriber } = makePublishKit();
-  const scheduler = await makeScheduler(
-    fakeAuctioneer,
-    timer,
-    paramManager,
-    timer.getTimerBrand(),
-    recorderKit.recorder,
-    // @ts-expect-error Oops. wrong kind of subscriber.
-    subscriber,
+  await t.throwsAsync(
+    () =>
+      makeScheduler(
+        fakeAuctioneer,
+        timer,
+        paramManager,
+        timer.getTimerBrand(),
+        recorderKit.recorder,
+        // @ts-expect-error Oops. wrong kind of subscriber.
+        subscriber,
+      ),
+    { message: /startFrequency must exceed startDelay.*\[20n\].*\[40n\].*/ },
   );
-  t.is(scheduler.getSchedule().nextAuctionSchedule, undefined);
 });
 
 test('lockPeriod > freq', async t => {
@@ -569,16 +590,21 @@ test('lockPeriod > freq', async t => {
   );
 
   const { subscriber } = makePublishKit();
-  const scheduler = await makeScheduler(
-    fakeAuctioneer,
-    timer,
-    paramManager,
-    timer.getTimerBrand(),
-    recorderKit.recorder,
-    // @ts-expect-error Oops. wrong kind of subscriber.
-    subscriber,
+  await t.throwsAsync(
+    () =>
+      makeScheduler(
+        fakeAuctioneer,
+        timer,
+        paramManager,
+        timer.getTimerBrand(),
+        recorderKit.recorder,
+        // @ts-expect-error Oops. wrong kind of subscriber.
+        subscriber,
+      ),
+    {
+      message: /startFrequency must exceed lock period.*\[3600n\].*\[7200n\].*/,
+    },
   );
-  t.is(scheduler.getSchedule().nextAuctionSchedule, undefined);
 });
 
 // if duration = frequency, we'll cut the duration short to fit.