diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala index 2d1524805a..ffe77b7297 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala @@ -21,8 +21,12 @@ class CheckpointingService( def getLatestBlock(req: GetLatestBlockRequest): ServiceResponse[GetLatestBlockResponse] = { lazy val bestBlockNum = blockchain.getBestBlockNumber() - lazy val blockToReturnNum = bestBlockNum - bestBlockNum % req.checkpointingInterval - lazy val isValidParent = req.parentCheckpoint.forall(blockchain.getBlockHeaderByHash(_).isDefined) + lazy val blockToReturnNum = + if (req.checkpointingInterval != 0) + bestBlockNum - bestBlockNum % req.checkpointingInterval + else bestBlockNum + lazy val isValidParent = + req.parentCheckpoint.forall(blockchain.getBlockHeaderByHash(_).exists(_.number < blockToReturnNum)) Task { blockchain.getBlockByNumber(blockToReturnNum) @@ -31,7 +35,7 @@ class CheckpointingService( Task.now(Right(GetLatestBlockResponse(Some(BlockInfo(b.hash, b.number))))) case Some(_) => - log.debug("Parent checkpoint is not found in a local blockchain") + log.debug("No checkpoint candidate found for a specified parent") Task.now(Right(GetLatestBlockResponse(None))) case None => @@ -59,10 +63,10 @@ class CheckpointingService( } object CheckpointingService { - case class GetLatestBlockRequest(checkpointingInterval: Int, parentCheckpoint: Option[ByteString]) - case class GetLatestBlockResponse(block: Option[BlockInfo]) - case class BlockInfo(hash: ByteString, number: BigInt) + final case class GetLatestBlockRequest(checkpointingInterval: Int, parentCheckpoint: Option[ByteString]) + final case class GetLatestBlockResponse(block: Option[BlockInfo]) + final case class BlockInfo(hash: ByteString, number: BigInt) - case class PushCheckpointRequest(hash: ByteString, signatures: List[ECDSASignature]) - case class PushCheckpointResponse() + final case class PushCheckpointRequest(hash: ByteString, signatures: List[ECDSASignature]) + final case class PushCheckpointResponse() } diff --git a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala index 2c79c393c7..b93ddabae2 100644 --- a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala +++ b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala @@ -57,7 +57,7 @@ class CheckpointingServiceSpec n <- Gen.choose(0, k - 1) // distance from best block to checkpointed block } yield (k, m, n) - val previousCheckpoint = Fixtures.Blocks.ValidBlock.block + val previousCheckpoint = Fixtures.Blocks.Block3125369.block val hash = previousCheckpoint.hash forAll(nums) { case (k, m, n) => @@ -70,7 +70,7 @@ class CheckpointingServiceSpec val expectedResponse = GetLatestBlockResponse(Some(BlockInfo(block.hash, block.number))) (blockchain.getBestBlockNumber _).expects().returning(bestBlockNum) - (blockchain.getBlockHeaderByHash _).expects(hash).returning(Some(previousCheckpoint.header)) + (blockchain.getBlockHeaderByHash _).expects(hash).returning(Some(previousCheckpoint.header.copy(number = 0))) (blockchain.getBlockByNumber _).expects(checkpointedBlockNum).returning(Some(block)) val result = service.getLatestBlock(request) @@ -78,6 +78,32 @@ class CheckpointingServiceSpec } } + it should "not return a block that is at the same height as the passed parent checkpoint block" in new TestSetup { + val nums = for { + k <- Gen.choose[Int](1, 10) // checkpointing interval + m <- Gen.choose(0, 1000) // number of checkpoints in the chain + n <- Gen.choose(0, k - 1) // distance from best block to checkpointed block + } yield (k, m, n) + + val previousCheckpoint = Fixtures.Blocks.ValidBlock.block + val hash = previousCheckpoint.hash + + forAll(nums) { case (k, m, n) => + val checkpointedBlockNum: BigInt = k * m + val bestBlockNum: BigInt = checkpointedBlockNum + n + + val request = GetLatestBlockRequest(k, Some(hash)) + val expectedResponse = GetLatestBlockResponse(None) + + (blockchain.getBestBlockNumber _).expects().returning(bestBlockNum) + (blockchain.getBlockHeaderByHash _).expects(hash).returning(Some(previousCheckpoint.header.copy(number = bestBlockNum))) + (blockchain.getBlockByNumber _).expects(*).returning(Some(previousCheckpoint)) + val result = service.getLatestBlock(request) + + result.runSyncUnsafe() shouldEqual Right(expectedResponse) + } + } + it should "return an empty response if the descendant is not a part of a local blockchain" in new TestSetup { val nums = for { k <- Gen.choose[Int](1, 10) // checkpointing interval