Skip to content

Commit

Permalink
Extract unnecessary logic out of the synchronized
Browse files Browse the repository at this point in the history
  • Loading branch information
@asoto-iov
asoto-iov committed Jan 17, 2025
1 parent 95a8f1a commit 02c6285
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 109 deletions.
46 changes: 19 additions & 27 deletions rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModuleTransactionBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,12 @@

package co.rsk.rpc.modules.eth;

import static org.ethereum.rpc.exception.RskJsonRpcRequestException.invalidParamError;

import co.rsk.core.RskAddress;
import co.rsk.core.Wallet;
import co.rsk.net.TransactionGateway;
import co.rsk.util.RLPException;
import org.ethereum.config.Constants;
import org.ethereum.core.Account;
import org.ethereum.core.ImmutableTransaction;
import org.ethereum.core.Transaction;
import org.ethereum.core.TransactionArguments;
import org.ethereum.core.TransactionPool;
import org.ethereum.core.TransactionPoolAddResult;
import org.ethereum.core.*;
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.exception.RskJsonRpcRequestException;
import org.ethereum.rpc.parameters.CallArgumentsParam;
Expand All @@ -36,9 +32,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import co.rsk.core.RskAddress;
import co.rsk.core.Wallet;
import co.rsk.net.TransactionGateway;
import static org.ethereum.rpc.exception.RskJsonRpcRequestException.invalidParamError;

public class EthModuleTransactionBase implements EthModuleTransaction {

Expand All @@ -60,7 +54,7 @@ public EthModuleTransactionBase(Constants constants, Wallet wallet, TransactionP
public synchronized String sendTransaction(CallArgumentsParam argsParam) {
CallArguments args = argsParam.toCallArguments();

if(args.getFrom() == null) {
if (args.getFrom() == null) {
throw invalidParamError("from is null");
}

Expand All @@ -73,28 +67,26 @@ public synchronized String sendTransaction(CallArgumentsParam argsParam) {
String txHash = null;

try {
TransactionArguments txArgs = TransactionArgumentsUtil.processArguments(args, constants.getChainId());
Transaction tx = Transaction.builder().withTransactionArguments(txArgs).build();
tx.sign(senderAccount.getEcKey().getPrivKeyBytes());

synchronized (transactionPool) {

TransactionArguments txArgs = TransactionArgumentsUtil.processArguments(args, transactionPool, senderAccount, constants.getChainId());

Transaction tx = Transaction.builder().withTransactionArguments(txArgs).build();

tx.sign(senderAccount.getEcKey().getPrivKeyBytes());
if (!tx.acceptTransactionSignature(constants.getChainId())) {
throw RskJsonRpcRequestException.invalidParamError(TransactionArgumentsUtil.ERR_INVALID_CHAIN_ID + args.getChainId());
}

if (!tx.acceptTransactionSignature(constants.getChainId())) {
throw RskJsonRpcRequestException.invalidParamError(TransactionArgumentsUtil.ERR_INVALID_CHAIN_ID + args.getChainId());
synchronized (transactionPool) {
if (txArgs.getNonce() == null) {
txArgs.setNonce(transactionPool.getPendingState().getNonce(senderAccount.getAddress()));
}

TransactionPoolAddResult result = transactionGateway.receiveTransaction(tx.toImmutableTransaction());

if (!result.transactionsWereAdded()) {
throw RskJsonRpcRequestException.transactionError(result.getErrorMessage());
}

txHash = tx.getHash().toJsonString();
}

txHash = tx.getHash().toJsonString();

return txHash;

} finally {
Expand All @@ -120,8 +112,8 @@ public String sendRawTransaction(HexDataParam rawData) {
if (!result.transactionsWereAdded()) {
throw RskJsonRpcRequestException.transactionError(result.getErrorMessage());
}

return s = tx.getHash().toJsonString();
s = tx.getHash().toJsonString();
return s;
} catch (RLPException e) {
throw invalidParamError("Invalid input: " + e.getMessage(), e);
} finally {
Expand Down
41 changes: 25 additions & 16 deletions rskj-core/src/main/java/co/rsk/rpc/modules/personal/PersonalModuleWalletEnabled.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@

public class PersonalModuleWalletEnabled implements PersonalModule {

private static final Logger LOGGER = LoggerFactory.getLogger("web3");
private static final Logger logger = LoggerFactory.getLogger("web3");

private final Ethereum eth;
private final Wallet wallet;
Expand Down Expand Up @@ -94,10 +94,10 @@ public String newAccountWithSeed(String seed) {

try {
byte[] address = this.wallet.addAccountWithSeed(seed);

return s = HexUtils.toJsonHex(address);
s = HexUtils.toJsonHex(address);
return s;
} finally {
LOGGER.debug("personal_newAccountWithSeed(*****): {}", s);
logger.debug("personal_newAccountWithSeed(*****): {}", s);
}
}

Expand All @@ -109,19 +109,23 @@ public String newAccount(String passphrase) {
RskAddress address = this.wallet.addAccount(passphrase);
// Unlock immediately with no specified duration
unlockAccount(address, passphrase, null);
return s = address.toJsonString();
s = address.toJsonString();
return s;
} finally {
LOGGER.debug("personal_newAccount(*****): {}", s);
logger.debug("personal_newAccount(*****): {}", s);
}
}

@Override
public String[] listAccounts() {
String[] ret = null;
try {
return ret = wallet.getAccountAddressesAsHex();
ret = wallet.getAccountAddressesAsHex();
return ret;
} finally {
LOGGER.debug("personal_listAccounts(): {}", Arrays.toString(ret));
if(logger.isDebugEnabled()) {
logger.debug("personal_listAccounts(): {}", Arrays.toString(ret));
}
}
}

Expand All @@ -139,9 +143,10 @@ public String importRawKey(HexKeyParam keyParam, String passphrase) {
}
RskAddress address = this.wallet.addAccountWithPrivateKey(Hex.decode(key), passphrase);
unlockAccount(address, passphrase, null);
return s = address.toJsonString();
s = address.toJsonString();
return s;
} finally {
LOGGER.debug("personal_importRawKey(*****): {}", s);
logger.debug("personal_importRawKey(*****): {}", s);
}
}

Expand All @@ -150,9 +155,10 @@ public String sendTransaction(CallArgumentsParam argsParam, String passphrase) t
String s = null;
CallArguments args = argsParam.toCallArguments();
try {
return s = sendTransaction(args, getAccount(args.getFrom(), passphrase));
s = sendTransaction(args, getAccount(args.getFrom(), passphrase));
return s;
} finally {
LOGGER.debug("eth_sendTransaction({}): {}", args, s);
logger.debug("eth_sendTransaction({}): {}", args, s);
}
}

Expand Down Expand Up @@ -187,10 +193,10 @@ public String dumpRawKey(HexAddressParam address) throws Exception {
if (account == null) {
throw new Exception("Address private key is locked or could not be found in this node");
}

return s = HexUtils.toJsonHex(ByteUtil.toHexString(account.getEcKey().getPrivKeyBytes()));
s = HexUtils.toJsonHex(ByteUtil.toHexString(account.getEcKey().getPrivKeyBytes()));
return s;
} finally {
LOGGER.debug("personal_dumpRawKey(*****): {}", s);
logger.debug("personal_dumpRawKey(*****): {}", s);
}
}

Expand All @@ -204,7 +210,10 @@ private String sendTransaction(CallArguments args, Account senderAccount) throws
throw new Exception("From address private key could not be found in this node");
}

TransactionArguments txArgs = TransactionArgumentsUtil.processArguments(args, transactionPool, senderAccount, config.getNetworkConstants().getChainId());
TransactionArguments txArgs = TransactionArgumentsUtil.processArguments(args, config.getNetworkConstants().getChainId());
if (txArgs.getNonce() == null) {
txArgs.setNonce(transactionPool.getPendingState().getNonce(senderAccount.getAddress()));
}

Transaction tx = Transaction.builder().withTransactionArguments(txArgs).build();

Expand Down
115 changes: 54 additions & 61 deletions rskj-core/src/main/java/org/ethereum/util/TransactionArgumentsUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,91 +18,84 @@

package org.ethereum.util;

import java.math.BigInteger;
import java.util.Optional;
import java.util.function.Supplier;

import org.ethereum.core.Account;
import co.rsk.util.HexUtils;
import org.ethereum.core.TransactionArguments;
import org.ethereum.core.TransactionPool;
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.exception.RskJsonRpcRequestException;
import org.ethereum.vm.GasCost;

import co.rsk.util.HexUtils;

public class TransactionArgumentsUtil {

private static final BigInteger DEFAULT_GAS_LIMIT = BigInteger.valueOf(GasCost.TRANSACTION_DEFAULT);

public static final String ERR_INVALID_CHAIN_ID = "Invalid chainId: ";
public static final String ERR_COULD_NOT_FIND_ACCOUNT = "Could not find account for address: ";
import java.math.BigInteger;
import java.util.Optional;
import java.util.function.Supplier;

/**
* transform the Web3.CallArguments in TransactionArguments that can be used in
* the TransactionBuilder
*/
public static TransactionArguments processArguments(CallArguments argsParam, TransactionPool transactionPool, Account senderAccount, byte defaultChainId) {
import static co.rsk.util.HexUtils.strHexOrStrNumberToBigInteger;

TransactionArguments argsRet = new TransactionArguments();
public class TransactionArgumentsUtil {

argsRet.setFrom(argsParam.getFrom());
private static final BigInteger DEFAULT_GAS_LIMIT = BigInteger.valueOf(GasCost.TRANSACTION_DEFAULT);

argsRet.setTo(stringHexToByteArray(argsParam.getTo()));
public static final String ERR_INVALID_CHAIN_ID = "Invalid chainId: ";
public static final String ERR_COULD_NOT_FIND_ACCOUNT = "Could not find account for address: ";

argsRet.setNonce(strHexOrStrNumberToBigInteger(argsParam.getNonce(), () -> transactionPool.getPendingState().getNonce(senderAccount.getAddress())));
/**
* transform the Web3.CallArguments in TransactionArguments that can be used in
* the TransactionBuilder
*/
public static TransactionArguments processArguments(CallArguments argsParam, byte defaultChainId) {

argsRet.setValue(strHexOrStrNumberToBigInteger(argsParam.getValue(), () -> BigInteger.ZERO));
TransactionArguments argsRet = new TransactionArguments();

argsRet.setGasPrice(strHexOrStrNumberToBigInteger(argsParam.getGasPrice(), () -> BigInteger.ZERO));
argsRet.setFrom(argsParam.getFrom());

argsRet.setGasLimit(strHexOrStrNumberToBigInteger(argsParam.getGas(), () -> null));
argsRet.setTo(stringHexToByteArray(argsParam.getTo()));

if (argsRet.getGasLimit() == null) {
argsRet.setGasLimit(strHexOrStrNumberToBigInteger(argsParam.getGasLimit(), () -> DEFAULT_GAS_LIMIT));
}
argsRet.setNonce(strHexOrStrNumberToBigInteger(argsParam.getNonce()));

if (argsParam.getData() != null && argsParam.getData().startsWith("0x")) {
argsRet.setData(argsParam.getData().substring(2));
argsParam.setData(argsRet.getData()); // needs to change the parameter because some places expect the changed value after sendTransaction call
}
argsRet.setValue(strHexOrStrNumberToBigIntegerOrDefault(argsParam.getValue(), () -> BigInteger.ZERO));

argsRet.setChainId(hexToChainId(argsParam.getChainId()));
if (argsRet.getChainId() == 0) {
argsRet.setChainId(defaultChainId);
}
argsRet.setGasPrice(strHexOrStrNumberToBigIntegerOrDefault(argsParam.getGasPrice(), () -> BigInteger.ZERO));

return argsRet;
}
argsRet.setGasLimit(strHexOrStrNumberToBigIntegerOrDefault(argsParam.getGas(), () -> null));

private static byte[] stringHexToByteArray(String value) {
if (argsRet.getGasLimit() == null) {
argsRet.setGasLimit(strHexOrStrNumberToBigIntegerOrDefault(argsParam.getGasLimit(), () -> DEFAULT_GAS_LIMIT));
}

byte[] ret = Optional.ofNullable(value).map(HexUtils::stringHexToByteArray).orElse(null);
if (argsParam.getData() != null && argsParam.getData().startsWith("0x")) {
argsRet.setData(argsParam.getData().substring(2));
argsParam.setData(argsRet.getData()); // needs to change the parameter because some places expect the changed value after sendTransaction call
}

return ret;
}
argsRet.setChainId(hexToChainId(argsParam.getChainId()));
if (argsRet.getChainId() == 0) {
argsRet.setChainId(defaultChainId);
}

private static BigInteger strHexOrStrNumberToBigInteger(String value, Supplier<BigInteger> getDefaultValue) {
return argsRet;
}

BigInteger ret = Optional.ofNullable(value).map(HexUtils::strHexOrStrNumberToBigInteger).orElseGet(getDefaultValue);
private static byte[] stringHexToByteArray(String value) {
return Optional.ofNullable(value).map(HexUtils::stringHexToByteArray).orElse(null);
}

return ret;
}
private static BigInteger strHexOrStrNumberToBigIntegerOrDefault(String value, Supplier<BigInteger> getDefaultValue) {
return Optional.ofNullable(value).map(HexUtils::strHexOrStrNumberToBigInteger).orElseGet(getDefaultValue);
}

private static byte hexToChainId(String hex) {
if (hex == null) {
return 0;
}
try {
byte[] bytes = HexUtils.strHexOrStrNumberToByteArray(hex);
if (bytes.length != 1) {
throw RskJsonRpcRequestException.invalidParamError(ERR_INVALID_CHAIN_ID + hex);
}
private static byte hexToChainId(String hex) {
if (hex == null) {
return 0;
}
try {
byte[] bytes = HexUtils.strHexOrStrNumberToByteArray(hex);
if (bytes.length != 1) {
throw RskJsonRpcRequestException.invalidParamError(ERR_INVALID_CHAIN_ID + hex);
}

return bytes[0];
} catch (Exception e) {
throw RskJsonRpcRequestException.invalidParamError(ERR_INVALID_CHAIN_ID + hex, e);
}
}
return bytes[0];
} catch (Exception e) {
throw RskJsonRpcRequestException.invalidParamError(ERR_INVALID_CHAIN_ID + hex, e);
}
}

}
9 changes: 4 additions & 5 deletions rskj-core/src/test/java/org/ethereum/util/TransactionArgumentsUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,15 @@

package org.ethereum.util;

import java.math.BigInteger;

import co.rsk.core.RskAddress;
import co.rsk.core.Wallet;
import org.ethereum.config.Constants;
import org.ethereum.core.TransactionArguments;
import org.ethereum.datasource.HashMapDB;
import org.ethereum.rpc.CallArguments;
import org.junit.jupiter.api.Test;

import co.rsk.core.RskAddress;
import co.rsk.core.Wallet;
import java.math.BigInteger;

import static org.junit.jupiter.api.Assertions.*;

Expand All @@ -44,7 +43,7 @@ void processArguments() {

CallArguments args = TransactionFactoryHelper.createArguments(sender, receiver);

TransactionArguments txArgs = TransactionArgumentsUtil.processArguments(args, null, wallet.getAccount(sender), constants.getChainId());
TransactionArguments txArgs = TransactionArgumentsUtil.processArguments(args, constants.getChainId());

assertEquals(txArgs.getValue(), BigInteger.valueOf(100000L));
assertEquals(txArgs.getGasPrice(), BigInteger.valueOf(10000000000000L));
Expand Down

0 comments on commit 02c6285

Please sign in to comment.