Merge pull request #705 from input-output-hk/feature/ETCM-126-rpc-cleanup

[ETCM-126] RPC cleanup

Author: lemastero

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

@@ -308,33 +308,4 @@ object JsonMethodsImplicits extends JsonMethodsImplicits {
     def encodeJson(t: LockAccountResponse): JValue =
       JBool(t.result)
   }
-
-  implicit val daedalus_deleteWallet = new Codec[DeleteWalletRequest, DeleteWalletResponse] {
-    def decodeJson(params: Option[JArray]): Either[JsonRpcError, DeleteWalletRequest] = {
-      params match {
-        case Some(JArray(JString(addr) :: _)) =>
-          extractAddress(addr).map(DeleteWalletRequest)
-        case _ =>
-          Left(InvalidParams())
-      }
-    }
-
-    def encodeJson(t: DeleteWalletResponse): JValue =
-      JBool(t.result)
-  }
-
-  implicit val daedalus_changePassphrase = new Codec[ChangePassphraseRequest, ChangePassphraseResponse] {
-    def decodeJson(params: Option[JArray]): Either[JsonRpcError, ChangePassphraseRequest] = {
-      params match {
-        case Some(JArray(JString(addr) :: JString(oldPassphrase) :: JString(newPassphrase) :: _)) =>
-          extractAddress(addr).map(a => ChangePassphraseRequest(a, oldPassphrase, newPassphrase))
-        case _ =>
-          Left(InvalidParams())
-      }
-    }
-
-    def encodeJson(t: ChangePassphraseResponse): JValue =
-      JString("")
-  }
-
 }

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

@@ -327,12 +327,6 @@ class JsonRpcController(
   private def handleDaedalusRequest: PartialFunction[JsonRpcRequest, Future[JsonRpcResponse]] = {
     case req @ JsonRpcRequest(_, "daedalus_getAccountTransactions", _, _) =>
       handle[GetAccountTransactionsRequest, GetAccountTransactionsResponse](ethService.getAccountTransactions, req)
-
-    case req @ JsonRpcRequest(_, "daedalus_deleteWallet", _, _) =>
-      handle[DeleteWalletRequest, DeleteWalletResponse](personalService.deleteWallet, req)
-
-    case req @ JsonRpcRequest(_, "daedalus_changePassphrase", _, _) =>
-      handle[ChangePassphraseRequest, ChangePassphraseResponse](personalService.changePassphrase, req)
   }
 
   private def handleQARequest: PartialFunction[JsonRpcRequest, Future[JsonRpcResponse]] = {
@@ -402,5 +396,4 @@ class JsonRpcController(
 
   private def errorResponse[T](req: JsonRpcRequest, error: JsonRpcError): JsonRpcResponse =
     JsonRpcResponse(req.jsonrpc, None, Some(error), req.id.getOrElse(0))
-
 }

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

@@ -55,12 +55,6 @@ object PersonalService {
   case class EcRecoverRequest(message: ByteString, signature: ECDSASignature)
   case class EcRecoverResponse(address: Address)
 
-  case class DeleteWalletRequest(address: Address)
-  case class DeleteWalletResponse(result: Boolean)
-
-  case class ChangePassphraseRequest(address: Address, oldPassphrase: String, newPassphrase: String)
-  case class ChangePassphraseResponse()
-
   val InvalidKey = InvalidParams("Invalid key provided, expected 32 bytes (64 hex digits)")
   val InvalidAddress = InvalidParams("Invalid address, expected 20 bytes (40 hex digits)")
   val InvalidPassphrase = LogicError("Could not decrypt key with given passphrase")
@@ -181,21 +175,6 @@ class PersonalService(
     }
   }
 
-  def deleteWallet(request: DeleteWalletRequest): ServiceResponse[DeleteWalletResponse] = Future {
-    unlockedWallets.remove(request.address)
-
-    keyStore.deleteWallet(request.address)
-      .map(DeleteWalletResponse.apply)
-      .left.map(handleError)
-  }
-
-  def changePassphrase(request: ChangePassphraseRequest): ServiceResponse[ChangePassphraseResponse] = Future {
-    import request._
-    keyStore.changePassphrase(address, oldPassphrase, newPassphrase)
-      .map(_ => ChangePassphraseResponse())
-      .left.map(handleError)
-  }
-
   private def sendTransaction(request: TransactionRequest, wallet: Wallet): Future[ByteString] = {
     implicit val timeout = Timeout(txPoolConfig.pendingTxManagerQueryTimeout)
 

src/main/scala/io/iohk/ethereum/keystore/KeyStore.scala

@@ -35,10 +35,6 @@ trait KeyStore {
   def listAccounts(): Either[KeyStoreError, List[Address]]
 
   def unlockAccount(address: Address, passphrase: String): Either[KeyStoreError, Wallet]
-
-  def deleteWallet(address: Address): Either[KeyStoreError, Boolean]
-
-  def changePassphrase(address: Address, oldPassphrase: String, newPassphrase: String): Either[KeyStoreError, Unit]
 }
 
 class KeyStoreImpl(keyStoreConfig: KeyStoreConfig, secureRandom: SecureRandom) extends KeyStore with Logger {
@@ -95,11 +91,6 @@ class KeyStoreImpl(keyStoreConfig: KeyStoreConfig, secureRandom: SecureRandom) e
   def unlockAccount(address: Address, passphrase: String): Either[KeyStoreError, Wallet] =
     load(address).flatMap(_.decrypt(passphrase).left.map(_ => DecryptionFailed)).map(key => Wallet(address, key))
 
-  def deleteWallet(address: Address): Either[KeyStoreError, Boolean] = for {
-    fileName <- findKeyFileName(address)
-    deleted  <- deleteFile(fileName)
-  } yield deleted
-
   def changePassphrase(address: Address, oldPassphrase: String, newPassphrase: String): Either[KeyStoreError, Unit] = for {
     _           <- validateNewPassPhrase(newPassphrase)
     oldEncKey   <- load(address)

src/test/scala/io/iohk/ethereum/jsonrpc/JsonRpcControllerSpec.scala

@@ -573,42 +573,6 @@ class JsonRpcControllerSpec
     response.result shouldBe Some(JArray(peers.map(info => JString(info.toString))))
   }
 
-  it should "daedalus_deleteWallet" in new TestSetup {
-    val address = Address(42)
-    val params = JArray(JString(address.toString) :: Nil)
-
-    (personalService.deleteWallet _)
-      .expects(DeleteWalletRequest(address))
-      .returning(Future.successful(Right(DeleteWalletResponse(true))))
-
-    val rpcRequest = JsonRpcRequest("2.0", "daedalus_deleteWallet", Some(params), Some(1))
-    val response = jsonRpcController.handleRequest(rpcRequest).futureValue
-
-    response.jsonrpc shouldBe "2.0"
-    response.id shouldBe JInt(1)
-    response.error shouldBe None
-    response.result shouldBe Some(JBool(true))
-  }
-
-  it should "daedalus_changePassphrase" in new TestSetup {
-    val address = Address(42)
-    val oldPassphrase = "weakpass"
-    val newPassphrase = "very5tr0ng&l0ngp4s5phr4s3"
-    val params = JArray(JString(address.toString) :: JString(oldPassphrase) :: JString(newPassphrase) :: Nil)
-
-    (personalService.changePassphrase _)
-      .expects(ChangePassphraseRequest(address, oldPassphrase, newPassphrase))
-      .returning(Future.successful(Right(ChangePassphraseResponse())))
-
-    val rpcRequest = JsonRpcRequest("2.0", "daedalus_changePassphrase", Some(params), Some(1))
-    val response = jsonRpcController.handleRequest(rpcRequest).futureValue
-
-    response.jsonrpc shouldBe "2.0"
-    response.id shouldBe JInt(1)
-    response.error shouldBe None
-    response.result shouldBe Some(JString(""))
-  }
-
   it should "eth_sendTransaction" in new TestSetup {
     val params = JArray(
       JObject(

src/test/scala/io/iohk/ethereum/jsonrpc/PersonalServiceSpec.scala

@@ -11,7 +11,7 @@ import io.iohk.ethereum.db.storage.AppStateStorage
 import io.iohk.ethereum.domain.{UInt256, _}
 import io.iohk.ethereum.jsonrpc.JsonRpcErrors._
 import io.iohk.ethereum.jsonrpc.PersonalService._
-import io.iohk.ethereum.keystore.KeyStore.{DecryptionFailed, IOError, KeyStoreError}
+import io.iohk.ethereum.keystore.KeyStore.{DecryptionFailed, IOError}
 import io.iohk.ethereum.keystore.{KeyStore, Wallet}
 import io.iohk.ethereum.transactions.PendingTransactionsManager._
 import io.iohk.ethereum.utils.{BlockchainConfig, MonetaryPolicyConfig, TxPoolConfig}
@@ -390,48 +390,6 @@ class PersonalServiceSpec
     }
   }
 
-  it should "delete existing wallet" in new TestSetup {
-    (keyStore.deleteWallet _)
-      .expects(address)
-      .returning(Right(true))
-
-    val delRes = personal.deleteWallet(DeleteWalletRequest(address)).futureValue
-    delRes shouldEqual Right(DeleteWalletResponse(true))
-  }
-
-  it should "return error when deleting not existing wallet" in new TestSetup {
-    (keyStore.deleteWallet _)
-      .expects(address)
-      .returning(Left(KeyStore.KeyNotFound))
-
-    val delRes = personal.deleteWallet(DeleteWalletRequest(address)).futureValue
-    delRes shouldEqual Left(KeyNotFound)
-  }
-
-  it should "handle changing passwords" in new TestSetup {
-    type KeyStoreRes = Either[KeyStoreError, Unit]
-    type ServiceRes = Either[JsonRpcError, ChangePassphraseResponse]
-
-    val table = Table[KeyStoreRes, ServiceRes](
-      ("keyStoreResult", "serviceResult"),
-      (Right(()), Right(ChangePassphraseResponse())),
-      (Left(KeyStore.KeyNotFound), Left(KeyNotFound)),
-      (Left(KeyStore.DecryptionFailed), Left(InvalidPassphrase))
-    )
-
-    val request = ChangePassphraseRequest(address, "weakpass", "very5tr0ng&l0ngp4s5phr4s3")
-
-    forAll(table) { (keyStoreResult, serviceResult) =>
-      (keyStore.changePassphrase _)
-        .expects(address, request.oldPassphrase, request.newPassphrase)
-        .returning(keyStoreResult)
-
-      val result = personal.changePassphrase(request).futureValue
-      result shouldEqual serviceResult
-    }
-
-  }
-
   trait TestSetup {
     val prvKey = ByteString(Hex.decode("7a44789ed3cd85861c0bbf9693c7e1de1862dd4396c390147ecf1275099c6e6f"))
     val address = Address(Hex.decode("aa6826f00d01fe4085f0c3dd12778e206ce4e2ac"))

src/test/scala/io/iohk/ethereum/keystore/KeyStoreImplSpec.scala

@@ -108,9 +108,6 @@ class KeyStoreImplSpec extends AnyFlatSpec with Matchers with BeforeAndAfter wit
 
     val res3 = keyStore.listAccounts()
     res3 should matchPattern { case Left(IOError(_)) => }
-
-    val res4 = keyStore.deleteWallet(Address(key))
-    res4 should matchPattern { case Left(IOError(_)) => }
   }
 
   it should "unlock an account provided a correct passphrase" in new TestSetup {
@@ -131,48 +128,6 @@ class KeyStoreImplSpec extends AnyFlatSpec with Matchers with BeforeAndAfter wit
     res shouldEqual Left(KeyNotFound)
   }
 
-  it should "return an error deleting not existing wallet" in new TestSetup {
-    val res = keyStore.deleteWallet(addr1)
-    res shouldEqual Left(KeyNotFound)
-  }
-
-  it should "delete existing wallet " in new TestSetup {
-    val newAddr1 = keyStore.newAccount("aaaaaaaa").right.get
-    val listOfNewAccounts = keyStore.listAccounts().right.get
-    listOfNewAccounts.toSet shouldEqual Set(newAddr1)
-
-
-    val res = keyStore.deleteWallet(newAddr1).right.get
-    res shouldBe true
-
-    val listOfNewAccountsAfterDelete = keyStore.listAccounts().right.get
-    listOfNewAccountsAfterDelete.toSet shouldEqual Set.empty
-  }
-
-  it should "change passphrase of an existing wallet" in new TestSetup {
-    val oldPassphrase = "weakpass"
-    val newPassphrase = "very5tr0ng&l0ngp4s5phr4s3"
-
-    keyStore.importPrivateKey(key1, oldPassphrase)
-    keyStore.changePassphrase(addr1, oldPassphrase, newPassphrase) shouldEqual Right(())
-
-    keyStore.unlockAccount(addr1, newPassphrase) shouldEqual Right(Wallet(addr1, key1))
-  }
-
-  it should "return an error when changing passphrase of an non-existent wallet" in new TestSetup {
-    keyStore.changePassphrase(addr1, "oldpass", "newpasss") shouldEqual Left(KeyNotFound)
-  }
-
-  it should "return an error when changing passphrase and provided with invalid old passphrase" in new TestSetup {
-    keyStore.importPrivateKey(key1, "oldpasss")
-    keyStore.changePassphrase(addr1, "wrongpass", "newpasss") shouldEqual Left(DecryptionFailed)
-  }
-
-  it should "return an error when changing passphrase and provided with too short new passphrase" in new TestSetup {
-    keyStore.importPrivateKey(key1, "oldpass")
-    keyStore.changePassphrase(addr1, "wrongpass", "newpass") shouldEqual Left(PassPhraseTooShort(keyStoreConfig.minimalPassphraseLength))
-  }
-
   trait TestSetup {
     val keyStoreConfig =  KeyStoreConfig(Config.config)