Merge pull request #1010 from ethereumjs/type-boundprotocol

client: type BoundProtocol messages

Author: holgerd77

packages/client/lib/net/peer/peer.ts

@@ -1,5 +1,11 @@
 import * as events from 'events'
-import { Protocol, BoundProtocol, Sender } from '../protocol'
+import {
+  Protocol,
+  BoundProtocol,
+  EthProtocolMethods,
+  LesProtocolMethods,
+  Sender,
+} from '../protocol'
 import { Server } from '../server'
 import { Config } from '../../config'
 
@@ -42,8 +48,8 @@ export class Peer extends events.EventEmitter {
   private _idle: boolean
 
   // Dynamically bound protocol properties
-  public les: BoundProtocol | undefined
-  public eth: BoundProtocol | undefined
+  public eth: (BoundProtocol & EthProtocolMethods) | undefined
+  public les: (BoundProtocol & LesProtocolMethods) | undefined
 
   /**
    * Create new peer

packages/client/lib/net/protocol/boundprotocol.ts

@@ -5,12 +5,16 @@ import { Sender } from './sender'
 import { Config } from '../../config'
 
 export interface BoundProtocolOptions {
+  /* Config */
   config: Config
 
+  /* Protocol */
   protocol: Protocol
 
+  /* Peer */
   peer: Peer
 
+  /* Sender */
   sender: Sender
 }
 
@@ -155,19 +159,18 @@ export class BoundProtocol extends EventEmitter {
   }
 
   /**
-   * Add a methods to the bound protocol for each protocol message that has a
-   * corresponding response message
+   * Add methods to the bound protocol for each protocol message that has a
+   * corresponding response message.
    */
   addMethods() {
     const messages = this.protocol.messages.filter((m: any) => m.response)
     for (const message of messages) {
       const name = message.name as string
       const camel = name[0].toLowerCase() + name.slice(1)
-
-      // Problem: How to dynamically add class properties in TS
-      // https://stackoverflow.com/questions/41038812
-      // @ts-ignore: implicit 'any'
-      this[name] = this[camel] = async (args: any[]) => this.request(name, args)
+      ;(this as any)[camel] = async (args: any[]) =>
+        this.request(name, args).catch((error: Error) => {
+          this.emit('error', error)
+        })
     }
   }
 }

packages/client/lib/net/protocol/ethprotocol.ts

@@ -1,5 +1,5 @@
 import { BN, bufferToInt } from 'ethereumjs-util'
-import { BlockHeader, BlockHeaderBuffer } from '@ethereumjs/block'
+import { Block, BlockHeader, BlockHeaderBuffer } from '@ethereumjs/block'
 import { Chain } from './../../blockchain'
 import { Message, Protocol, ProtocolOptions } from './protocol'
 
@@ -8,6 +8,17 @@ interface EthProtocolOptions extends ProtocolOptions {
   chain: Chain
 }
 
+/* Messages with responses that are added as methods in camelCase to BoundProtocol. */
+export interface EthProtocolMethods {
+  getBlockHeaders: (opts: {
+    block: BN | Buffer
+    max: number
+    skip?: number
+    reverse?: number
+  }) => Promise<BlockHeader[]>
+  getBlockBodies: (hashes: Buffer[]) => Promise<Block[]>
+}
+
 const messages: Message[] = [
   {
     name: 'NewBlockHashes',

packages/client/lib/net/protocol/flowcontrol.ts

@@ -1,5 +1,4 @@
 import { Peer } from '../peer/peer'
-import { BoundProtocol } from './boundprotocol'
 
 interface Mrc {
   [key: string]: {
@@ -62,10 +61,10 @@ export class FlowControl {
    */
   maxRequestCount(peer: Peer, messageName: string): number {
     const now = Date.now()
-    const mrcBase = (peer.les as BoundProtocol).status.mrc[messageName].base
-    const mrcReq = (peer.les as BoundProtocol).status.mrc[messageName].req
-    const mrr = (peer.les as BoundProtocol).status.mrr
-    const bl = (peer.les as BoundProtocol).status.bl
+    const mrcBase = peer.les!.status.mrc[messageName].base
+    const mrcReq = peer.les!.status.mrc[messageName].req
+    const mrr = peer.les!.status.mrr
+    const bl = peer.les!.status.bl
     const params = this.in.get(peer.id) ?? ({ ble: bl } as FlowParams)
     if (params.last) {
       // recharge BLE at rate of MRR when less than BL

packages/client/lib/net/protocol/lesprotocol.ts

@@ -12,6 +12,17 @@ export interface LesProtocolOptions extends ProtocolOptions {
   flow?: FlowControl
 }
 
+/* Messages with responses that are added as methods in camelCase to BoundProtocol. */
+export interface LesProtocolMethods {
+  getBlockHeaders: (opts: {
+    reqId?: BN
+    block: BN | Buffer
+    max: number
+    skip?: number
+    reverse?: number
+  }) => Promise<{ reqId: BN; bv: BN; headers: BlockHeader[] }>
+}
+
 const id = new BN(0)
 
 const messages: Message[] = [

packages/client/lib/service/fullethereumservice.ts

@@ -3,7 +3,7 @@ import { FullSynchronizer } from '../sync/fullsync'
 import { EthProtocol } from '../net/protocol/ethprotocol'
 import { LesProtocol } from '../net/protocol/lesprotocol'
 import { Peer } from '../net/peer/peer'
-import { Protocol, BoundProtocol } from '../net/protocol'
+import { Protocol } from '../net/protocol'
 
 interface FullEthereumServiceOptions extends EthereumServiceOptions {
   /* Serve LES requests (default: false) */
@@ -85,12 +85,12 @@ export class FullEthereumService extends EthereumService {
     if (message.name === 'GetBlockHeaders') {
       const { block, max, skip, reverse } = message.data
       const headers: any = await this.chain.getHeaders(block, max, skip, reverse)
-      ;(peer.eth as BoundProtocol).send('BlockHeaders', headers)
+      peer.eth!.send('BlockHeaders', headers)
     } else if (message.name === 'GetBlockBodies') {
       const hashes = message.data
       const blocks = await Promise.all(hashes.map((hash: any) => this.chain.getBlock(hash)))
       const bodies: any = blocks.map((block: any) => block.raw().slice(1))
-      ;(peer.eth as BoundProtocol).send('BlockBodies', bodies)
+      peer.eth!.send('BlockBodies', bodies)
     } else if (message.name === 'NewBlockHashes') {
       await this.synchronizer.announced(message.data, peer)
     }
@@ -110,7 +110,7 @@ export class FullEthereumService extends EthereumService {
         this.config.logger.debug(`Dropping peer for violating flow control ${peer}`)
       } else {
         const headers: any = await this.chain.getHeaders(block, max, skip, reverse)
-        ;(peer.les as BoundProtocol).send('BlockHeaders', { reqId, bv, headers })
+        peer.les!.send('BlockHeaders', { reqId, bv, headers })
       }
     }
   }

packages/client/lib/sync/fullsync.ts

@@ -1,7 +1,6 @@
 import { BN } from 'ethereumjs-util'
 import VM from '@ethereumjs/vm'
 import { Peer } from '../net/peer/peer'
-import { BoundProtocol } from '../net/protocol'
 import { short } from '../util'
 import { Synchronizer, SynchronizerOptions } from './sync'
 import { BlockFetcher } from './fetcher'
@@ -43,8 +42,6 @@ export class FullSynchronizer extends Synchronizer {
    * Returns true if peer can be used for syncing
    * @return {boolean}
    */
-  // TODO: Correct logic for return type (b/c peer.eth is not boolean)
-  // "Type 'BoundProtocol | undefined' is not assignable to type 'boolean'"
   syncable(peer: Peer): boolean {
     return peer.eth !== undefined
   }
@@ -58,7 +55,7 @@ export class FullSynchronizer extends Synchronizer {
     const peers = this.pool.peers.filter(this.syncable.bind(this))
     if (peers.length < this.config.minPeers && !this.forceSync) return
     for (const peer of peers) {
-      if (peer.eth && peer.eth.status) {
+      if (peer.eth?.status) {
         const td = peer.eth.status.td
         if (
           (!best && td.gte((this.chain.blocks as any).td)) ||
@@ -76,12 +73,11 @@ export class FullSynchronizer extends Synchronizer {
    * @return {Promise} Resolves with header
    */
   async latest(peer: Peer) {
-    // @ts-ignore
-    const headers = await (peer.eth as BoundProtocol).getBlockHeaders({
-      block: (peer.eth as BoundProtocol).status.bestHash,
+    const headers = await peer.eth?.getBlockHeaders({
+      block: peer.eth!.status.bestHash,
       max: 1,
     })
-    return headers[0]
+    return headers?.[0]
   }
 
   /**
@@ -92,8 +88,9 @@ export class FullSynchronizer extends Synchronizer {
   async syncWithPeer(peer?: Peer): Promise<boolean> {
     if (!peer) return false
     const latest = await this.latest(peer)
+    if (!latest) return false
     const height = new BN(latest.number)
-    const first = ((this.chain.blocks as any).height as BN).addn(1)
+    const first = this.chain.blocks.height.addn(1)
     const count = height.sub(first).addn(1)
     if (count.lten(0)) return false
 

packages/client/lib/sync/lightsync.ts

@@ -1,5 +1,4 @@
 import { Peer } from '../net/peer/peer'
-import { BoundProtocol } from '../net/protocol'
 import { Synchronizer, SynchronizerOptions } from './sync'
 import { HeaderFetcher } from './fetcher/headerfetcher'
 import { BN } from 'ethereumjs-util'
@@ -30,7 +29,7 @@ export class LightSynchronizer extends Synchronizer {
    * @return {boolean}
    */
   syncable(peer: Peer): boolean {
-    return peer.les && peer.les.status.serveHeaders
+    return peer.les?.status.serveHeaders
   }
 
   /**
@@ -62,7 +61,7 @@ export class LightSynchronizer extends Synchronizer {
    */
   async syncWithPeer(peer?: Peer): Promise<boolean> {
     if (!peer) return false
-    const height = new BN((peer.les as BoundProtocol).status.headNum)
+    const height = new BN(peer.les!.status.headNum)
     const first = ((this.chain.headers as any).height as BN).addn(1)
     const count = height.sub(first).addn(1)
     if (count.lten(0)) return false

packages/client/test/integration/fullethereumservice.spec.ts

@@ -26,15 +26,15 @@ tape('[Integration:FullEthereumService]', async (t) => {
   t.test('should handle ETH requests', async (t) => {
     const [server, service] = await setup()
     const peer = await server.accept('peer0')
-    const headers = await (peer.eth as any).getBlockHeaders({ block: 1, max: 2 })
+    const headers = await peer.eth!.getBlockHeaders({ block: new BN(1), max: 2 })
     const hash = Buffer.from(
       'a321d27cd2743617c1c1b0d7ecb607dd14febcdfca8f01b79c3f0249505ea069',
       'hex'
     )
     t.ok(headers[1].hash().equals(hash), 'handled GetBlockHeaders')
-    const bodies = await (peer.eth as any).getBlockBodies([hash])
+    const bodies = await peer.eth!.getBlockBodies([hash])
     t.deepEquals(bodies, [[[], []]], 'handled GetBlockBodies')
-    await (peer.eth as any).send('NewBlockHashes', [[hash, new BN(2)]])
+    await peer.eth!.send('NewBlockHashes', [[hash, new BN(2)]])
     t.pass('handled NewBlockHashes')
     await destroy(server, service)
     t.end()
@@ -43,7 +43,7 @@ tape('[Integration:FullEthereumService]', async (t) => {
   t.test('should handle LES requests', async (t) => {
     const [server, service] = await setup()
     const peer = await server.accept('peer0')
-    const { headers } = await (peer.les as any).getBlockHeaders({ block: 1, max: 2 })
+    const { headers } = await peer.les!.getBlockHeaders({ block: new BN(1), max: 2 })
     t.equals(
       headers[1].hash().toString('hex'),
       'a321d27cd2743617c1c1b0d7ecb607dd14febcdfca8f01b79c3f0249505ea069',

packages/client/test/net/protocol/boundprotocol.spec.ts

@@ -34,7 +34,6 @@ tape('[BoundProtocol]', (t) => {
       sender,
     })
     t.ok(/this.request/.test((bound as any).testMessage.toString()), 'added testMessage')
-    t.ok(/this.request/.test((bound as any).TestMessage.toString()), 'added TestMessage')
     t.end()
   })