Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Json RPC method parameter validation #2097

Merged
merged 59 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
af96058
Updating BlockHash related methods having validated parameters. Added…
asoto-iov Aug 9, 2023
22cc178
Updated deserializers parent class and updated new eth_getTransaction…
asoto-iov Aug 10, 2023
f1baf19
fix hex input validation
casiojapi Aug 15, 2023
d45a14f
fix unit test exception message
casiojapi Aug 15, 2023
1722017
fix sonarqube warning
casiojapi Aug 16, 2023
50df9f9
add serialVersionUID to deserializer
casiojapi Aug 17, 2023
bc6baa5
Add deserializers for CallArguments, RawData, Address and BlockIdenti…
rmoreliovlabs Aug 26, 2023
ba11d0f
Fix sonarcloud issues
rmoreliovlabs Aug 28, 2023
e9f27a0
Refactor deserializers
rmoreliovlabs Aug 28, 2023
9e04a89
Add BlockRef and BlockRefParam classes
rmoreliovlabs Aug 30, 2023
63e7dd6
Refactor BlockRefParam
rmoreliovlabs Aug 31, 2023
85c2fe0
Adding new Parameter classes
asoto-iov Aug 30, 2023
acbde9e
Refactor BlockRefParam deserialize to check for the param's type
rmoreliovlabs Aug 31, 2023
8c5affe
Updating eth_newFilter method with FilterRequestParam
asoto-iov Aug 31, 2023
27cc17d
Updating eth_getLogs method with FilterRequestParam
asoto-iov Aug 31, 2023
7a9a5d4
Adding param validation to eth_unistallFilter, eth_getFilterChanges a…
asoto-iov Aug 31, 2023
95d61d6
Merge pull request #2117 from rsksmart/json_rpc_param_validation_pt1
Vovchyk Sep 4, 2023
eda346b
Refactor CallArgumentsParam and add HexNumberParam
rmoreliovlabs Sep 4, 2023
5c00e1e
Fix SonarCloud issues
rmoreliovlabs Sep 4, 2023
a0f9f5a
Code cleaning
rmoreliovlabs Sep 5, 2023
7bfa8ee
Add HexKeyParam and HexDurationParam classes
rmoreliovlabs Sep 7, 2023
e970d38
Fix sonar issues
rmoreliovlabs Sep 7, 2023
207d69a
Remove Serializable implementation from HexStringParam child classes
rmoreliovlabs Sep 7, 2023
8cc47ef
Merge branch 'master' into json_rpc_param_validation
Vovchyk Sep 8, 2023
9104c17
Fix failing tests
rmoreliovlabs Sep 8, 2023
e6f9cc7
Implement deserializers
rmoreliovlabs Aug 31, 2023
5963770
Refactor CallArgumentsParam implementation
rmoreliovlabs Sep 5, 2023
d1f32b5
Make sendTransaction method synchronized
rmoreliovlabs Sep 6, 2023
fddec39
Fix code smells
rmoreliovlabs Sep 6, 2023
d0fe948
Suppress Sonar warnings
rmoreliovlabs Sep 6, 2023
b3ed700
Fix failing tests on EthModuleTest
rmoreliovlabs Sep 8, 2023
f36a9fc
Merge pull request #2121 from rsksmart/implement_json_param_validation
Vovchyk Sep 8, 2023
5131f49
Update methods argument classes
rmoreliovlabs Sep 11, 2023
4ad802d
Merge pull request #2130 from rsksmart/json_rpc_param_validation_pt3
Vovchyk Sep 12, 2023
aefe3d7
Remove unused methods
rmoreliovlabs Sep 12, 2023
c717e62
Update methods arguments types
rmoreliovlabs Sep 14, 2023
66589f9
Fix failing tests
rmoreliovlabs Sep 14, 2023
35df451
Merge pull request #2132 from rsksmart/json_rpc_param_validation_pt5
Vovchyk Sep 15, 2023
59ebddf
Merge remote-tracking branch 'origin/master' into json_rpc_param_vali…
Vovchyk Sep 15, 2023
50412d2
Addressing comments
rmoreliovlabs Sep 18, 2023
d770ff7
Fix failing tests
rmoreliovlabs Sep 18, 2023
016b547
Move null check from HexKeyParam to the importRawKey method
rmoreliovlabs Sep 19, 2023
bdc6dd1
Check null value in newAccount method
asoto-iov Sep 20, 2023
82079f9
Merge pull request #2136 from rsksmart/check_null_parameter_for_new_a…
Vovchyk Sep 20, 2023
aabccdd
Revert changes on EthModuleWallet sign method
rmoreliovlabs Sep 20, 2023
61c0687
Remove unused imports
rmoreliovlabs Sep 20, 2023
0dd8a3e
Add (C) header to new files
rmoreliovlabs Sep 21, 2023
5ca790a
Update invalid hex error messages
rmoreliovlabs Sep 21, 2023
8d27c79
Send error object to invalidParamError for logging
rmoreliovlabs Sep 21, 2023
24fe940
Adding support for null topics
asoto-iov Sep 21, 2023
73909be
minor code improvement
asoto-iov Sep 22, 2023
f05a209
Merge pull request #2139 from rsksmart/support_null_topics_in_FilterR…
Vovchyk Sep 22, 2023
66549ed
Merge remote-tracking branch 'origin/master' into json_rpc_param_vali…
Vovchyk Sep 22, 2023
55b42e8
Fixed tests after the merge from master
Vovchyk Sep 22, 2023
3b3bcc6
Added proper handling of number parsing
Vovchyk Sep 22, 2023
5d0a327
Added check for empty string in HexIndexParamTest
Vovchyk Sep 22, 2023
4e64426
Merge pull request #2140 from rsksmart/hex-num-parsing
Vovchyk Sep 22, 2023
59173f6
Merge branch 'master' into json_rpc_param_validation
Vovchyk Oct 3, 2023
bdc22a1
Merge branch 'master' into json_rpc_param_validation
Vovchyk Oct 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ public ExecutionBlockRetriever(Blockchain blockchain, BlockToMineBuilder builder
}

public Result retrieveExecutionBlock(String bnOrId) {

if(BlockTag.EARLIEST.tagEquals(bnOrId)) {
if (BlockTag.EARLIEST.tagEquals(bnOrId)) {
return Result.ofBlock(blockchain.getBlockByNumber(0));
}

Expand Down
81 changes: 42 additions & 39 deletions rskj-core/src/main/java/co/rsk/rpc/Web3EthModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,35 @@
package co.rsk.rpc;

import co.rsk.rpc.modules.eth.EthModule;
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.FilterRequest;
import org.ethereum.rpc.dto.BlockResultDTO;
import org.ethereum.rpc.dto.CompilationResultDTO;
import org.ethereum.rpc.dto.TransactionReceiptDTO;
import org.ethereum.rpc.dto.TransactionResultDTO;
import org.ethereum.rpc.parameters.BlockHashParam;
import org.ethereum.rpc.parameters.FilterRequestParam;
import org.ethereum.rpc.parameters.BlockIdentifierParam;
import org.ethereum.rpc.parameters.BlockRefParam;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexAddressParam;
import org.ethereum.rpc.parameters.HexDataParam;
import org.ethereum.rpc.parameters.HexIndexParam;
import org.ethereum.rpc.parameters.HexNumberParam;
import org.ethereum.rpc.parameters.TxHashParam;

import java.math.BigInteger;
import java.util.Map;

@SuppressWarnings({"java:S100", "java:S112"})
public interface Web3EthModule {
default String[] eth_accounts() {
return getEthModule().accounts();
}

default String eth_sign(String addr, String data) {
return getEthModule().sign(addr, data);
default String eth_sign(HexAddressParam addr, HexDataParam data) {
return getEthModule().sign(addr.getAddress().toHexString(), data.getAsHexString());
}

default String eth_call(CallArguments args, String bnOrId) {
default String eth_call(CallArgumentsParam args, BlockIdentifierParam bnOrId) {
return getEthModule().call(args, bnOrId);
}

Expand All @@ -50,8 +59,8 @@ default String eth_chainId() {
return getEthModule().chainId();
}

String eth_estimateGas(CallArguments args, String bnOrId);
String eth_estimateGas(CallArguments args);
String eth_estimateGas(CallArgumentsParam args);
String eth_estimateGas(CallArgumentsParam args, BlockIdentifierParam bnOrId);

EthModule getEthModule();

Expand All @@ -69,59 +78,53 @@ default String eth_chainId() {

String eth_blockNumber();

String eth_call(CallArguments args, Map<String, String> blockRef) throws Exception; // NOSONAR
String eth_call(CallArgumentsParam args, Map<String, String> blockRef) throws Exception; // NOSONAR

String eth_getBalance(String address, String block) throws Exception;
String eth_getBalance(HexAddressParam address, BlockRefParam blockRefParam) throws Exception;

String eth_getBalance(String address) throws Exception;
String eth_getBalance(HexAddressParam address) throws Exception;

String eth_getBalance(String address, Map<String, String> blockRef) throws Exception; // NOSONAR
String eth_getStorageAt(HexAddressParam address, HexNumberParam storageIdx, BlockRefParam blockRefParam) throws Exception;

String eth_getStorageAt(String address, String storageIdx, Map<String, String> blockRef) throws Exception; // NOSONAR
String eth_getTransactionCount(HexAddressParam address, BlockRefParam blockRefParam) throws Exception;

String eth_getStorageAt(String address, String storageIdx, String blockId) throws Exception;
String eth_getBlockTransactionCountByHash(BlockHashParam blockHash)throws Exception;

String eth_getTransactionCount(String address, Map<String, String> blockRef) throws Exception; // NOSONAR
String eth_getBlockTransactionCountByNumber(BlockIdentifierParam bnOrId)throws Exception;

String eth_getTransactionCount(String address, String blockId) throws Exception ;
String eth_getUncleCountByBlockHash(BlockHashParam blockHash)throws Exception;

String eth_getBlockTransactionCountByHash(String blockHash)throws Exception;
String eth_getUncleCountByBlockNumber(BlockIdentifierParam bnOrId)throws Exception;

String eth_getBlockTransactionCountByNumber(String bnOrId)throws Exception;
String eth_getCode(HexAddressParam address, BlockRefParam blockRefParam) throws Exception;

String eth_getUncleCountByBlockHash(String blockHash)throws Exception;

String eth_getUncleCountByBlockNumber(String bnOrId)throws Exception;

default String eth_getCode(String address, String blockId) {
default String getCode(HexAddressParam address, String blockId) {
return getEthModule().getCode(address, blockId);
}

String eth_getCode(String address, Map<String, String> blockRef) throws Exception; // NOSONAR

default String eth_sendRawTransaction(String rawData) {
default String eth_sendRawTransaction(HexDataParam rawData) {
return getEthModule().sendRawTransaction(rawData);
}

default String eth_sendTransaction(CallArguments args) {
default String eth_sendTransaction(CallArgumentsParam args) {
return getEthModule().sendTransaction(args);
}

BlockResultDTO eth_getBlockByHash(String blockHash, Boolean fullTransactionObjects) throws Exception;
BlockResultDTO eth_getBlockByHash(BlockHashParam blockHash, Boolean fullTransactionObjects) throws Exception;

BlockResultDTO eth_getBlockByNumber(String bnOrId, Boolean fullTransactionObjects) throws Exception;
BlockResultDTO eth_getBlockByNumber(BlockIdentifierParam bnOrId, Boolean fullTransactionObjects) throws Exception;

TransactionResultDTO eth_getTransactionByHash(String transactionHash) throws Exception;
TransactionResultDTO eth_getTransactionByHash(TxHashParam transactionHash) throws Exception;

TransactionResultDTO eth_getTransactionByBlockHashAndIndex(String blockHash, String index) throws Exception;
TransactionResultDTO eth_getTransactionByBlockHashAndIndex(BlockHashParam blockHash, HexIndexParam index) throws Exception;

TransactionResultDTO eth_getTransactionByBlockNumberAndIndex(String bnOrId, String index) throws Exception;
TransactionResultDTO eth_getTransactionByBlockNumberAndIndex(BlockIdentifierParam bnOrId, HexIndexParam index) throws Exception;

TransactionReceiptDTO eth_getTransactionReceipt(String transactionHash) throws Exception;
TransactionReceiptDTO eth_getTransactionReceipt(TxHashParam transactionHash) throws Exception;

BlockResultDTO eth_getUncleByBlockHashAndIndex(String blockHash, String uncleIdx) throws Exception;
BlockResultDTO eth_getUncleByBlockHashAndIndex(BlockHashParam blockHash, HexIndexParam uncleIdx) throws Exception;

BlockResultDTO eth_getUncleByBlockNumberAndIndex(String blockId, String uncleIdx) throws Exception;
BlockResultDTO eth_getUncleByBlockNumberAndIndex(BlockIdentifierParam blockId, HexIndexParam uncleIdx) throws Exception;

String[] eth_getCompilers();

Expand All @@ -131,19 +134,19 @@ default String eth_sendTransaction(CallArguments args) {

Map<String, CompilationResultDTO> eth_compileSolidity(String contract);

String eth_newFilter(FilterRequest fr) throws Exception;
String eth_newFilter(FilterRequestParam fr) throws Exception;

String eth_newBlockFilter();

String eth_newPendingTransactionFilter();

boolean eth_uninstallFilter(String id);
boolean eth_uninstallFilter(HexIndexParam id);

Object[] eth_getFilterChanges(String id);
Object[] eth_getFilterChanges(HexIndexParam id);

Object[] eth_getFilterLogs(String id);
Object[] eth_getFilterLogs(HexIndexParam id);

Object[] eth_getLogs(FilterRequest fr) throws Exception;
Object[] eth_getLogs(FilterRequestParam fr) throws Exception;

BigInteger eth_netHashrate();

Expand Down
26 changes: 15 additions & 11 deletions rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.converters.CallArgumentsToByteArray;
import org.ethereum.rpc.exception.RskJsonRpcRequestException;
import org.ethereum.rpc.parameters.BlockIdentifierParam;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexAddressParam;
import org.ethereum.rpc.parameters.HexDataParam;
import org.ethereum.vm.GasCost;
import org.ethereum.vm.PrecompiledContracts;
import org.ethereum.vm.program.ProgramResult;
Expand Down Expand Up @@ -121,10 +125,11 @@ public Map<String, Object> bridgeState() throws IOException, BlockStoreException
return state.stateToMap();
}

public String call(CallArguments args, String bnOrId) {
public String call(CallArgumentsParam argsParam, BlockIdentifierParam bnOrId) {
String hReturn = null;
CallArguments args = argsParam.toCallArguments();
try {
ExecutionBlockRetriever.Result result = executionBlockRetriever.retrieveExecutionBlock(bnOrId);
ExecutionBlockRetriever.Result result = executionBlockRetriever.retrieveExecutionBlock(bnOrId.getIdentifier());
Block block = result.getBlock();
Trie finalState = result.getFinalState();
ProgramResult res;
Expand All @@ -151,14 +156,13 @@ public String call(CallArguments args, String bnOrId) {
}
}

public String estimateGas(CallArguments args, @Nonnull String bnOrId) {

ExecutionBlockRetriever.Result result = executionBlockRetriever.retrieveExecutionBlock(bnOrId);
public String estimateGas(CallArgumentsParam args, @Nonnull BlockIdentifierParam bnOrId) {
ExecutionBlockRetriever.Result result = executionBlockRetriever.retrieveExecutionBlock(bnOrId.getIdentifier());
Block block = result.getBlock();

String estimation = null;
try {
CallArgumentsToByteArray hexArgs = new CallArgumentsToByteArray(args);
CallArgumentsToByteArray hexArgs = new CallArgumentsToByteArray(args.toCallArguments());

TransactionExecutor executor = reversibleTransactionExecutor.estimateGas(
block,
Expand Down Expand Up @@ -198,12 +202,12 @@ protected String internalEstimateGas(ProgramResult reversibleExecutionResult) {
}

@Override
public String sendTransaction(CallArguments args) {
public String sendTransaction(CallArgumentsParam args) {
return ethModuleTransaction.sendTransaction(args);
}

@Override
public String sendRawTransaction(String rawData) {
public String sendRawTransaction(HexDataParam rawData) {
return ethModuleTransaction.sendRawTransaction(rawData);
}

Expand All @@ -216,14 +220,14 @@ public String chainId() {
return HexUtils.toJsonHex(new byte[]{chainId});
}

public String getCode(String address, String blockId) {
public String getCode(HexAddressParam address, String blockId) {
if (blockId == null) {
throw new NullPointerException();
}

String s = null;
RskAddress addr = address.getAddress();
try {
RskAddress addr = new RskAddress(address);

AccountInformationProvider accountInformationProvider = getAccountInformationProvider(blockId);

Expand All @@ -241,7 +245,7 @@ public String getCode(String address, String blockId) {
return s;
} finally {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("eth_getCode({}, {}): {}", address, blockId, s);
LOGGER.debug("eth_getCode({}, {}): {}", addr.toHexString(), blockId, s);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@

package co.rsk.rpc.modules.eth;

import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexDataParam;

public interface EthModuleTransaction {
String sendTransaction(CallArguments args);
String sendTransaction(CallArgumentsParam args);

String sendRawTransaction(String rawData);
String sendRawTransaction(HexDataParam rawData);
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@
import org.ethereum.core.TransactionPoolAddResult;
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.exception.RskJsonRpcRequestException;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexDataParam;
import org.ethereum.util.TransactionArgumentsUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import co.rsk.core.RskAddress;
import co.rsk.core.Wallet;
import co.rsk.net.TransactionGateway;
import co.rsk.util.HexUtils;

public class EthModuleTransactionBase implements EthModuleTransaction {

Expand All @@ -56,7 +57,12 @@ public EthModuleTransactionBase(Constants constants, Wallet wallet, TransactionP
}

@Override
public synchronized String sendTransaction(CallArguments args) {
public synchronized String sendTransaction(CallArgumentsParam argsParam) {
CallArguments args = argsParam.toCallArguments();

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

Account senderAccount = this.wallet.getAccount(new RskAddress(args.getFrom()));

Expand Down Expand Up @@ -97,10 +103,10 @@ public synchronized String sendTransaction(CallArguments args) {
}

@Override
public String sendRawTransaction(String rawData) {
public String sendRawTransaction(HexDataParam rawData) {
String s = null;
try {
Transaction tx = new ImmutableTransaction(HexUtils.stringHexToByteArray(rawData));
Transaction tx = new ImmutableTransaction(rawData.getRawDataBytes());

if (null == tx.getGasLimit() || null == tx.getGasPrice() || null == tx.getValue()) {
throw invalidParamError("Missing parameter, gasPrice, gas or value");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import co.rsk.net.TransactionGateway;
import org.ethereum.config.Constants;
import org.ethereum.core.TransactionPool;
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.parameters.CallArgumentsParam;

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

Expand All @@ -36,7 +36,7 @@ public EthModuleTransactionDisabled(Constants constants, TransactionPool transac
}

@Override
public String sendTransaction(CallArguments args) { // lgtm [java/non-sync-override]
public synchronized String sendTransaction(CallArgumentsParam args) { // lgtm [java/non-sync-override]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

about making this method synchronized, what is the reason for that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sonar Cloud triggered a warning asking to change it to a syncronized method because the class this one was overridden from, EthModuleTransactionBase, has the sendTransaction method as synchronized.

LOGGER.debug("eth_sendTransaction({}): {}", args, null);
throw invalidParamError("Local wallet is disabled in this node");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@
import org.ethereum.core.Blockchain;
import org.ethereum.core.TransactionPool;
import org.ethereum.db.TransactionInfo;
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.exception.RskJsonRpcRequestException;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexDataParam;
import org.ethereum.vm.program.ProgramResult;

import co.rsk.core.Wallet;
Expand Down Expand Up @@ -64,7 +65,7 @@ public EthModuleTransactionInstant(
}

@Override
public synchronized String sendTransaction(CallArguments args) {
public synchronized String sendTransaction(CallArgumentsParam args) {
try {
this.blockExecutor.setRegisterProgramResults(true);

Expand All @@ -80,7 +81,7 @@ public synchronized String sendTransaction(CallArguments args) {
}

@Override
public String sendRawTransaction(String rawData) {
public String sendRawTransaction(HexDataParam rawData) {
try {
this.blockExecutor.setRegisterProgramResults(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.nio.charset.StandardCharsets;
import java.util.Arrays;

import co.rsk.core.RskAddress;
import org.bouncycastle.util.BigIntegers;
import org.ethereum.core.Account;
import org.ethereum.crypto.ECKey;
Expand All @@ -32,7 +33,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import co.rsk.core.RskAddress;
import co.rsk.core.Wallet;
import co.rsk.util.HexUtils;

Expand All @@ -55,7 +55,9 @@ public String sign(String addr, String data) {
throw invalidParamError("Account not found");
}

return s = this.sign(data, account.getEcKey());
s = this.sign(data, account.getEcKey());

return s;
} finally {
LOGGER.debug("eth_sign({}, {}): {}", addr, data, s);
}
Expand Down
Loading