Skip to content

Commit

Permalink
[ETCM-139] Report precise information on rpc when account data is mis…
Browse files Browse the repository at this point in the history
…sing
  • Loading branch information
kapke committed Oct 28, 2020
1 parent f0befc1 commit d1acb6f
Show file tree
Hide file tree
Showing 19 changed files with 123 additions and 69 deletions.
Expand Up @@ -4,7 +4,7 @@ import akka.util.ByteString
import io.iohk.ethereum.crypto.ECDSASignature
import io.iohk.ethereum.jsonrpc.CheckpointingService._
import io.iohk.ethereum.jsonrpc.JsonRpcController.Codec
import io.iohk.ethereum.jsonrpc.JsonRpcErrors.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonRpcError.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonSerializers.QuantitiesSerializer
import org.json4s.JsonAST._
import org.json4s.{Extraction, JsonAST}
Expand Down
Expand Up @@ -4,7 +4,7 @@ import akka.util.ByteString
import io.iohk.ethereum.jsonrpc.EthService._
import io.iohk.ethereum.jsonrpc.JsonRpcController.JsonDecoder.NoParamsDecoder
import io.iohk.ethereum.jsonrpc.JsonRpcController.{Codec, JsonDecoder, JsonEncoder}
import io.iohk.ethereum.jsonrpc.JsonRpcErrors.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonRpcError.InvalidParams
import io.iohk.ethereum.jsonrpc.PersonalService.{SendTransactionRequest, SendTransactionResponse, SignRequest}
import org.json4s.{Extraction, JsonAST}
import org.json4s.JsonAST.{JArray, JBool, JString, JValue, _}
Expand Down
76 changes: 35 additions & 41 deletions src/main/scala/io/iohk/ethereum/jsonrpc/EthService.scala
Expand Up @@ -17,6 +17,7 @@ import io.iohk.ethereum.jsonrpc.FilterManager.{FilterChanges, FilterLogs, LogFil
import io.iohk.ethereum.jsonrpc.JsonRpcController.JsonRpcConfig
import io.iohk.ethereum.keystore.KeyStore
import io.iohk.ethereum.ledger.{InMemoryWorldStateProxy, Ledger, StxLedger}
import io.iohk.ethereum.mpt.MerklePatriciaTrie.MissingNodeException
import io.iohk.ethereum.ommers.OmmersPool
import io.iohk.ethereum.rlp
import io.iohk.ethereum.rlp.RLPImplicitConversions._
Expand Down Expand Up @@ -226,7 +227,7 @@ class EthService(

private[this] def ifEthash[Req, Res](req: Req)(f: Req => Res): ServiceResponse[Res] = {
@inline def F[A](x: A): Future[A] = Future.successful(x)
consensus.ifEthash[ServiceResponse[Res]](_ => F(Right(f(req))))(F(Left(JsonRpcErrors.ConsensusIsNotEthash)))
consensus.ifEthash[ServiceResponse[Res]](_ => F(Right(f(req))))(F(Left(JsonRpcError.ConsensusIsNotEthash)))
}

def protocolVersion(req: ProtocolVersionRequest): ServiceResponse[ProtocolVersionResponse] =
Expand Down Expand Up @@ -557,7 +558,7 @@ class EthService(
)
}
response
})(Future.successful(Left(JsonRpcErrors.ConsensusIsNotEthash)))
})(Future.successful(Left(JsonRpcError.ConsensusIsNotEthash)))

private def getOmmersFromPool(parentBlockHash: ByteString): Future[OmmersPool.Ommers] =
consensus.ifEthash(ethash => {
Expand Down Expand Up @@ -602,7 +603,7 @@ class EthService(
Right(SubmitWorkResponse(false))
}
}
})(Future.successful(Left(JsonRpcErrors.ConsensusIsNotEthash)))
})(Future.successful(Left(JsonRpcError.ConsensusIsNotEthash)))

/**
* Implements the eth_syncing method that returns syncing information if the node is syncing.
Expand Down Expand Up @@ -637,10 +638,10 @@ class EthService(
pendingTransactionsManager ! PendingTransactionsManager.AddOrOverrideTransaction(signedTransaction)
Future.successful(Right(SendRawTransactionResponse(signedTransaction.hash)))
} else {
Future.successful(Left(JsonRpcErrors.InvalidRequest))
Future.successful(Left(JsonRpcError.InvalidRequest))
}
case Failure(_) =>
Future.successful(Left(JsonRpcErrors.InvalidRequest))
Future.successful(Left(JsonRpcError.InvalidRequest))
}
}

Expand All @@ -657,7 +658,7 @@ class EthService(
val dataEither = (tx.function, tx.contractCode) match {
case (Some(function), None) => Right(rlp.encode(RLPList(function, args)))
case (None, Some(contractCode)) => Right(rlp.encode(RLPList(contractCode, args)))
case _ => Left(JsonRpcErrors.InvalidParams("Iele transaction should contain either functionName or contractCode"))
case _ => Left(JsonRpcError.InvalidParams("Iele transaction should contain either functionName or contractCode"))
}

dataEither match {
Expand Down Expand Up @@ -710,7 +711,7 @@ class EthService(
Right(GetUncleCountByBlockHashResponse(blockBody.uncleNodesList.size))
case None =>
Left(
JsonRpcErrors.InvalidParams(s"Block with hash ${Hex.toHexString(req.blockHash.toArray[Byte])} not found")
JsonRpcError.InvalidParams(s"Block with hash ${Hex.toHexString(req.blockHash.toArray[Byte])} not found")
)
}
}
Expand Down Expand Up @@ -774,31 +775,22 @@ class EthService(
.flatMap(_ => Right(None))
}

def getBalance(req: GetBalanceRequest): ServiceResponse[GetBalanceResponse] = {
Future {
withAccount(req.address, req.block) { account =>
GetBalanceResponse(account.balance)
}
def getBalance(req: GetBalanceRequest): ServiceResponse[GetBalanceResponse] =
withAccount(req.address, req.block) { account =>
GetBalanceResponse(account.balance)
}
}

def getStorageAt(req: GetStorageAtRequest): ServiceResponse[GetStorageAtResponse] = {
Future {
withAccount(req.address, req.block) { account =>
GetStorageAtResponse(
blockchain.getAccountStorageAt(account.storageRoot, req.position, blockchainConfig.ethCompatibleStorage)
)
}
def getStorageAt(req: GetStorageAtRequest): ServiceResponse[GetStorageAtResponse] =
withAccount(req.address, req.block) { account =>
GetStorageAtResponse(
blockchain.getAccountStorageAt(account.storageRoot, req.position, blockchainConfig.ethCompatibleStorage)
)
}
}

def getTransactionCount(req: GetTransactionCountRequest): ServiceResponse[GetTransactionCountResponse] = {
Future {
withAccount(req.address, req.block) { account =>
GetTransactionCountResponse(account.nonce)
}
def getTransactionCount(req: GetTransactionCountRequest): ServiceResponse[GetTransactionCountResponse] =
withAccount(req.address, req.block) { account =>
GetTransactionCountResponse(account.nonce)
}
}

def newFilter(req: NewFilterRequest): ServiceResponse[NewFilterResponse] = {
implicit val timeout: Timeout = Timeout(filterConfig.filterManagerQueryTimeout)
Expand Down Expand Up @@ -863,20 +855,25 @@ class EthService(
}
}

private def withAccount[T](address: Address, blockParam: BlockParam)(f: Account => T): Either[JsonRpcError, T] = {
resolveBlock(blockParam).map { case ResolvedBlock(block, _) =>
f(
blockchain.getAccount(address, block.header.number).getOrElse(Account.empty(blockchainConfig.accountStartNonce))
)
private def withAccount[T](address: Address, blockParam: BlockParam)(makeResponse: Account => T): ServiceResponse[T] =
Future {
resolveBlock(blockParam)
.map { case ResolvedBlock(block, _) =>
blockchain
.getAccount(address, block.header.number)
.getOrElse(Account.empty(blockchainConfig.accountStartNonce))
}
.map(makeResponse)
}.recover { case _: MissingNodeException =>
Left(JsonRpcError.NodeNotFound)
}
}

private def resolveBlock(blockParam: BlockParam): Either[JsonRpcError, ResolvedBlock] = {
def getBlock(number: BigInt): Either[JsonRpcError, Block] = {
blockchain
.getBlockByNumber(number)
.map(Right.apply)
.getOrElse(Left(JsonRpcErrors.InvalidParams(s"Block $number not found")))
.getOrElse(Left(JsonRpcError.InvalidParams(s"Block $number not found")))
}

blockParam match {
Expand Down Expand Up @@ -929,7 +926,7 @@ class EthService(
if (numBlocksToSearch > jsonRpcConfig.accountTransactionsMaxBlocks) {
Future.successful(
Left(
JsonRpcErrors.InvalidParams(
JsonRpcError.InvalidParams(
s"""Maximum number of blocks to search is ${jsonRpcConfig.accountTransactionsMaxBlocks}, requested: $numBlocksToSearch.
|See: 'network.rpc.account-transactions-max-blocks' config.""".stripMargin
)
Expand Down Expand Up @@ -963,13 +960,10 @@ class EthService(
}
}

def getStorageRoot(req: GetStorageRootRequest): ServiceResponse[GetStorageRootResponse] = {
Future {
withAccount(req.address, req.block) { account =>
GetStorageRootResponse(account.storageRoot)
}
def getStorageRoot(req: GetStorageRootRequest): ServiceResponse[GetStorageRootResponse] =
withAccount(req.address, req.block) { account =>
GetStorageRootResponse(account.storageRoot)
}
}

/**
* Returns the transactions that are pending in the transaction pool and have a from address that is one of the accounts this node manages.
Expand Down
Expand Up @@ -3,7 +3,7 @@ package io.iohk.ethereum.jsonrpc
import akka.util.ByteString
import io.iohk.ethereum.jsonrpc.EthService._
import io.iohk.ethereum.jsonrpc.JsonRpcController.{JsonDecoder, JsonEncoder}
import io.iohk.ethereum.jsonrpc.JsonRpcErrors.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonRpcError.InvalidParams
import io.iohk.ethereum.jsonrpc.PersonalService.{InvalidAddress, SendIeleTransactionRequest}
import org.json4s.JsonAST.{JArray, JObject, JString, JValue}

Expand Down
Expand Up @@ -8,7 +8,7 @@ import io.iohk.ethereum.domain.Address
import io.iohk.ethereum.jsonrpc.EthService.BlockParam
import io.iohk.ethereum.jsonrpc.JsonRpcController.JsonDecoder.NoParamsDecoder
import io.iohk.ethereum.jsonrpc.JsonRpcController.{Codec, JsonDecoder, JsonEncoder}
import io.iohk.ethereum.jsonrpc.JsonRpcErrors.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonRpcError.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonSerializers.{
AddressJsonSerializer,
OptionNoneToJNullSerializer,
Expand Down Expand Up @@ -141,7 +141,7 @@ trait JsonMethodsImplicits {
extractQuantity(other)
.map(BlockParam.WithNumber)
.left
.map(_ => JsonRpcErrors.InvalidParams(s"Invalid default block param: $other"))
.map(_ => JsonRpcError.InvalidParams(s"Invalid default block param: $other"))
}
}

Expand All @@ -152,7 +152,7 @@ trait JsonMethodsImplicits {

object JsonMethodsImplicits extends JsonMethodsImplicits {

import JsonRpcErrors._
import JsonRpcError._

implicit val web3_sha3 = new JsonDecoder[Sha3Request] with JsonEncoder[Sha3Response] {
override def decodeJson(params: Option[JArray]): Either[JsonRpcError, Sha3Request] =
Expand Down
10 changes: 8 additions & 2 deletions src/main/scala/io/iohk/ethereum/jsonrpc/JsonRpcController.scala
Expand Up @@ -10,7 +10,7 @@ import org.json4s.JsonAST.{JArray, JValue}
import org.json4s.JsonDSL._
import com.typesafe.config.{Config => TypesafeConfig}
import io.iohk.ethereum.jsonrpc.DebugService.{ListPeersInfoRequest, ListPeersInfoResponse}
import io.iohk.ethereum.jsonrpc.JsonRpcErrors.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonRpcError.InvalidParams
import io.iohk.ethereum.jsonrpc.QAService.{
GenerateCheckpointRequest,
GenerateCheckpointResponse,
Expand Down Expand Up @@ -47,6 +47,12 @@ object JsonRpcController {
trait JsonEncoder[T] {
def encodeJson(t: T): JValue
}
object JsonEncoder {
def apply[T](implicit encoder: JsonEncoder[T]): JsonEncoder[T] = encoder

implicit def listEncoder[T](implicit itemEncoder: JsonEncoder[T]): JsonEncoder[List[T]] = list =>
JArray(list.map(itemEncoder.encodeJson))
}

trait Codec[Req, Res] extends JsonDecoder[Req] with JsonEncoder[Res]
object Codec {
Expand Down Expand Up @@ -126,7 +132,7 @@ class JsonRpcController(
import TestJsonMethodsImplicits._
import IeleJsonMethodsImplicits._
import JsonMethodsImplicits._
import JsonRpcErrors._
import JsonRpcError._
import DebugJsonMethodsImplicits._
import QAJsonMethodsImplicits._
import CheckpointingJsonMethodsImplicits._
Expand Down
27 changes: 24 additions & 3 deletions src/main/scala/io/iohk/ethereum/jsonrpc/JsonRpcError.scala
@@ -1,13 +1,17 @@
package io.iohk.ethereum.jsonrpc

import io.iohk.ethereum.consensus.Protocol
import org.json4s.JsonAST.JValue
import io.iohk.ethereum.jsonrpc.JsonRpcController.JsonEncoder
import org.json4s.{JInt, JString, JObject, JValue}

case class JsonRpcError(code: Int, message: String, data: Option[JValue])

// scalastyle:off magic.number
// scalastyle:off public.methods.have.type
object JsonRpcErrors {
object JsonRpcError {
def apply[T: JsonEncoder](code: Int, message: String, data: T): JsonRpcError =
JsonRpcError(code, message, Some(JsonEncoder[T].encodeJson(data)))

val ParseError = JsonRpcError(-32700, "An error occurred on the server while parsing the JSON text", None)
val InvalidRequest = JsonRpcError(-32600, "The JSON sent is not a valid Request object", None)
val MethodNotFound = JsonRpcError(-32601, "The method does not exist / is not available", None)
Expand All @@ -16,9 +20,26 @@ object JsonRpcErrors {
def LogicError(msg: String) = JsonRpcError(-32000, msg, None)
val AccountLocked = LogicError("account is locked or unknown")

// We use the recommendation from https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal
// We use the recommendation from https://eth.wiki/json-rpc/json-rpc-error-codes-improvement-proposal
//
// Note Error Code "2", "Action not allowed" could be a candidate here, but the description they provide
// probably does not match this use-case.
final val ConsensusIsNotEthash = JsonRpcError(200, s"The consensus algorithm is not ${Protocol.Names.Ethash}", None)

def executionError(reasons: List[EthCustomError]): JsonRpcError = JsonRpcError(3, "Execution error", reasons)
val NodeNotFound = executionError(List(EthCustomError.DoesntExist("State node")))

// Custom errors based on proposal https://eth.wiki/json-rpc/json-rpc-error-codes-improvement-proposal
sealed abstract class EthCustomError private (val code: Int, val message: String)
object EthCustomError {
implicit val ethCustomErrorEncoder: JsonEncoder[EthCustomError] = err =>
JObject("code" -> JInt(err.code), "message" -> JString(err.message))

case class DoesntExist(what: String) extends EthCustomError(100, s"${what} doesn't exist")
case object RequiresEther extends EthCustomError(101, "Requires ether")
case object GasTooLow extends EthCustomError(102, "Gas too low")
case object GasLimitExceeded extends EthCustomError(103, "Gas limit exceeded")
case object Rejected extends EthCustomError(104, "Rejected")
case object EtherTooLow extends EthCustomError(105, "Ether too low")
}
}
4 changes: 2 additions & 2 deletions src/main/scala/io/iohk/ethereum/jsonrpc/PersonalService.scala
Expand Up @@ -11,7 +11,7 @@ import io.iohk.ethereum.db.storage.AppStateStorage
import io.iohk.ethereum.domain.{Account, Address, Blockchain}
import io.iohk.ethereum.jsonrpc.PersonalService._
import io.iohk.ethereum.keystore.{KeyStore, Wallet}
import io.iohk.ethereum.jsonrpc.JsonRpcErrors._
import io.iohk.ethereum.jsonrpc.JsonRpcError._
import io.iohk.ethereum.rlp.RLPList
import io.iohk.ethereum.transactions.PendingTransactionsManager
import io.iohk.ethereum.transactions.PendingTransactionsManager.{AddOrOverrideTransaction, PendingTransactionsResponse}
Expand Down Expand Up @@ -178,7 +178,7 @@ class PersonalService(
val dataEither = (tx.function, tx.contractCode) match {
case (Some(function), None) => Right(rlp.encode(RLPList(function, args)))
case (None, Some(contractCode)) => Right(rlp.encode(RLPList(contractCode, args)))
case _ => Left(JsonRpcErrors.InvalidParams("Iele transaction should contain either functionName or contractCode"))
case _ => Left(JsonRpcError.InvalidParams("Iele transaction should contain either functionName or contractCode"))
}

dataEither match {
Expand Down
Expand Up @@ -3,7 +3,7 @@ package io.iohk.ethereum.jsonrpc
import akka.util.ByteString
import io.iohk.ethereum.jsonrpc.JsonRpcController.JsonDecoder.NoParamsDecoder
import io.iohk.ethereum.jsonrpc.JsonRpcController.{Codec, JsonEncoder}
import io.iohk.ethereum.jsonrpc.JsonRpcErrors.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonRpcError.InvalidParams
import io.iohk.ethereum.jsonrpc.QAService._
import org.json4s.Extraction
import org.json4s.JsonAST._
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/io/iohk/ethereum/jsonrpc/QAService.scala
Expand Up @@ -39,7 +39,7 @@ class QAService(
.map(_ |> (MineBlocksResponse(_)) |> (_.asRight))
.recover { case t: Throwable =>
log.info("Unable to mine requested blocks", t)
Left(JsonRpcErrors.InternalError)
Left(JsonRpcError.InternalError)
}
}

Expand Down
Expand Up @@ -2,7 +2,7 @@ package io.iohk.ethereum.jsonrpc

import akka.util.ByteString
import io.iohk.ethereum.jsonrpc.JsonRpcController.{JsonDecoder, JsonEncoder}
import io.iohk.ethereum.jsonrpc.JsonRpcErrors.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonRpcError.InvalidParams
import io.iohk.ethereum.jsonrpc.TestService._
import org.json4s.JsonAST._
import org.json4s.JsonDSL._
Expand Down
Expand Up @@ -37,7 +37,7 @@ trait JsonRpcHttpServer extends Json4sSupport {
.newBuilder()
.handle {
case _: MalformedRequestContentRejection =>
complete((StatusCodes.BadRequest, JsonRpcResponse("2.0", None, Some(JsonRpcErrors.ParseError), JInt(0))))
complete((StatusCodes.BadRequest, JsonRpcResponse("2.0", None, Some(JsonRpcError.ParseError), JInt(0))))
case _: CorsRejection =>
complete(StatusCodes.Forbidden)
}
Expand Down
Expand Up @@ -4,7 +4,7 @@ import io.iohk.ethereum.checkpointing.CheckpointingTestHelpers
import io.iohk.ethereum.crypto.ECDSASignature
import io.iohk.ethereum.jsonrpc.CheckpointingService._
import io.iohk.ethereum.jsonrpc.JsonRpcController.JsonRpcConfig
import io.iohk.ethereum.jsonrpc.JsonRpcErrors.InvalidParams
import io.iohk.ethereum.jsonrpc.JsonRpcError.InvalidParams
import io.iohk.ethereum.nodebuilder.SecureRandomBuilder
import io.iohk.ethereum.utils.{ByteStringUtils, Config}
import io.iohk.ethereum.{Fixtures, NormalPatience, crypto}
Expand Down
13 changes: 13 additions & 0 deletions src/test/scala/io/iohk/ethereum/jsonrpc/EthServiceSpec.scala
Expand Up @@ -884,6 +884,19 @@ class EthServiceSpec
response.futureValue shouldEqual Right(GetBalanceResponse(123))
}

it should "handle MissingNodeException when getting balance" in new TestSetup {
val address = Address(ByteString(Hex.decode("abbb6bebfa05aa13e908eaa492bd7a8343760477")))

val newBlockHeader = blockToRequest.header
val newblock = blockToRequest.copy(header = newBlockHeader)
blockchain.storeBlock(newblock).commit()
blockchain.saveBestKnownBlocks(newblock.header.number)

val response = ethService.getBalance(GetBalanceRequest(address, BlockParam.Latest))

response.futureValue shouldEqual Left(JsonRpcError.NodeNotFound)
}

it should "handle getStorageAt request" in new TestSetup {
import io.iohk.ethereum.rlp.UInt256RLPImplicits._

Expand Down

0 comments on commit d1acb6f

Please sign in to comment.