Merge pull request #54 from LN-Zap/fix/fee-min-filter

Ensure all estimates below feeMinimum are excluded

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,20 +157,41 @@ 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.
    *
    * @param dataPoints - An array of data points from which to merge fee estimates.
    * @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`,

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,
+  });
+});

test/DataProviderManager.test.ts → test/DataProviderManager-sort.test.ts

@@ -6,81 +6,44 @@ 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 });
 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);