Remove converter from transactionsigner (Consensys#24)

ethsigner/src/integration-test/java/tech/pegasys/ethsigner/jsonrpcproxy/IntegrationTestBase.java

@@ -103,7 +103,12 @@ protected static void setupEthSigner(final long chainId) throws IOException, Cip
     httpServerOptions.setHost("localhost");
 
     runner =
-        new Runner(transactionSigner, httpClientOptions, httpServerOptions, Duration.ofSeconds(5));
+        new Runner(
+            transactionSigner,
+            httpClientOptions,
+            httpServerOptions,
+            Duration.ofSeconds(5),
+            new RawTransactionConverter());
     runner.start();
 
     LOG.info(
@@ -185,7 +190,7 @@ private static TransactionSigner transactionSigner(final ChainIdProvider chain)
     final File keyFile = createKeyFile();
     final Credentials credentials = WalletUtils.loadCredentials("password", keyFile);
 
-    return new TransactionSigner(chain, credentials, new RawTransactionConverter());
+    return new TransactionSigner(chain, credentials);
   }
 
   @SuppressWarnings("UnstableApiUsage")

ethsigner/src/main/java/tech/pegasys/ethsigner/EthSigner.java

@@ -78,6 +78,7 @@ public void run() {
               .setReuseAddress(true)
               .setReusePort(true));
       runnerBuilder.setHttpRequestTimeout(config.getDownstreamHttpRequestTimeout());
+      runnerBuilder.setTransactionConverter(new RawTransactionConverter());
       runnerBuilder.build().start();
     } catch (IOException ex) {
       LOG.info(
@@ -92,7 +93,7 @@ private TransactionSigner transactionSigner(
       throws IOException, CipherException {
     final Credentials credentials = WalletUtils.loadCredentials(password, keyFile);
 
-    return new TransactionSigner(chain, credentials, new RawTransactionConverter());
+    return new TransactionSigner(chain, credentials);
   }
 
   private Optional<String> readPasswordFromFile() {

ethsigner/src/main/java/tech/pegasys/ethsigner/Runner.java

@@ -39,6 +39,7 @@ public class Runner {
   private final HttpClientOptions clientOptions;
   private final HttpServerOptions serverOptions;
   private final Duration httpRequestTimeout;
+  private final RawTransactionConverter transactionConverter;
   private final HttpResponseFactory responseFactory = new HttpResponseFactory();
   private final JsonRpcErrorReporter errorReporter = new JsonRpcErrorReporter(responseFactory);
 
@@ -49,11 +50,13 @@ public Runner(
       final TransactionSigner transactionSigner,
       final HttpClientOptions clientOptions,
       final HttpServerOptions serverOptions,
-      final Duration httpRequestTimeout) {
+      final Duration httpRequestTimeout,
+      final RawTransactionConverter transactionConverter) {
     this.transactionSigner = transactionSigner;
     this.clientOptions = clientOptions;
     this.serverOptions = serverOptions;
     this.httpRequestTimeout = httpRequestTimeout;
+    this.transactionConverter = transactionConverter;
   }
 
   public void start() {
@@ -79,7 +82,8 @@ private RequestMapper createRequestMapper(
 
     requestMapper.addHandler(
         "eth_sendTransaction",
-        new SendTransactionHandler(errorReporter, downStreamConnection, transactionSigner));
+        new SendTransactionHandler(
+            errorReporter, downStreamConnection, transactionSigner, transactionConverter));
 
     requestMapper.addHandler(
         "eth_accounts",

ethsigner/src/main/java/tech/pegasys/ethsigner/RunnerBuilder.java

@@ -29,6 +29,7 @@ public class RunnerBuilder {
   private WebClientOptions clientOptions;
   private HttpServerOptions serverOptions;
   private Duration requestTimeout;
+  private RawTransactionConverter transactionConverter;
 
   public RunnerBuilder() {}
 
@@ -48,6 +49,10 @@ public void setHttpRequestTimeout(final Duration requestTimeout) {
     this.requestTimeout = requestTimeout;
   }
 
+  public void setTransactionConverter(final RawTransactionConverter transactionConverter) {
+    this.transactionConverter = transactionConverter;
+  }
+
   public Runner build() {
     if (transactionSigner == null) {
       LOG.error("Unable to construct Runner, transactionSigner is unset.");
@@ -61,10 +66,17 @@ public Runner build() {
       LOG.error("Unable to construct Runner, serverOptions is unset.");
       return null;
     }
+
+    if (transactionConverter == null) {
+      LOG.error("Unable to construct Runner, transaction transactionConverter is unset.");
+      return null;
+    }
+
     if (requestTimeout == null) {
       LOG.error("Unable to construct Runner, requestTimeout is unset.");
       return null;
     }
-    return new Runner(transactionSigner, clientOptions, serverOptions, requestTimeout);
+    return new Runner(
+        transactionSigner, clientOptions, serverOptions, requestTimeout, transactionConverter);
   }
 }

ethsigner/src/main/java/tech/pegasys/ethsigner/jsonrpcproxy/SendTransactionHandler.java

@@ -14,6 +14,7 @@
 
 import static java.util.Collections.singletonList;
 
+import tech.pegasys.ethsigner.RawTransactionConverter;
 import tech.pegasys.ethsigner.jsonrpc.JsonRpcRequest;
 import tech.pegasys.ethsigner.jsonrpc.JsonRpcRequestId;
 import tech.pegasys.ethsigner.jsonrpc.SendTransactionJsonParameters;
@@ -29,6 +30,7 @@
 import io.vertx.core.json.Json;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.web3j.crypto.RawTransaction;
 
 public class SendTransactionHandler implements JsonRpcRequestHandler {
 
@@ -40,14 +42,17 @@ public class SendTransactionHandler implements JsonRpcRequestHandler {
   private final JsonRpcErrorReporter errorReporter;
   private final HttpClient ethNodeClient;
   private final TransactionSigner signer;
+  private final RawTransactionConverter converter;
 
   public SendTransactionHandler(
       final JsonRpcErrorReporter errorReporter,
       final HttpClient ethNodeClient,
-      TransactionSigner signer) {
+      final TransactionSigner signer,
+      final RawTransactionConverter converter) {
     this.errorReporter = errorReporter;
     this.ethNodeClient = ethNodeClient;
     this.signer = signer;
+    this.converter = converter;
   }
 
   @Override
@@ -121,9 +126,18 @@ private JsonRpcBody getBody(final JsonRpcRequest request) {
       return createJsonRpcBodyFrom(request.getId(), JsonRpcError.INVALID_PARAMS);
     }
 
+    if (senderNotUnlockedAccount(params)) {
+      LOG.info(
+          "From address ({}) does not match unlocked account ({})",
+          params.sender(),
+          signer.getAddress());
+      return createJsonRpcBodyFrom(request.getId(), JsonRpcError.INVALID_PARAMS);
+    }
+
     final String signedTransactionHexString;
     try {
-      signedTransactionHexString = signer.signTransaction(params);
+      final RawTransaction rawTransaction = converter.from(params);
+      signedTransactionHexString = signer.signTransaction(rawTransaction);
     } catch (final IllegalArgumentException e) {
       LOG.debug("Bad input value from request: {}", request, e);
       return createJsonRpcBodyFrom(request.getId(), JsonRpcError.INVALID_PARAMS);
@@ -149,4 +163,8 @@ private static JsonRpcBody createJsonRpcBodyFrom(
       final JsonRpcRequestId id, final JsonRpcError error) {
     return new JsonRpcBody(new JsonRpcErrorResponse(id, error));
   }
+
+  private boolean senderNotUnlockedAccount(final SendTransactionJsonParameters params) {
+    return !params.sender().equalsIgnoreCase(signer.getAddress());
+  }
 }

ethsigner/src/main/java/tech/pegasys/ethsigner/signing/TransactionSigner.java

@@ -12,8 +12,6 @@
  */
 package tech.pegasys.ethsigner.signing;
 
-import tech.pegasys.ethsigner.RawTransactionConverter;
-import tech.pegasys.ethsigner.jsonrpc.SendTransactionJsonParameters;
 import tech.pegasys.ethsigner.signing.web3j.TransactionEncoder;
 
 import org.web3j.crypto.Credentials;
@@ -24,35 +22,20 @@ public class TransactionSigner {
 
   private final Credentials credentials;
   private final ChainIdProvider chain;
-  private final RawTransactionConverter converter;
 
-  public TransactionSigner(
-      final ChainIdProvider chain,
-      final Credentials credentials,
-      final RawTransactionConverter converter) {
+  public TransactionSigner(final ChainIdProvider chain, final Credentials credentials) {
     this.chain = chain;
     this.credentials = credentials;
-    this.converter = converter;
   }
 
   public String getAddress() {
     return credentials.getAddress();
   }
 
-  public String signTransaction(final SendTransactionJsonParameters params) {
-    if (senderNotUnlockedAccount(params)) {
-      throw new IllegalArgumentException("From address does not match unlocked account");
-    }
-
-    final RawTransaction rawTransaction = converter.from(params);
-
+  public String signTransaction(final RawTransaction rawTransaction) {
     // Sign the transaction using the post Spurious Dragon technique
     final byte[] signedMessage =
         TransactionEncoder.signMessage(rawTransaction, chain.id(), credentials);
     return Numeric.toHexString(signedMessage);
   }
-
-  private boolean senderNotUnlockedAccount(final SendTransactionJsonParameters params) {
-    return !params.sender().equalsIgnoreCase(credentials.getAddress());
-  }
 }