From 746c2a402c9d583a82a8a3e0d7b99899a1c93b31 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Tue, 23 Apr 2024 11:28:30 +0200 Subject: [PATCH] Ensure all estimates below feeMinimum are excluded --- src/lib/DataProviderManager.ts | 54 +++++---- test/DataProviderManager-merge.test.ts | 105 ++++++++++++++++++ ...st.ts => DataProviderManager-sort.test.ts} | 53 ++------- 3 files changed, 148 insertions(+), 64 deletions(-) create mode 100644 test/DataProviderManager-merge.test.ts rename test/{DataProviderManager.test.ts => DataProviderManager-sort.test.ts} (77%) diff --git a/src/lib/DataProviderManager.ts b/src/lib/DataProviderManager.ts index 7a6a53e..861371d 100644 --- a/src/lib/DataProviderManager.ts +++ b/src/lib/DataProviderManager.ts @@ -52,17 +52,11 @@ export class DataProviderManager { const blockHash = dataPoints[0].blockHash; const feeEstimates = this.mergeFeeEstimates(dataPoints); - // Apply the fee minimum and multiplier. + // Apply the fee formatter and multiplier. for (let [blockTarget, estimate] of Object.entries(feeEstimates)) { - if (estimate >= this.feeMinimum) { - feeEstimates[blockTarget] = Math.ceil( - (estimate *= 1000 * this.feeMultiplier), - ); - } else { - log.warn({ - msg: `Fee estimate for target ${blockTarget} was below the minimum of ${this.feeMinimum}.`, - }); - } + feeEstimates[blockTarget] = Math.ceil( + (estimate *= 1000 * this.feeMultiplier), + ); } data = { @@ -163,6 +157,28 @@ export class DataProviderManager { }); } + /** + * Filters fee estimates based on the fee minimum. + * + * @param feeEstimates - An object containing fee estimates. + * @returns An object containing the filtered fee estimates. + */ + private filterEstimates(feeEstimates: FeeByBlockTarget): FeeByBlockTarget { + return Object.fromEntries( + Object.entries(feeEstimates).filter( + ([blockTarget, estimate]) => { + if (estimate < this.feeMinimum) { + log.warn({ + msg: `Fee estimate for target ${blockTarget} was below the minimum of ${this.feeMinimum}.`, + }); + return false; + } + return true; + } + ) + ); + } + /** * Merges fee estimates from multiple data points. * @@ -170,13 +186,12 @@ export class DataProviderManager { * @returns An object containing the merged fee estimates. */ private mergeFeeEstimates(dataPoints: DataPoint[]): FeeByBlockTarget { - // Start with the fee estimates from the most relevant provider - let mergedEstimates = { ...dataPoints[0].feeEstimates }; - log.debug({ msg: "Initial mergedEstimates:", mergedEstimates }); - // Iterate over the remaining data points - for (let i = 1; i < dataPoints.length; i++) { - const estimates = dataPoints[i].feeEstimates; - const providerName = dataPoints[i].provider.constructor.name; + let mergedEstimates: FeeByBlockTarget = {}; + + // Iterate over all data points + for (const dataPoint of dataPoints) { + const estimates = this.filterEstimates(dataPoint.feeEstimates); + const providerName = dataPoint.provider.constructor.name; const keys = Object.keys(estimates) .map(Number) .sort((a, b) => a - b); @@ -185,8 +200,9 @@ export class DataProviderManager { keys.forEach((key) => { // Only add the estimate if it has a higher confirmation target and a lower fee if ( - key > Math.max(...Object.keys(mergedEstimates).map(Number)) && - estimates[key] < Math.min(...Object.values(mergedEstimates)) + (!mergedEstimates[key] && estimates[key]) || + (mergedEstimates[key] && key > Math.max(...Object.keys(mergedEstimates).map(Number)) && + estimates[key] < Math.min(...Object.values(mergedEstimates))) ) { log.debug({ msg: `Adding estimate from ${providerName} with target ${key} and fee ${estimates[key]} to mergedEstimates`, diff --git a/test/DataProviderManager-merge.test.ts b/test/DataProviderManager-merge.test.ts new file mode 100644 index 0000000..88050d2 --- /dev/null +++ b/test/DataProviderManager-merge.test.ts @@ -0,0 +1,105 @@ +import { expect, test } from "bun:test"; +import { DataProviderManager } from "../src/lib/DataProviderManager"; + +class MockProvider0 implements Provider { + getBlockHeight = () => Promise.resolve(1001); + getBlockHash = () => Promise.resolve("hash1001"); + getFeeEstimates = () => + Promise.resolve({ + "1": 1, + "2": 1, + "3": 1, + }); + getAllData = () => + Promise.resolve({ + blockHeight: 1001, + blockHash: "hash1001", + feeEstimates: { + "1": 1, + "2": 1, + "3": 1, + }, + }); +} + +class MockProvider1 implements Provider { + getBlockHeight = () => Promise.resolve(998); + getBlockHash = () => Promise.resolve("hash998"); + getFeeEstimates = () => + Promise.resolve({ + "1": 20, + "10": 1, + }); + getAllData = () => + Promise.resolve({ + blockHeight: 998, + blockHash: "hash998", + feeEstimates: { + "1": 20, + "10": 1, + }, + }); +} + +class MockProvider2 implements Provider { + getBlockHeight = () => Promise.resolve(1000); + getBlockHash = () => Promise.resolve("hash1000"); + getFeeEstimates = () => + Promise.resolve({ + "1": 30, + "2": 20, + }); + getAllData = () => + Promise.resolve({ + blockHeight: 1000, + blockHash: "hash1000", + feeEstimates: { + "1": 30, + "2": 20, + }, + }); +} + +class MockProvider3 implements Provider { + getBlockHeight = () => Promise.resolve(999); + getBlockHash = () => Promise.resolve("hash999"); + getFeeEstimates = () => + Promise.resolve({ + "1": 25, + "2": 15, + "3": 5, + "5": 3, + }); + getAllData = () => + Promise.resolve({ + blockHeight: 999, + blockHash: "hash999", + feeEstimates: { + "1": 25, + "2": 15, + "3": 5, + "5": 3, + }, + }); +} + +const maxHeightDelta = 2; +const feeMultiplier = 2; +const feeMinimum = 2; +const manager = new DataProviderManager({ stdTTL: 0, checkperiod: 0 }, maxHeightDelta, feeMultiplier, feeMinimum); +manager.registerProvider(new MockProvider0()); +manager.registerProvider(new MockProvider1()); +manager.registerProvider(new MockProvider2()); +manager.registerProvider(new MockProvider3()); + +test("should merge fee estimates from multiple providers correctly", async () => { + const mergedData = await manager.getData(); + expect(mergedData.current_block_height).toEqual(1001); + expect(mergedData.current_block_hash).toEqual("hash1001"); + expect(mergedData.fee_by_block_target).toEqual({ + "1": 60000, + "2": 40000, + "3": 10000, + "5": 6000, + }); +}); diff --git a/test/DataProviderManager.test.ts b/test/DataProviderManager-sort.test.ts similarity index 77% rename from test/DataProviderManager.test.ts rename to test/DataProviderManager-sort.test.ts index 6c834fc..aaeae37 100644 --- a/test/DataProviderManager.test.ts +++ b/test/DataProviderManager-sort.test.ts @@ -6,62 +6,37 @@ import { EsploraProvider } from "../src/providers/esplora"; class MockProvider1 implements Provider { getBlockHeight = () => Promise.resolve(998); getBlockHash = () => Promise.resolve("hash1"); - getFeeEstimates = () => - Promise.resolve({ - "1": 20, - "10": 1, - }); + getFeeEstimates = () => Promise.resolve({}); getAllData = () => Promise.resolve({ blockHeight: 998, blockHash: "hash1", - feeEstimates: { - "1": 20, - "10": 1, - }, + feeEstimates: {}, }); } class MockProvider2 implements Provider { getBlockHeight = () => Promise.resolve(1000); getBlockHash = () => Promise.resolve("hash3"); - getFeeEstimates = () => - Promise.resolve({ - "1": 30, - "2": 20, - }); + getFeeEstimates = () => Promise.resolve({}); getAllData = () => Promise.resolve({ blockHeight: 1000, blockHash: "hash3", - feeEstimates: { - "1": 30, - "2": 20, - }, + feeEstimates: {}, }); } class MockProvider3 implements Provider { getBlockHeight = () => Promise.resolve(999); getBlockHash = () => Promise.resolve("hash2"); - getFeeEstimates = () => - Promise.resolve({ - "1": 25, - "2": 15, - "3": 5, - "5": 3, - }); + getFeeEstimates = () => Promise.resolve({}); getAllData = () => - Promise.resolve({ + Promise.resolve(({ blockHeight: 999, blockHash: "hash2", - feeEstimates: { - "1": 25, - "2": 15, - "3": 5, - "5": 3, - }, - }); + feeEstimates: {}, + })); } const manager = new DataProviderManager({ stdTTL: 0, checkperiod: 0 }); @@ -69,18 +44,6 @@ manager.registerProvider(new MockProvider1()); manager.registerProvider(new MockProvider2()); manager.registerProvider(new MockProvider3()); -test("should merge fee estimates from multiple providers correctly", async () => { - const mergedData = await manager.getData(); - expect(mergedData.current_block_height).toEqual(1000); - expect(mergedData.current_block_hash).toEqual("hash3"); - expect(mergedData.fee_by_block_target).toEqual({ - "1": 30000, - "2": 20000, - "3": 5000, - "5": 3000, - }); -}); - test("sorts providers correctly", async () => { const mempoolProvider = new MempoolProvider("https://mempool.space", 6); const esploraProvider = new EsploraProvider("https://blockstream.info", 6);