From 635a9ba9f7412402ae9d21ea08be882aa5472ff0 Mon Sep 17 00:00:00 2001
From: Rahul Patni <patnir@users.noreply.github.com>
Date: Fri, 5 Apr 2024 15:34:25 -0400
Subject: [PATCH] Rahul/ifl 2455 refactor explicitly call benchmark spend post
 time (#4872)

* Refactors combine to explicitly call benchmarkSpendPostTime

Also removes `spendPostTimeAt` from internal store. In an upcoming PR this will be replaced with a different variable to create more accurate measurements for spendPostTime.
This refactor is to make the code more readable and to make it easier to understand where `spendPostTime` is being called and how the value is being calculated.

* updating imports

* covering edgecase of calculating a negative time

* Removing temp test variable

* < to <= for calculating spendPostTime

* throwing error when user doesn't have enough notes to benchmark
---
 .../src/commands/wallet/notes/combine.ts      |  8 +++-
 ironfish-cli/src/utils/spendPostTime.ts       | 46 ++++++++-----------
 ironfish/src/fileStores/internal.ts           |  2 -
 3 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/ironfish-cli/src/commands/wallet/notes/combine.ts b/ironfish-cli/src/commands/wallet/notes/combine.ts
index 8925ee88a1..58553231b8 100644
--- a/ironfish-cli/src/commands/wallet/notes/combine.ts
+++ b/ironfish-cli/src/commands/wallet/notes/combine.ts
@@ -18,7 +18,7 @@ import { IronFlag, RemoteFlags } from '../../../flags'
 import { getExplorer } from '../../../utils/explorer'
 import { selectFee } from '../../../utils/fees'
 import { fetchNotes } from '../../../utils/note'
-import { getSpendPostTimeInMs } from '../../../utils/spendPostTime'
+import { benchmarkSpendPostTime, getSpendPostTimeInMs } from '../../../utils/spendPostTime'
 import {
   displayTransactionSummary,
   TransactionTimer,
@@ -232,7 +232,11 @@ export class CombineNotesCommand extends IronfishCommand {
 
     await this.ensureUserHasEnoughNotesToCombine(client, from)
 
-    const spendPostTime = await getSpendPostTimeInMs(this.sdk, client, from, flags.benchmark)
+    let spendPostTime = getSpendPostTimeInMs(this.sdk)
+
+    if (spendPostTime <= 0 || flags.benchmark) {
+      spendPostTime = await benchmarkSpendPostTime(this.sdk, client, from)
+    }
 
     let numberOfNotes = flags.notes
 
diff --git a/ironfish-cli/src/utils/spendPostTime.ts b/ironfish-cli/src/utils/spendPostTime.ts
index 8f9964ac6b..33ebbb8e9f 100644
--- a/ironfish-cli/src/utils/spendPostTime.ts
+++ b/ironfish-cli/src/utils/spendPostTime.ts
@@ -15,33 +15,17 @@ import {
 import { CliUx } from '@oclif/core'
 import { fetchNotes } from './note'
 
-export async function getSpendPostTimeInMs(
+export function getSpendPostTimeInMs(sdk: IronfishSdk): number {
+  return sdk.internal.get('spendPostTime')
+}
+
+export async function benchmarkSpendPostTime(
   sdk: IronfishSdk,
   client: RpcClient,
   account: string,
-  forceBenchmark: boolean,
 ): Promise<number> {
-  let spendPostTime = sdk.internal.get('spendPostTime')
-
-  const spendPostTimeAt = sdk.internal.get('spendPostTimeAt')
-
-  const shouldbenchmark =
-    forceBenchmark ||
-    spendPostTime <= 0 ||
-    Date.now() - spendPostTimeAt > 1000 * 60 * 60 * 24 * 30 // 1 month
-
-  if (shouldbenchmark) {
-    spendPostTime = await benchmarkSpendPostTime(client, account)
-
-    sdk.internal.set('spendPostTime', spendPostTime)
-    sdk.internal.set('spendPostTimeAt', Date.now())
-    await sdk.internal.save()
-  }
-
-  return spendPostTime
-}
+  CliUx.ux.action.start('Measuring time to combine 1 note')
 
-async function benchmarkSpendPostTime(client: RpcClient, account: string): Promise<number> {
   const publicKey = (
     await client.wallet.getAccountPublicKey({
       account: account,
@@ -50,7 +34,10 @@ async function benchmarkSpendPostTime(client: RpcClient, account: string): Promi
 
   const notes = await fetchNotes(client, account, 10)
 
-  CliUx.ux.action.start('Measuring time to combine 1 note')
+  // Not enough notes in the account to measure the time to combine a note
+  if (notes.length < 3) {
+    CliUx.ux.error('Not enough notes.')
+  }
 
   const feeRates = await client.wallet.estimateFeeRates()
 
@@ -97,15 +84,22 @@ async function benchmarkSpendPostTime(client: RpcClient, account: string): Promi
   const resultTxn1 = await Promise.all(promisesTxn1)
   const resultTxn2 = await Promise.all(promisesTxn2)
 
-  const delta = Math.ceil(
+  const spendPostTime = Math.ceil(
     (resultTxn2.reduce((acc, curr) => acc + curr, 0) -
       resultTxn1.reduce((acc, curr) => acc + curr, 0)) /
       3,
   )
 
-  CliUx.ux.action.stop(TimeUtils.renderSpan(delta))
+  if (spendPostTime <= 0) {
+    CliUx.ux.error('Error calculating spendPostTime. Please try again.')
+  }
+
+  CliUx.ux.action.stop(TimeUtils.renderSpan(spendPostTime))
+  sdk.internal.set('spendPostTime', spendPostTime)
 
-  return delta
+  await sdk.internal.save()
+
+  return spendPostTime
 }
 
 async function measureTransactionPostTime(
diff --git a/ironfish/src/fileStores/internal.ts b/ironfish/src/fileStores/internal.ts
index 1ce7251f42..934c2d7734 100644
--- a/ironfish/src/fileStores/internal.ts
+++ b/ironfish/src/fileStores/internal.ts
@@ -12,7 +12,6 @@ export type InternalOptions = {
   rpcAuthToken: string
   networkId: number
   spendPostTime: number // in milliseconds
-  spendPostTimeAt: number // when the spend post time measurement was done
 }
 
 export const InternalOptionsDefaults: InternalOptions = {
@@ -22,7 +21,6 @@ export const InternalOptionsDefaults: InternalOptions = {
   rpcAuthToken: '',
   networkId: DEFAULT_NETWORK_ID,
   spendPostTime: 0,
-  spendPostTimeAt: 0,
 }
 
 export class InternalStore extends KeyStore<InternalOptions> {