Skip to content

Commit

Permalink
Rahul/ifl 2455 refactor explicitly call benchmark spend post time (#4872
Browse files Browse the repository at this point in the history
)

* 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
  • Loading branch information
patnir authored Apr 5, 2024
1 parent ac395df commit 635a9ba
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 30 deletions.
8 changes: 6 additions & 2 deletions ironfish-cli/src/commands/wallet/notes/combine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down
46 changes: 20 additions & 26 deletions ironfish-cli/src/utils/spendPostTime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()

Expand Down Expand Up @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions ironfish/src/fileStores/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -22,7 +21,6 @@ export const InternalOptionsDefaults: InternalOptions = {
rpcAuthToken: '',
networkId: DEFAULT_NETWORK_ID,
spendPostTime: 0,
spendPostTimeAt: 0,
}

export class InternalStore extends KeyStore<InternalOptions> {
Expand Down

0 comments on commit 635a9ba

Please sign in to comment.