Correct the signature of getUnfinalizedBlocksAtSlots

ChainSafe · Jun 25, 2020 · 5939740 · 5939740
1 parent b0982b0
commit 5939740
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 24 deletions.
diff --git a/packages/lodestar/src/chain/chain.ts b/packages/lodestar/src/chain/chain.ts
@@ -114,16 +114,17 @@ export class BeaconChain extends (EventEmitter as { new(): ChainEventEmitter })
     return this.epochCtx.copy();
   }
 
-  public async getUnfinalizedBlocksAtSlots(slots: Slot[]): Promise<(SignedBeaconBlock|null)[]> {
+  public async getUnfinalizedBlocksAtSlots(slots: Slot[]): Promise<SignedBeaconBlock[]|null> {
     if (!slots) {
-      return [];
+      return null;
     }
     const blockRoots = slots.map((slot) => {
       const summary = this.forkChoice.getBlockSummaryAtSlot(slot);
       return summary? summary.blockRoot : null;
-    });
+    }).filter((blockRoot) => !!blockRoot);
+    // these blocks are on the same chain to head
     return await Promise.all(blockRoots.map(
-      (blockRoot) => blockRoot? this.db.block.get(blockRoot) : Promise.resolve(null)));
+      (blockRoot) => this.db.block.get(blockRoot)));
   }
 
   public async getFinalizedCheckpoint(): Promise<Checkpoint> {

diff --git a/packages/lodestar/src/chain/interface.ts b/packages/lodestar/src/chain/interface.ts
@@ -66,7 +66,7 @@ export interface IBeaconChain extends ChainEventEmitter {
 
   getBlockAtSlot(slot: Slot): Promise<SignedBeaconBlock|null>;
 
-  getUnfinalizedBlocksAtSlots(slots: Slot[]): Promise<(SignedBeaconBlock|null)[]>;
+  getUnfinalizedBlocksAtSlots(slots: Slot[]): Promise<SignedBeaconBlock[]|null>;
 
   /**
    * Add attestation to the fork-choice rule

diff --git a/packages/lodestar/src/sync/reqResp/reqResp.ts b/packages/lodestar/src/sync/reqResp/reqResp.ts
@@ -265,21 +265,15 @@ export class BeaconReqRespHandler implements IReqRespHandler {
     }
     slot = (slot === -1)? request.startSlot : slot + request.step;
     const upperSlot = request.startSlot + request.count * request.step;
-    const headRoot = await chain.forkChoice.head().blockRoot;
     const slots = [];
     while (slot < upperSlot) {
       slots.push(slot);
       slot += request.step;
     }
-    const blocks = await chain.getUnfinalizedBlocksAtSlots(slots);
+    const blocks = await chain.getUnfinalizedBlocksAtSlots(slots) || [];
     for (const block of blocks) {
       if(block) {
-        // make sure block is on the same chain to head
-        const blockRoot = config.types.BeaconBlock.hashTreeRoot(block.message);
-        const ancestorRoot = chain.forkChoice.getAncestor(headRoot, block.message.slot);
-        if (ancestorRoot && config.types.Root.equals(blockRoot, ancestorRoot)) {
-          yield block;
-        }
+        yield block;
       }
     }
   };

diff --git a/packages/lodestar/test/unit/sync/reqRes.test.ts b/packages/lodestar/test/unit/sync/reqRes.test.ts
@@ -189,7 +189,7 @@ describe("sync req resp", function () {
     const peerInfo: PeerInfo = new PeerInfo(new PeerId(Buffer.from("lodestar")));
     const body: BeaconBlocksByRangeRequest = {
       startSlot: 2,
-      count: 5,
+      count: 4,
       step: 2,
     };
     dbStub.blockArchive.valuesStream.returns(async function* () {
@@ -201,14 +201,8 @@ describe("sync req resp", function () {
     }());
     const block8 = generateEmptySignedBlock();
     block8.message.slot = 8;
-    const block10 = generateEmptySignedBlock();
-    block10.message.slot = 10;
     // block 6 does not exist
-    chainStub.getUnfinalizedBlocksAtSlots.resolves([null, block8, block10]);
-    // block8 exists but not on same chain to head
-    forkChoiceStub.getAncestor.onFirstCall().returns(null);
-    // block10 is good
-    forkChoiceStub.getAncestor.onSecondCall().returns(config.types.BeaconBlock.hashTreeRoot(block10.message));
+    chainStub.getUnfinalizedBlocksAtSlots.resolves([null, block8]);
     let blockStream: AsyncIterable<ResponseBody>;
     reqRespStub.sendResponseStream.callsFake((id: RequestId, err: RpcError, chunkIter: AsyncIterable<ResponseBody>) => {
       blockStream = chunkIter;
@@ -218,7 +212,7 @@ describe("sync req resp", function () {
     for await(const body of blockStream) {
       slots.push((body as SignedBeaconBlock).message.slot);
     }
-    // count is 5 but it returns only 3 blocks because block 6 does not exist and block 8 is not on same chain to head
-    expect(slots).to.be.deep.equal([2,4,10]);
+    // count is 4 but it returns only 3 blocks because block 6 does not exist
+    expect(slots).to.be.deep.equal([2,4,8]);
   });
 });
diff --git a/packages/lodestar/test/utils/mocks/chain/chain.ts b/packages/lodestar/test/utils/mocks/chain/chain.ts
@@ -51,7 +51,7 @@ export class MockBeaconChain extends EventEmitter implements IBeaconChain {
     return this.epochCtx.copy();
   }
 
-  public async getUnfinalizedBlocksAtSlots(slots: Slot[]): Promise<(SignedBeaconBlock|null)[]> {
+  public async getUnfinalizedBlocksAtSlots(slots: Slot[]): Promise<SignedBeaconBlock[]|null> {
     if (!slots) {
       return [];
     }