From 5af558ca3a0bd326f831b1364e3080760309f33c Mon Sep 17 00:00:00 2001 From: Agustin Aguilar Date: Tue, 27 Jun 2023 17:00:50 +0000 Subject: [PATCH 1/3] Skip config check if both results are equal --- packages/sessions/src/trackers/cached.ts | 76 ++++++++++++------------ 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/packages/sessions/src/trackers/cached.ts b/packages/sessions/src/trackers/cached.ts index 0c74505b4..9ef519a70 100644 --- a/packages/sessions/src/trackers/cached.ts +++ b/packages/sessions/src/trackers/cached.ts @@ -12,53 +12,51 @@ export class CachedTracker implements migrator.PresignedMigrationTracker, Config ) {} async loadPresignedConfiguration(args: { wallet: string; fromImageHash: string; longestPath?: boolean | undefined }): Promise { - const configs = new Map>() - const configOf = (imageHash: string): Promise => { - if (!configs.has(imageHash)) { - configs.set(imageHash, this.configOfImageHash({ imageHash })) - } - return configs.get(imageHash)! - } - // We need to check both, and return the one with the highest checkpoint // eventually we could try to combine them, but for now we'll just return // the one with the highest checkpoint const results = [this.tracker.loadPresignedConfiguration(args), this.cache.loadPresignedConfiguration(args)] - const checkpoints = await Promise.all(results.map(async result => { - const r = await result - const last = r[r.length - 1] - if (!last) return undefined - - // TODO: This will fire a lot of requests, optimize it - const config = await configOf(last.nextImageHash) - if (!config) return undefined - return { checkpoint: universal.genericCoderFor(config.version).config.checkpointOf(config), result: r } - })) - - const best = checkpoints.reduce((acc, val) => { - if (!val) return acc - if (!acc) return val - if (val.checkpoint.gt(acc.checkpoint)) return val - return acc - }) + let best: PresignedConfigLink[] + + // If both results end with the same image hash, we can just return the longest/shortest one + const [result1, result2] = await Promise.all(results) + if ( + result1.length > 0 && + result2.length > 0 && + result1[result1.length - 1].nextImageHash === result2[result2.length - 1].nextImageHash + ) { + best = ( + args.longestPath === true ? + result1.length > result2.length ? result1 : result2 : + result1.length < result2.length ? result1 : result2 + ) + } else { + // Otherwise we need to check the checkpoints + // this requires us to fetch the config for each image hash + const checkpoints = await Promise.all(results.map(async result => { + const r = await result + const last = r[r.length - 1] + if (!last) return undefined + + // TODO: This will fire a lot of requests, optimize it + const config = await this.configOfImageHash({ imageHash: last.nextImageHash }) + if (!config) return undefined + + return { checkpoint: universal.genericCoderFor(config.version).config.checkpointOf(config), result: r } + })) + + best = checkpoints.reduce((acc, val) => { + if (!val) return acc + if (!acc) return val + if (val.checkpoint.gt(acc.checkpoint)) return val + return acc + })?.result ?? [] + } if (!best) return [] - ;(async () => { - for (const result of best.result) { - const nextConfig = await configOf(result.nextImageHash) - if (nextConfig) { - this.cache.savePresignedConfiguration({ - wallet: args.wallet, - nextConfig, - signature: result.signature - }) - } - } - })() - - return best.result + return best } async savePresignedConfiguration(args: PresignedConfig): Promise { From e521953d853f199e42d7d664da3ea1290f6185ea Mon Sep 17 00:00:00 2001 From: Agustin Aguilar Date: Tue, 27 Jun 2023 17:01:11 +0000 Subject: [PATCH 2/3] Improve happy paths and add memory cache --- packages/sessions/src/trackers/local.ts | 72 ++++++++++++++++++++----- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/packages/sessions/src/trackers/local.ts b/packages/sessions/src/trackers/local.ts index 2787993f7..a4488ee20 100644 --- a/packages/sessions/src/trackers/local.ts +++ b/packages/sessions/src/trackers/local.ts @@ -128,25 +128,36 @@ export class LocalConfigTracker implements ConfigTracker, migrator.PresignedMigr return } + private configOfImageHashCache = {} as { [key: string]: commons.config.Config } + configOfImageHash = async (args: { imageHash: string }): Promise => { const { imageHash } = args + if (this.configOfImageHashCache[args.imageHash]) { + return this.configOfImageHashCache[args.imageHash] + } + const config = await this.store.loadConfig(imageHash) - if (!config) return undefined + if (!config) { + return undefined + } if (config.version === 1 || (config.version === 2 && !isPlainV2Config(config))) { + this.configOfImageHashCache[args.imageHash] = config return config } if (isPlainV2Config(config)) { - return { + const fullConfig = { version: 2, threshold: ethers.BigNumber.from(config.threshold), checkpoint: ethers.BigNumber.from(config.checkpoint), tree: await this.loadTopology(config.tree) } as v2.config.WalletConfig + this.configOfImageHashCache[args.imageHash] = fullConfig + return fullConfig } throw new Error(`Unknown config type: ${config}`) @@ -193,11 +204,23 @@ export class LocalConfigTracker implements ConfigTracker, migrator.PresignedMigr await this.store.savePayloadOfSubdigest(subdigest, payload) } + private payloadOfSubdigestCache = {} as { [key: string]: commons.signature.SignedPayload } + payloadOfSubdigest = async (args: { subdigest: string }): Promise => { + if (this.payloadOfSubdigestCache[args.subdigest]) { + return this.payloadOfSubdigestCache[args.subdigest] + } + const { subdigest } = args - return this.store.loadPayloadOfSubdigest(subdigest) + const res = await this.store.loadPayloadOfSubdigest(subdigest) + + if (res) { + this.payloadOfSubdigestCache[subdigest] = res + } + + return res } savePresignedConfiguration = async (args: PresignedConfig): Promise => { @@ -264,14 +287,34 @@ export class LocalConfigTracker implements ConfigTracker, migrator.PresignedMigr signature: string, } | undefined - for (const { payload, nextImageHash } of nextImageHashes) { - // Get config of next imageHash + const nextConfigsAndCheckpoints = await Promise.all(nextImageHashes.map(async ({ nextImageHash, payload }) => { const nextConfig = await this.configOfImageHash({ imageHash: nextImageHash }) - if (!nextConfig || !v2.config.isWalletConfig(nextConfig)) continue + if (!nextConfig || !v2.config.isWalletConfig(nextConfig)) return undefined const nextCheckpoint = ethers.BigNumber.from(nextConfig.checkpoint) - - // Only consider candidates later than the starting checkpoint - if (nextCheckpoint.lte(fromConfig.checkpoint)) continue + return { nextConfig, nextCheckpoint, nextImageHash, payload } + })) + + const sortedNextConfigsAndCheckpoints = nextConfigsAndCheckpoints + .filter((c) => c !== undefined) + .filter((c) => c!.nextCheckpoint.gt(fromConfig.checkpoint)) + .sort((a, b) => ( + // If we are looking for the longest path, sort by ascending checkpoint + // because we want to find the smalles jump, and we should start with the + // closest one. If we are not looking for the longest path, sort by + // descending checkpoint, because we want to find the largest jump. + // + // We don't have a guarantee that all "next configs" will be valid + // so worst case scenario we will need to try all of them. + // But we can try to optimize for the most common case. + a!.nextCheckpoint.gt(b!.nextCheckpoint) ? ( + longestPath ? 1 : -1 + ) : ( + longestPath ? -1 : 1 + ) + )) + + for (const entry of sortedNextConfigsAndCheckpoints) { + const { nextConfig, nextCheckpoint, nextImageHash, payload } = entry! if (bestCandidate) { const bestCheckpoint = bestCandidate.checkpoint @@ -306,7 +349,10 @@ export class LocalConfigTracker implements ConfigTracker, migrator.PresignedMigr ).filter((signature): signature is [string, commons.signature.SignaturePart] => Boolean(signature[1])) ) - // Encode the full signature + // Skip if we don't have ANY signatures (it can never reach the threshold) + if (Object.keys(signatures).length === 0) continue + + // Encode the full signature (to see if it has enough weight) const encoded = v2.signature.SignatureCoder.encodeSigners(fromConfig, signatures, [], 0) if (encoded.weight.lt(fromConfig.threshold)) continue @@ -317,8 +363,10 @@ export class LocalConfigTracker implements ConfigTracker, migrator.PresignedMigr signature: encoded.encoded } } - - if (!bestCandidate) return [] + + if (!bestCandidate) { + return [] + } // Get the next step const nextStep = await this.loadPresignedConfiguration({ From 074d7524bff33d5f98a7304ef6dfae8e9ebe0e98 Mon Sep 17 00:00:00 2001 From: Agustin Aguilar Date: Tue, 27 Jun 2023 17:39:07 +0000 Subject: [PATCH 3/3] Fix skip signature without signers --- packages/sessions/src/trackers/local.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sessions/src/trackers/local.ts b/packages/sessions/src/trackers/local.ts index a4488ee20..24ce9d50a 100644 --- a/packages/sessions/src/trackers/local.ts +++ b/packages/sessions/src/trackers/local.ts @@ -350,7 +350,7 @@ export class LocalConfigTracker implements ConfigTracker, migrator.PresignedMigr ) // Skip if we don't have ANY signatures (it can never reach the threshold) - if (Object.keys(signatures).length === 0) continue + if (signatures.size === 0) continue // Encode the full signature (to see if it has enough weight) const encoded = v2.signature.SignatureCoder.encodeSigners(fromConfig, signatures, [], 0)