A bit of consistency clean-up in accumulator code (#1201)

fluffy/network/history/accumulator.nim

@@ -16,11 +16,8 @@ import
 
 export ssz_serialization, merkleization, proofs
 
-# Header Accumulator
+# Header Accumulator, as per specification:
-# Part from specification
+# https://github.com/ethereum/portal-network-specs/blob/master/history-network.md#the-header-accumulator
-# https://github.com/ethereum/portal-network-specs/blob/master/header-gossip-network.md#accumulator-snapshot
-# However, applied for the history network instead of the header gossip network
-# as per https://github.com/ethereum/portal-network-specs/issues/153
 
 const
   epochSize* = 8192 # blocks
@@ -51,10 +48,10 @@ type
     of UnknownBlockNumber:
       discard
 
-proc newEmptyAccumulator*(): Accumulator =
+func init*(T: type Accumulator): T =
-  return Accumulator(
+  Accumulator(
     historicalEpochs:  List[Bytes32, maxHistoricalEpochs].init(@[]),
-    currentEpoch: List[HeaderRecord, epochSize].init(@[])
+    currentEpoch: EpochAccumulator.init(@[])
   )
 
 func updateAccumulator*(a: var Accumulator, header: BlockHeader) =
@@ -111,31 +108,31 @@ func buildAccumulatorData*(headers: seq[BlockHeader]):
 ## Calls and helper calls for building header proofs and verifying headers
 ## against the Accumulator and the header proofs.
 
-func inCurrentEpoch*(bn: uint64, a: Accumulator): bool =
+func inCurrentEpoch*(blockNumber: uint64, a: Accumulator): bool =
-  bn > uint64(a.historicalEpochs.len() * epochSize) - 1
+  blockNumber > uint64(a.historicalEpochs.len() * epochSize) - 1
 
 func inCurrentEpoch*(header: BlockHeader, a: Accumulator): bool =
   let blockNumber = header.blockNumber.truncate(uint64)
-  return inCurrentEpoch(blockNumber, a)
+  blockNumber.inCurrentEpoch(a)
 
-func getEpochIndex*(bn: uint64): uint64 =
+func getEpochIndex*(blockNumber: uint64): uint64 =
-  bn div epochSize
+  blockNumber div epochSize
 
 func getEpochIndex*(header: BlockHeader): uint64 =
   let blockNumber = header.blockNumber.truncate(uint64)
   ## Get the index for the historical epochs
-  return getEpochIndex(blockNumber)
+  getEpochIndex(blockNumber)
 
-func getHeaderRecordIndex(bn: uint64, epochIndex: uint64): uint64 =
+func getHeaderRecordIndex(blockNumber: uint64, epochIndex: uint64): uint64 =
   ## Get the relative header index for the epoch accumulator
-  uint64(bn - epochIndex * epochSize)
+  uint64(blockNumber - epochIndex * epochSize)
 
 func getHeaderRecordIndex*(header: BlockHeader, epochIndex: uint64): uint64 =
   ## Get the relative header index for the epoch accumulator
-  return getHeaderRecordIndex(header.blockNumber.truncate(uint64), epochIndex)
+  getHeaderRecordIndex(header.blockNumber.truncate(uint64), epochIndex)
 
 func verifyProof*(
-    a: Accumulator, proof: openArray[Digest], header: BlockHeader): bool =
+    a: Accumulator, header: BlockHeader, proof: openArray[Digest]): bool =
   let
     epochIndex = getEpochIndex(header)
     epochAccumulatorHash = Digest(data: a.historicalEpochs[epochIndex])
@@ -148,7 +145,7 @@ func verifyProof*(
 
   verify_merkle_multiproof(@[leave], proof, @[gIndex], epochAccumulatorHash)
 
-proc verifyHeader*(
+func verifyHeader*(
     accumulator: Accumulator, header: BlockHeader, proof: Option[seq[Digest]]):
     Result[void, string] =
   if header.inCurrentEpoch(accumulator):
@@ -164,7 +161,7 @@ proc verifyHeader*(
       err("Header not part of canonical chain")
   else:
     if proof.isSome():
-      if accumulator.verifyProof(proof.get, header):
+      if accumulator.verifyProof(header, proof.get):
         ok()
       else:
         err("Proof verification failed")
@@ -173,7 +170,7 @@ proc verifyHeader*(
 
 func getHeaderHashForBlockNumber*(a: Accumulator, bn: UInt256): BlockHashResult=
   let blockNumber = bn.truncate(uint64)
-  if inCurrentEpoch(blockNumber, a):
+  if blockNumber.inCurrentEpoch(a):
     let relIndex = blockNumber - uint64(a.historicalEpochs.len()) * epochSize
 
     if relIndex > uint64(a.currentEpoch.len() - 1):

fluffy/tests/test_accumulator.nim

@@ -15,7 +15,7 @@ import
   ../populate_db,
   ../network/history/[history_content, accumulator]
 
-proc buildProof(
+func buildProof(
     accumulator: Accumulator,
     epochAccumulators: seq[(ContentKey, EpochAccumulator)],
     header: BlockHeader):
@@ -118,11 +118,12 @@ suite "Header Accumulator":
         check verifyHeader(accumulator, header, none(seq[Digest])).isErr()
 
   test "Header Accumulator header hash for blocknumber":
-    var acc = newEmptyAccumulator()
+    var acc = Accumulator.init()
 
-    let numEpochs = 2
+    let
-    let numHeadersInCurrentEpoch = 5
+      numEpochs = 2
-    let numHeaders = numEpochs * epochSize + numHeadersInCurrentEpoch
+      numHeadersInCurrentEpoch = 5
+      numHeaders = numEpochs * epochSize + numHeadersInCurrentEpoch
 
     var headerHashes: seq[Hash256] = @[]
 
@@ -168,5 +169,3 @@ suite "Header Accumulator":
       let res1 = acc.getHeaderHashForBlockNumber(u256(3 * epochSize))
       check:
         res1.kind == UnknownBlockNumber
-
-