add some logging, wait for importBlock before continuing, don't omit null fields in JSON response, add coinbase field to BlockResponse

Author: Jaap van der Plas

src/main/scala/io/iohk/ethereum/jsonrpc/BlockResponse.scala

@@ -30,6 +30,7 @@ trait BaseBlockResponse {
   def timestamp: BigInt
   def transactions: Either[Seq[ByteString], Seq[TransactionResponse]]
   def uncles: Seq[ByteString]
+  def coinbase: Option[ByteString]
 }
 
 case class EthBlockResponse(
@@ -52,7 +53,8 @@ case class EthBlockResponse(
     gasUsed: BigInt,
     timestamp: BigInt,
     transactions: Either[Seq[ByteString], Seq[TransactionResponse]],
-    uncles: Seq[ByteString]
+    uncles: Seq[ByteString],
+    coinbase: Option[ByteString]
 ) extends BaseBlockResponse
 
 //scalastyle:off method.length
@@ -81,7 +83,8 @@ case class BlockResponse(
     transactions: Either[Seq[ByteString], Seq[TransactionResponse]],
     uncles: Seq[ByteString],
     signature: String,
-    signer: String
+    signer: String,
+    coinbase: Option[ByteString]
 ) extends BaseBlockResponse {
   val chainWeight: Option[ChainWeight] = for {
     lcn <- lastCheckpointNumber
@@ -97,7 +100,8 @@ object BlockResponse {
       block: Block,
       weight: Option[ChainWeight] = None,
       fullTxs: Boolean = false,
-      pendingBlock: Boolean = false
+      pendingBlock: Boolean = false,
+      coinbase: Option[ByteString] = None
   ): BlockResponse = {
     val transactions =
       if (fullTxs)
@@ -129,9 +133,9 @@ object BlockResponse {
     BlockResponse(
       number = block.header.number,
       hash = if (pendingBlock) None else Some(block.header.hash),
-      mixHash = if (pendingBlock) None else Some(block.header.mixHash),
+      mixHash = if (block.header.mixHash.isEmpty) None else Some(block.header.mixHash),
       parentHash = block.header.parentHash,
-      nonce = if (pendingBlock) None else Some(block.header.nonce),
+      nonce = if (block.header.nonce.isEmpty) None else Some(block.header.nonce),
       sha3Uncles = block.header.ommersHash,
       logsBloom = block.header.logsBloom,
       transactionsRoot = block.header.transactionsRoot,
@@ -151,7 +155,8 @@ object BlockResponse {
       transactions = transactions,
       uncles = block.body.uncleNodesList.map(_.hash),
       signature = signatureStr,
-      signer = signerStr
+      signer = signerStr,
+      coinbase = None
     )
   }
 

src/main/scala/io/iohk/ethereum/jsonrpc/JsonRpcRequest.scala

@@ -18,7 +18,8 @@ trait SensitiveInformationToString {
 
 case class JsonRpcRequest(jsonrpc: String, method: String, params: Option[JArray], id: Option[JValue])
     extends SensitiveInformationToString {
-  override def toString(): String = {
+
+  def inspect: String = {
     implicit val formats: Formats = DefaultFormats
     "JsonRpcRequest" + (jsonrpc, method, params.map(write(_)), id.map(write(_))).toString
   }

src/main/scala/io/iohk/ethereum/jsonrpc/JsonRpcResponse.scala

@@ -1,5 +1,13 @@
 package io.iohk.ethereum.jsonrpc
 
+import org.json4s.native.Serialization.write
 import org.json4s.JsonAST.JValue
+import org.json4s.DefaultFormats
+import org.json4s.Formats
 
-case class JsonRpcResponse(jsonrpc: String, result: Option[JValue], error: Option[JsonRpcError], id: JValue)
+case class JsonRpcResponse(jsonrpc: String, result: Option[JValue], error: Option[JsonRpcError], id: JValue) {
+  def inspect: String = {
+    implicit val formats: Formats = DefaultFormats
+    "JsonRpcResponse" + (jsonrpc, result.map(write(_)), error.map(write(_))).toString
+  }
+}

src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala

@@ -135,16 +135,17 @@ class TestService(
 
   def mineBlocks(request: MineBlocksRequest): ServiceResponse[MineBlocksResponse] = {
     def mineBlock(): Task[Unit] = {
-      getBlockForMining(blockchain.getBestBlock().get).map { blockForMining =>
-        val res = testLedgerWrapper.ledger.importBlock(blockForMining.block)
-        log.info("Block mining result: " + res)
-        pendingTransactionsManager ! PendingTransactionsManager.ClearPendingTransactions
-        consensus.blockTimestamp += 1
-      }
+      getBlockForMining(blockchain.getBestBlock().get)
+        .flatMap(blockForMining => testLedgerWrapper.ledger.importBlock(blockForMining.block))
+        .map { res =>
+          log.info("Block mining result: " + res)
+          pendingTransactionsManager ! PendingTransactionsManager.ClearPendingTransactions
+          consensus.blockTimestamp += 1
+        }
     }
 
-    def doNTimesF(n: Int)(fn: Task[Unit]): Task[Unit] = fn.flatMap { res =>
-      if (n <= 1) Task.now(res)
+    def doNTimesF(n: Int)(fn: Task[Unit]): Task[Unit] = fn.flatMap { _ =>
+      if (n <= 1) Task.unit
       else doNTimesF(n - 1)(fn)
     }
 
@@ -194,7 +195,7 @@ class TestService(
     implicit val timeout = Timeout(5.seconds)
     pendingTransactionsManager
       .askFor[PendingTransactionsResponse](PendingTransactionsManager.GetPendingTransactions)
-      // .timeout(timeout.duration)
+      .timeout(timeout.duration)
       .onErrorRecover { case _ => PendingTransactionsResponse(Nil) }
       .map { pendingTxs =>
         consensus.blockGenerator
@@ -207,7 +208,7 @@ class TestService(
           )
           .pendingBlock
       }
-    // .timeout(timeout.duration)
+      .timeout(timeout.duration)
   }
 
   def getAccountsInRange(request: AccountsInRangeRequest): ServiceResponse[AccountsInRangeResponse] = ???

src/main/scala/io/iohk/ethereum/jsonrpc/serialization/JsonSerializers.scala

@@ -8,7 +8,7 @@ import org.json4s.{CustomSerializer, DefaultFormats, Formats, JNull, JString}
 
 object JsonSerializers {
   implicit val formats: Formats =
-    DefaultFormats + UnformattedDataJsonSerializer + QuantitiesSerializer + OptionNoneToJNullSerializer + AddressJsonSerializer
+    DefaultFormats.preservingEmptyValues + UnformattedDataJsonSerializer + QuantitiesSerializer + OptionNoneToJNullSerializer + AddressJsonSerializer
 
   object UnformattedDataJsonSerializer
       extends CustomSerializer[ByteString](_ =>

src/main/scala/io/iohk/ethereum/jsonrpc/server/controllers/JsonRpcBaseController.scala

@@ -49,7 +49,7 @@ trait JsonRpcBaseController {
   def handleRequest(request: JsonRpcRequest): Task[JsonRpcResponse] = {
     val startTimeNanos = System.nanoTime()
 
-    log.debug("received request {}", request)
+    log.debug(s"received request ${request.inspect}")
 
     val notFoundFn: PartialFunction[JsonRpcRequest, Task[JsonRpcResponse]] = { case _ =>
       JsonRpcControllerMetrics.NotFoundMethodsCounter.increment()
@@ -76,6 +76,7 @@ trait JsonRpcBaseController {
             JsonRpcControllerMetrics.recordMethodTime(request.method, time)
           }
       }
+      .flatTap { response => Task { log.debug(s"sending response ${response.inspect}") } }
       .onErrorRecoverWith { case t: Throwable =>
         JsonRpcControllerMetrics.MethodsExceptionCounter.increment()
         log.error(s"Error serving request: ${request.toStringWithSensitiveInformation}", t)

src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala

@@ -368,8 +368,8 @@ trait TestServiceBuilder {
 }
 
 trait TestEthBlockServiceBuilder extends EthBlocksServiceBuilder {
-  self: BlockchainBuilder with TestLedgerBuilder =>
-  override lazy val ethBlocksService = new TestEthBlockServiceWrapper(blockchain, ledger)
+  self: BlockchainBuilder with TestLedgerBuilder with ConsensusBuilder =>
+  override lazy val ethBlocksService = new TestEthBlockServiceWrapper(blockchain, ledger, consensus)
 }
 
 trait EthProofServiceBuilder {

src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala

@@ -5,8 +5,10 @@ import io.iohk.ethereum.jsonrpc.EthBlocksService.{BlockByBlockHashResponse, Bloc
 import io.iohk.ethereum.jsonrpc.{BaseBlockResponse, EthBlockResponse, EthBlocksService, ServiceResponse}
 import io.iohk.ethereum.ledger.Ledger
 import io.iohk.ethereum.utils.Logger
+import io.iohk.ethereum.consensus.Consensus
+import akka.util.ByteString
 
-class TestEthBlockServiceWrapper(blockchain: Blockchain, ledger: Ledger)
+class TestEthBlockServiceWrapper(blockchain: Blockchain, ledger: Ledger, consensus: Consensus)
     extends EthBlocksService(blockchain, ledger)
     with Logger {
 
@@ -41,7 +43,10 @@ class TestEthBlockServiceWrapper(blockchain: Blockchain, ledger: Ledger)
     .map(
       _.map(blockByBlockResponse =>
         BlockByNumberResponse(
-          blockByBlockResponse.blockResponse.map(response => toEthResponse(response))
+          blockByBlockResponse.blockResponse
+            .map(response =>
+              toEthResponse(response).copy(coinbase = Some(ByteString(consensus.config.generic.coinbase.toArray)))
+            )
         )
       )
     )
@@ -66,6 +71,7 @@ class TestEthBlockServiceWrapper(blockchain: Blockchain, ledger: Ledger)
     response.gasUsed,
     response.timestamp,
     response.transactions,
-    response.uncles
+    response.uncles,
+    response.coinbase
   )
 }