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.

Sign up for GitHub

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

Jump to bottom

Update methods argument classes #2130

Merged
merged 1 commit into from
Sep 12, 2023
Merged

Update methods argument classes #2130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions rskj-core/src/main/java/co/rsk/rpc/Web3EthModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
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;
Expand Down Expand Up @@ -84,9 +85,7 @@ default String eth_chainId() {

String eth_getBalance(HexAddressParam address) throws Exception;

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

String eth_getStorageAt(String address, String storageIdx, String blockId) throws Exception;
String eth_getStorageAt(HexAddressParam address, HexNumberParam storageIdx, BlockRefParam blockRefParam) throws Exception;

String eth_getTransactionCount(HexAddressParam address, BlockRefParam blockRefParam) throws Exception;

Expand Down
15 changes: 9 additions & 6 deletions rskj-core/src/main/java/co/rsk/rpc/modules/personal/PersonalModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,27 @@

package co.rsk.rpc.modules.personal;

import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexAddressParam;
import org.ethereum.rpc.parameters.HexDurationParam;
import org.ethereum.rpc.parameters.HexKeyParam;

public interface PersonalModule {
String dumpRawKey(String address) throws Exception;
String dumpRawKey(HexAddressParam address) throws Exception;

String importRawKey(String key, String passphrase);
String importRawKey(HexKeyParam key, String passphrase);

void init();

String[] listAccounts();

boolean lockAccount(String address);
boolean lockAccount(HexAddressParam address);

String newAccountWithSeed(String seed);

String newAccount(String passphrase);

String sendTransaction(CallArguments args, String passphrase) throws Exception;
String sendTransaction(CallArgumentsParam args, String passphrase) throws Exception;

boolean unlockAccount(String address, String passphrase, String duration);
boolean unlockAccount(HexAddressParam address, String passphrase, HexDurationParam duration);
}
15 changes: 9 additions & 6 deletions rskj-core/src/main/java/co/rsk/rpc/modules/personal/PersonalModuleWalletDisabled.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

package co.rsk.rpc.modules.personal;

import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.exception.DisabledWalletException;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexAddressParam;
import org.ethereum.rpc.parameters.HexDurationParam;
import org.ethereum.rpc.parameters.HexKeyParam;

public class PersonalModuleWalletDisabled implements PersonalModule {
@Override
Expand All @@ -45,27 +48,27 @@ public String[] listAccounts() {
}

@Override
public String importRawKey(String key, String passphrase) {
public String importRawKey(HexKeyParam key, String passphrase) {
throw new DisabledWalletException();
}

@Override
public String sendTransaction(CallArguments args, String passphrase) {
public String sendTransaction(CallArgumentsParam args, String passphrase) {
throw new DisabledWalletException();
}

@Override
public boolean unlockAccount(String address, String passphrase, String duration) {
public boolean unlockAccount(HexAddressParam address, String passphrase, HexDurationParam duration) {
throw new DisabledWalletException();
}

@Override
public boolean lockAccount(String address) {
public boolean lockAccount(HexAddressParam address) {
throw new DisabledWalletException();
}

@Override
public String dumpRawKey(String address) {
public String dumpRawKey(HexAddressParam address) {
throw new DisabledWalletException();
}
}
29 changes: 18 additions & 11 deletions rskj-core/src/main/java/co/rsk/rpc/modules/personal/PersonalModuleWalletEnabled.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,17 @@
import org.ethereum.facade.Ethereum;
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.exception.RskJsonRpcRequestException;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexAddressParam;
import org.ethereum.rpc.parameters.HexDurationParam;
import org.ethereum.rpc.parameters.HexKeyParam;
import org.ethereum.util.ByteUtil;
import org.ethereum.util.TransactionArgumentsUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Arrays;
import java.util.Optional;

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

Expand Down Expand Up @@ -121,8 +126,9 @@ public String[] listAccounts() {
}

@Override
public String importRawKey(String key, String passphrase) {
public String importRawKey(HexKeyParam keyParam, String passphrase) {
String s = null;
String key = Optional.ofNullable(keyParam).map(HexKeyParam::getHexKey).orElse(null);
try {
if (key != null && key.startsWith("0x")) {
key = key.substring(2);
Expand All @@ -136,8 +142,9 @@ public String importRawKey(String key, String passphrase) {
}

@Override
public String sendTransaction(CallArguments args, String passphrase) throws Exception {
public String sendTransaction(CallArgumentsParam argsParam, String passphrase) throws Exception {
String s = null;
CallArguments args = argsParam.toCallArguments();
try {
return s = sendTransaction(args, getAccount(args.getFrom(), passphrase));
} finally {
Expand All @@ -146,15 +153,15 @@ public String sendTransaction(CallArguments args, String passphrase) throws Exce
}

@Override
public boolean unlockAccount(String address, String passphrase, String duration) {
return unlockAccount(new RskAddress(address), passphrase, duration);
public boolean unlockAccount(HexAddressParam address, String passphrase, HexDurationParam duration) {
return unlockAccount(address.getAddress(), passphrase, duration);
}

private boolean unlockAccount(RskAddress addr, String passphrase, String duration) {
private boolean unlockAccount(RskAddress addr, String passphrase, HexDurationParam duration) {
long dur = (long) 1000 * 60 * 30;
if (duration != null && duration.length() > 0) {
if (duration != null && duration.getDuration() != null) {
try {
dur = convertFromJsonHexToLong(duration);
dur = duration.getDuration();
} catch (Exception e) {
throw invalidParamError("Can't parse duration param", e);
}
Expand All @@ -164,15 +171,15 @@ private boolean unlockAccount(RskAddress addr, String passphrase, String duratio
}

@Override
public boolean lockAccount(String address) {
return this.wallet.lockAccount(new RskAddress(address));
public boolean lockAccount(HexAddressParam address) {
return this.wallet.lockAccount(address.getAddress());
}

@Override
public String dumpRawKey(String address) throws Exception {
public String dumpRawKey(HexAddressParam address) throws Exception {
String s = null;
try {
Account account = wallet.getAccount(new RskAddress(convertFromJsonHexToHex(address)));
Account account = wallet.getAccount(address.getAddress());
if (account == null) {
throw new Exception("Address private key is locked or could not be found in this node");
}
Expand Down
14 changes: 9 additions & 5 deletions rskj-core/src/main/java/org/ethereum/rpc/Web3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
import co.rsk.rpc.Web3TxPoolModule;
import co.rsk.scoring.PeerScoringInformation;
import co.rsk.scoring.PeerScoringReputationSummary;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexAddressParam;
import org.ethereum.rpc.parameters.HexDurationParam;
import org.ethereum.rpc.parameters.HexKeyParam;

@java.lang.SuppressWarnings("squid:S100")
public interface Web3 extends InternalService, Web3TxPoolModule, Web3EthModule, Web3EvmModule, Web3MnrModule, Web3DebugModule, Web3TraceModule, Web3RskModule {
Expand All @@ -54,11 +58,11 @@ public interface Web3 extends InternalService, Web3TxPoolModule, Web3EthModule,
String personal_newAccountWithSeed(String seed);
String personal_newAccount(String passphrase);
String[] personal_listAccounts();
String personal_importRawKey(String key, String passphrase);
String personal_sendTransaction(CallArguments transactionArgs, String passphrase) throws Exception;
boolean personal_unlockAccount(String key, String passphrase, String duration);
boolean personal_lockAccount(String key);
String personal_dumpRawKey(String address) throws Exception;
String personal_importRawKey(HexKeyParam key, String passphrase);
String personal_sendTransaction(CallArgumentsParam transactionArgs, String passphrase) throws Exception;
boolean personal_unlockAccount(HexAddressParam key, String passphrase, HexDurationParam duration);
boolean personal_lockAccount(HexAddressParam key);
String personal_dumpRawKey(HexAddressParam address) throws Exception;

void sco_banAddress(String address);
void sco_unbanAddress(String address);
Expand Down
30 changes: 20 additions & 10 deletions rskj-core/src/main/java/org/ethereum/rpc/Web3Impl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@
import org.ethereum.rpc.parameters.BlockRefParam;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexAddressParam;
import org.ethereum.rpc.parameters.HexDurationParam;
import org.ethereum.rpc.parameters.HexIndexParam;
import org.ethereum.rpc.parameters.HexKeyParam;
import org.ethereum.rpc.parameters.HexNumberParam;
import org.ethereum.rpc.parameters.TxHashParam;
import org.ethereum.util.BuildInfo;
import org.ethereum.vm.DataWord;
Expand Down Expand Up @@ -458,22 +461,29 @@ public String eth_getBalance(HexAddressParam address) {
}

@Override
public String eth_getStorageAt(String address, String storageIdx, Map<String, String> blockRef) {
public String eth_getStorageAt(HexAddressParam address, HexNumberParam storageIdx, BlockRefParam blockRefParam) {
if(blockRefParam.getIdentifier() != null) {
return this.eth_getStorageAt(address, storageIdx, blockRefParam.getIdentifier());
} else {
return this.eth_getStorageAt(address, storageIdx, blockRefParam.getInputs());
}
}

private String eth_getStorageAt(HexAddressParam address, HexNumberParam storageIdx, Map<String, String> blockRef) {
return invokeByBlockRef(blockRef, blockNumber -> this.eth_getStorageAt(address, storageIdx, blockNumber));
}

@Override
public String eth_getStorageAt(String address, String storageIdx, String blockId) {
private String eth_getStorageAt(HexAddressParam address, HexNumberParam storageIdx, String blockId) {
String s = null;

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

AccountInformationProvider accountInformationProvider =
web3InformationRetriever.getInformationProvider(blockId);

DataWord sv = accountInformationProvider
.getStorageValue(addr, DataWord.valueOf(HexUtils.strHexOrStrNumberToByteArray(storageIdx)));
.getStorageValue(addr, DataWord.valueOf(HexUtils.strHexOrStrNumberToByteArray(storageIdx.getHexNumber())));

if (sv == null) {
s = "0x0";
Expand Down Expand Up @@ -1071,12 +1081,12 @@ public String personal_newAccount(String passphrase) {
}

@Override
public String personal_importRawKey(String key, String passphrase) {
public String personal_importRawKey(HexKeyParam key, String passphrase) {
return personalModule.importRawKey(key, passphrase);
}

@Override
public String personal_dumpRawKey(String address) throws Exception {
public String personal_dumpRawKey(HexAddressParam address) throws Exception {
return personalModule.dumpRawKey(address);
}

Expand All @@ -1086,17 +1096,17 @@ public String[] personal_listAccounts() {
}

@Override
public String personal_sendTransaction(CallArguments args, String passphrase) throws Exception {
public String personal_sendTransaction(CallArgumentsParam args, String passphrase) throws Exception {
return personalModule.sendTransaction(args, passphrase);
}

@Override
public boolean personal_unlockAccount(String address, String passphrase, String duration) {
public boolean personal_unlockAccount(HexAddressParam address, String passphrase, HexDurationParam duration) {
return personalModule.unlockAccount(address, passphrase, duration);
}

@Override
public boolean personal_lockAccount(String address) {
public boolean personal_lockAccount(HexAddressParam address) {
return personalModule.lockAccount(address);
}

Expand Down
6 changes: 5 additions & 1 deletion rskj-core/src/main/java/org/ethereum/rpc/parameters/HexDurationParam.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ public class HexDurationParam extends HexStringParam {
public HexDurationParam(String hexDurationStr) {
super(hexDurationStr);

this.duration = Long.parseLong(hexDurationStr.substring(2), 16);
if(hexDurationStr.isEmpty()) {
this.duration = null;
} else {
this.duration = Long.parseLong(hexDurationStr.substring(2), 16);
}
}

public Long getDuration() {
Expand Down
4 changes: 4 additions & 0 deletions rskj-core/src/main/java/org/ethereum/rpc/parameters/HexStringParam.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

public abstract class HexStringParam {
HexStringParam(String hexString) {
if(hexString.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we check if it's null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will add that.

return;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we throw an error instead if it's null or empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left that behevior to the other classes that extends from this one, like HexDurationParam, which should not throw an error if the value is null since the methods using them behave different depending on that.

}

if (!HexUtils.hasHexPrefix(hexString) || !HexUtils.isHex(hexString,2)) {
throw RskJsonRpcRequestException.invalidParamError("Invalid argument \"" + hexString + "\": param should be a hex value string.");
}
Expand Down
11 changes: 5 additions & 6 deletions rskj-core/src/test/java/co/rsk/rpc/modules/personal/PersonalModuleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,11 @@
import org.ethereum.facade.Ethereum;
import org.ethereum.rpc.CallArguments;
import org.ethereum.rpc.exception.RskJsonRpcRequestException;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.util.TransactionFactoryHelper;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class PersonalModuleTest {

private static final String PASS_FRASE = "passfrase";
Expand All @@ -54,6 +51,7 @@ void sendTransactionWithGasLimitTest() throws Exception {

// Hash of the expected transaction
CallArguments args = TransactionFactoryHelper.createArguments(sender, receiver);
CallArgumentsParam argsParam = TransactionFactoryHelper.toCallArgumentsParam(args);
Transaction tx = TransactionFactoryHelper.createTransaction(args, props.getNetworkConstants().getChainId(), wallet.getAccount(sender, PASS_FRASE));
String txExpectedResult = tx.getHash().toJsonString();

Expand All @@ -66,7 +64,7 @@ void sendTransactionWithGasLimitTest() throws Exception {
PersonalModuleWalletEnabled personalModuleWalletEnabled = new PersonalModuleWalletEnabled(props, ethereum, wallet, null);

// Hash of the actual transaction builded inside the sendTransaction
String txResult = personalModuleWalletEnabled.sendTransaction(args, PASS_FRASE);
String txResult = personalModuleWalletEnabled.sendTransaction(argsParam, PASS_FRASE);

assertEquals(txExpectedResult, txResult);
}
Expand All @@ -83,6 +81,7 @@ void sendTransactionThrowsErrorOnChainIdValidationTest() {
// Hash of the expected transaction
CallArguments args = TransactionFactoryHelper.createArguments(sender, receiver);
args.setChainId("" + ((int) props.getNetworkConstants().getChainId() - 2));
CallArgumentsParam argsParam = TransactionFactoryHelper.toCallArgumentsParam(args);

TransactionPoolAddResult transactionPoolAddResult = mock(TransactionPoolAddResult.class);
when(transactionPoolAddResult.transactionsWereAdded()).thenReturn(true);
Expand All @@ -91,7 +90,7 @@ void sendTransactionThrowsErrorOnChainIdValidationTest() {

PersonalModuleWalletEnabled personalModuleWalletEnabled = new PersonalModuleWalletEnabled(props, ethereum, wallet, null);

Assertions.assertThrows(RskJsonRpcRequestException.class, () -> personalModuleWalletEnabled.sendTransaction(args, PASS_FRASE));
Assertions.assertThrows(RskJsonRpcRequestException.class, () -> personalModuleWalletEnabled.sendTransaction(argsParam, PASS_FRASE));
}

}
7 changes: 5 additions & 2 deletions rskj-core/src/test/java/co/rsk/rpc/modules/personal/PersonalModuleWalletEnabledTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.bouncycastle.util.encoders.DecoderException;
import org.bouncycastle.util.encoders.Hex;
import org.ethereum.crypto.ECKey;
import org.ethereum.rpc.parameters.HexKeyParam;
import org.ethereum.util.ByteUtil;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -43,7 +44,8 @@ void importRawKey_KeyContains0xPrefix_OK() {
doReturn(true).when(walletMock).unlockAccount(eq(addressMock), eq(passphrase), any(Long.class));

PersonalModuleWalletEnabled personalModuleWalletEnabled = createPersonalModuleWalletEnabled(walletMock);
String result = personalModuleWalletEnabled.importRawKey(String.format("0x%s", rawKey), passphrase);
HexKeyParam hexKeyParam = new HexKeyParam(String.format("0x%s", rawKey));
String result = personalModuleWalletEnabled.importRawKey(hexKeyParam, passphrase);

verify(walletMock, times(1)).addAccountWithPrivateKey(hexDecodedKey, passphrase);
verify(walletMock, times(1)).unlockAccount(addressMock, passphrase, 1800000L);
Expand All @@ -67,7 +69,8 @@ void importRawKey_KeyDoesNotContains0xPrefix_OK() {
doReturn(true).when(walletMock).unlockAccount(eq(addressMock), eq(passphrase), any(Long.class));

PersonalModuleWalletEnabled personalModuleWalletEnabled = createPersonalModuleWalletEnabled(walletMock);
String result = personalModuleWalletEnabled.importRawKey(rawKey, passphrase);
HexKeyParam hexKeyParam = new HexKeyParam(rawKey);
String result = personalModuleWalletEnabled.importRawKey(hexKeyParam, passphrase);

verify(walletMock, times(1)).addAccountWithPrivateKey(hexDecodedKey, passphrase);
verify(walletMock, times(1)).unlockAccount(addressMock, passphrase, 1800000L);
Expand Down