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

Implementation of EthSwap mechanism #2276

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from
Draft

Implementation of EthSwap mechanism #2276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0601629
Implmentation of EthSwap mechanism
rmoreliovlabs Mar 26, 2024
46a29e7
Add integration test
rmoreliovlabs Apr 11, 2024
2ac95f1
Tests fixes
rmoreliovlabs Apr 11, 2024
a0a4b6e
Tests fixes
rmoreliovlabs Apr 12, 2024
26b495d
Automate tests and fix SonarCloud error
rmoreliovlabs Apr 18, 2024
d9b2019
Adding ClaimTransactionInfoHolder class and covering edge cases
rmoreliovlabs May 13, 2024
f4eed87
Test fixes
rmoreliovlabs May 13, 2024
c5cd703
Fix SonarCloud error
rmoreliovlabs May 13, 2024
425a313
Implementing some suggestions from review comments
rmoreliovlabs May 15, 2024
3a94c39
Remove unused imports
rmoreliovlabs May 15, 2024
96d6c11
Refactor isClaimTxAndValid to remove txCost argument
rmoreliovlabs May 16, 2024
5b32bd0
Remove unused imports
rmoreliovlabs May 16, 2024
c1fe7d2
Refactor claim transaction validation class and implement some sugges…
rmoreliovlabs May 21, 2024
683001c
Test fixes and add placeholder hardfork
rmoreliovlabs May 22, 2024
0e46f9b
Fix sonarcloud issues
rmoreliovlabs May 22, 2024
7e801de
Add RSKIP number and refactor validate method in TxValidatorStep
rmoreliovlabs Jun 3, 2024
e8393bf
Fix integration test
rmoreliovlabs Jun 3, 2024
57a4f7f
Fix sonarcloud issues
rmoreliovlabs Jun 3, 2024
97d1d99
Removing unnecessary test
rmoreliovlabs Jun 6, 2024
72272e0
Addressing comments
rmoreliovlabs Aug 9, 2024
99712db
Addressing comments
rmoreliovlabs Aug 9, 2024
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
324 changes: 324 additions & 0 deletions rskj-core/src/integrationTest/java/co/rsk/ClaimTransactionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,324 @@
/*
* This file is part of RskJ
* Copyright (C) 2024 RSK Labs Ltd.
* (derived from ethereumJ library, Copyright (c) 2016 <ether.camp>)
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package co.rsk;

import co.rsk.core.bc.ClaimTransactionValidator;
import co.rsk.util.HexUtils;
import co.rsk.util.OkHttpClientTestFixture;
import co.rsk.util.cli.CommandLineFixture;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.squareup.okhttp.Response;
import org.ethereum.config.Constants;
import org.ethereum.core.BlockTxSignatureCache;
import org.ethereum.core.CallTransaction;
import org.ethereum.core.ReceivedTxSignatureCache;
import org.ethereum.crypto.HashUtil;
import org.ethereum.util.ByteUtil;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

import java.io.IOException;
import java.math.BigInteger;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;

public class ClaimTransactionTest {
private static final int LOCAL_PORT = 4444;

private static final CallTransaction.Function CALL_LOCK_FUNCTION = CallTransaction.Function.fromSignature(
"lock",
new String[]{"bytes32", "address", "uint256"},
new String[]{}
);
private static final CallTransaction.Function CALL_CLAIM_FUNCTION = CallTransaction.Function.fromSignature(
"claim",
new String[]{"bytes32", "uint256", "address", "uint256"},
new String[]{}
);
private final ObjectMapper objectMapper = new ObjectMapper();
private String buildLibsPath;
private String jarName;
private String databaseDir;
@TempDir
private Path tempDir;

private String[] baseArgs;
private String strBaseArgs;
private String baseJavaCmd;

private ClaimTransactionValidator claimTransactionValidator;

@BeforeEach
public void setup() throws IOException {
String projectPath = System.getProperty("user.dir");
buildLibsPath = String.format("%s/build/libs", projectPath);
String integrationTestResourcesPath = String.format("%s/src/integrationTest/resources", projectPath);
String rskConfFile = String.format("%s/integration-test-rskj.conf", integrationTestResourcesPath);
Stream<Path> pathsStream = Files.list(Paths.get(buildLibsPath));
jarName = pathsStream.filter(p -> !p.toFile().isDirectory())
.map(p -> p.getFileName().toString())
.filter(fn -> fn.endsWith("-all.jar"))
.findFirst()
.get();
Path databaseDirPath = tempDir.resolve("database");
databaseDir = databaseDirPath.toString();
baseArgs = new String[]{"--regtest"};
strBaseArgs = String.join(" ", baseArgs);
baseJavaCmd = String.format("java %s", String.format("-Drsk.conf.file=%s", rskConfFile));

claimTransactionValidator = new ClaimTransactionValidator( new BlockTxSignatureCache(new ReceivedTxSignatureCache()), Constants.regtest());
}

private Response lockTxRequest(String refundAddress, byte[] lockData, BigInteger amount) throws IOException {
String lockTxRequestContent = "[{\n" +
" \"method\": \"eth_sendTransaction\",\n" +
" \"params\": [{\n" +
" \"from\": \"" + refundAddress + "\",\n" +
" \"to\": \"0x" + Constants.regtest().getEtherSwapContractAddress() + "\",\n" +
" \"data\": \"0x" + ByteUtil.toHexString(lockData) + "\",\n" +
" \"value\": \"" + HexUtils.toQuantityJsonHex(amount.longValue()) + "\",\n" +
" \"gas\": \"0xc350\",\n" +
" \"gasPrice\": \"0x1\"\n" +
" }],\n" +
" \"id\": 1,\n" +
" \"jsonrpc\": \"2.0\"\n" +
"}]";

return OkHttpClientTestFixture.sendJsonRpcMessage(lockTxRequestContent, LOCAL_PORT);
}

private Response claimTxRequest(String claimAddress, byte[] claimData) throws IOException {
String claimTxRequestContent = "[{\n" +
" \"method\": \"eth_sendTransaction\",\n" +
" \"params\": [{\n" +
" \"from\": \"" + claimAddress + "\",\n" +
" \"to\": \"0x" + Constants.regtest().getEtherSwapContractAddress() + "\",\n" +
" \"data\": \"0x" + ByteUtil.toHexString(claimData) + "\",\n" +
" \"value\": \"0x0\",\n" +
" \"gas\": \"0xc350\",\n" +
" \"gasPrice\": \"0x1\"\n" +
" }],\n" +
" \"id\": 1,\n" +
" \"jsonrpc\": \"2.0\"\n" +
"}]";

return OkHttpClientTestFixture.sendJsonRpcMessage(claimTxRequestContent, LOCAL_PORT);
}

private Response getBalanceRequest(String address) throws IOException {
String getBalanceRequestContent = "[{\n" +
" \"method\": \"eth_getBalance\",\n" +
" \"params\": [\n" +
" \"" + address + "\",\n" +
" \"latest\"\n" +
" ],\n" +
" \"id\": 1,\n" +
" \"jsonrpc\": \"2.0\"\n" +
"}]";

return OkHttpClientTestFixture.sendJsonRpcMessage(getBalanceRequestContent, LOCAL_PORT);
}

private Response getTxReceiptRequest(String txHash) throws IOException {
String getReceiptRequestContent = "[{\n" +
" \"method\": \"eth_getTransactionReceipt\",\n" +
" \"params\": [\n" +
" \"" + txHash + "\"\n" +
" ],\n" +
" \"id\": 1,\n" +
" \"jsonrpc\": \"2.0\"\n" +
"}]";

return OkHttpClientTestFixture.sendJsonRpcMessage(getReceiptRequestContent, LOCAL_PORT);
}

@Test
void whenClaimTxIsSend_shouldShouldFailDueToLowFundsInContract() throws Exception {
String refundAddress = "0x7986b3df570230288501eea3d890bd66948c9b79";
String claimAddress = "0x8486054b907b0d79569723c761b7113736d32c5a";
byte[] preimage = "preimage".getBytes(StandardCharsets.UTF_8);
byte[] preimageHash = HashUtil.sha256(claimTransactionValidator.encodePacked(preimage));
BigInteger amount = BigInteger.valueOf(5000);

byte[] lockData = CALL_LOCK_FUNCTION.encode(
preimageHash,
claimAddress,
8000000);

byte[] claimData = CALL_CLAIM_FUNCTION.encode(
preimage,
amount,
refundAddress,
8000000);

String cmd = String.format("%s -cp %s/%s co.rsk.Start --reset %s", baseJavaCmd, buildLibsPath, jarName, strBaseArgs);
CommandLineFixture.runCommand(
cmd,
60,
TimeUnit.SECONDS,
proc -> {
try {
Response getBalanceResponse = getBalanceRequest(claimAddress);
JsonNode jsonRpcResponse = objectMapper.readTree(getBalanceResponse.body().string());
JsonNode currentBalance = jsonRpcResponse.get(0).get("result");
BigInteger balanceBigInt = new BigInteger(HexUtils.removeHexPrefix(currentBalance.asText()), 16);
Assertions.assertEquals(0, balanceBigInt.compareTo(BigInteger.ZERO));

lockTxRequest(refundAddress, lockData, amount);
TimeUnit.SECONDS.sleep(5);

claimTxRequest(claimAddress, claimData);
TimeUnit.SECONDS.sleep(5);

getBalanceResponse = getBalanceRequest(claimAddress);
jsonRpcResponse = objectMapper.readTree(getBalanceResponse.body().string());
currentBalance = jsonRpcResponse.get(0).get("result");
balanceBigInt = new BigInteger(HexUtils.removeHexPrefix(currentBalance.asText()), 16);

Assertions.assertEquals(0, balanceBigInt.compareTo(BigInteger.ZERO));
} catch (IOException | InterruptedException e) {
Assertions.fail(e);
}
}
);
}


@Test
void whenClaimTxIsSend_shouldExecuteEvenIfSenderHasNoFunds() throws Exception {
String refundAddress = "0x7986b3df570230288501eea3d890bd66948c9b79";
String claimAddress = "0x8486054b907b0d79569723c761b7113736d32c5a";
byte[] preimage = "preimage".getBytes(StandardCharsets.UTF_8);
byte[] preimageHash = HashUtil.sha256(claimTransactionValidator.encodePacked(preimage));
BigInteger amount = BigInteger.valueOf(500000);

byte[] lockData = CALL_LOCK_FUNCTION.encode(
preimageHash,
claimAddress,
8000000);

byte[] claimData = CALL_CLAIM_FUNCTION.encode(
preimage,
amount,
refundAddress,
8000000);

String cmd = String.format("%s -cp %s/%s co.rsk.Start --reset %s", baseJavaCmd, buildLibsPath, jarName, strBaseArgs);
CommandLineFixture.runCommand(
cmd,
60,
TimeUnit.SECONDS,
proc -> {
try {
Response getBalanceResponse = getBalanceRequest(claimAddress);
JsonNode jsonRpcResponse = objectMapper.readTree(getBalanceResponse.body().string());
JsonNode currentBalance = jsonRpcResponse.get(0).get("result");
BigInteger balanceBigInt = new BigInteger(HexUtils.removeHexPrefix(currentBalance.asText()), 16);
Assertions.assertEquals(0, balanceBigInt.compareTo(BigInteger.ZERO));

lockTxRequest(refundAddress, lockData, amount);
TimeUnit.SECONDS.sleep(5);

Response claimResponse = claimTxRequest(claimAddress, claimData);
TimeUnit.SECONDS.sleep(5);

jsonRpcResponse = objectMapper.readTree(claimResponse.body().string());
JsonNode claimTxHash = jsonRpcResponse.get(0).get("result");

Response getTxReceiptResponse = getTxReceiptRequest(claimTxHash.asText());
jsonRpcResponse = objectMapper.readTree(getTxReceiptResponse.body().string());
JsonNode gasUsed = jsonRpcResponse.get(0).get("result").get("gasUsed");
BigInteger expectedBalance = amount.subtract(new BigInteger(HexUtils.removeHexPrefix(gasUsed.asText()), 16));

getBalanceResponse = getBalanceRequest(claimAddress);
jsonRpcResponse = objectMapper.readTree(getBalanceResponse.body().string());
currentBalance = jsonRpcResponse.get(0).get("result");
balanceBigInt = new BigInteger(HexUtils.removeHexPrefix(currentBalance.asText()), 16);

Assertions.assertEquals(0, balanceBigInt.compareTo(expectedBalance));
} catch (IOException | InterruptedException e) {
Assertions.fail(e);
}
}
);
}

@Test
void whenClaimTxIsSentTwice_secondClaimTxShouldNotBeIncludedInMempool() throws Exception {
String refundAddress = "0x7986b3df570230288501eea3d890bd66948c9b79";
String claimAddress = "0x8486054b907b0d79569723c761b7113736d32c5a";
byte[] preimage = "preimage".getBytes(StandardCharsets.UTF_8);
byte[] preimageHash = HashUtil.sha256(claimTransactionValidator.encodePacked(preimage));
BigInteger amount = BigInteger.valueOf(500000);

byte[] lockData = CALL_LOCK_FUNCTION.encode(
preimageHash,
claimAddress,
8000000);

byte[] claimData = CALL_CLAIM_FUNCTION.encode(
preimage,
amount,
refundAddress,
8000000);

String cmd = String.format("%s -cp %s/%s co.rsk.Start --reset %s", baseJavaCmd, buildLibsPath, jarName, strBaseArgs);
CommandLineFixture.runCommand(
cmd,
60,
TimeUnit.SECONDS,
proc -> {
try {
Response getBalanceResponse = getBalanceRequest(claimAddress);
JsonNode jsonRpcResponse = objectMapper.readTree(getBalanceResponse.body().string());
JsonNode currentBalance = jsonRpcResponse.get(0).get("result");
BigInteger balanceBigInt = new BigInteger(HexUtils.removeHexPrefix(currentBalance.asText()), 16);
Assertions.assertEquals(0, balanceBigInt.compareTo(BigInteger.ZERO));

lockTxRequest(refundAddress, lockData, amount);
TimeUnit.SECONDS.sleep(5);

Response claimResponse = claimTxRequest(claimAddress, claimData);
Response duplicatedClaimResponse = claimTxRequest(claimAddress, claimData);
TimeUnit.SECONDS.sleep(5);

jsonRpcResponse = objectMapper.readTree(claimResponse.body().string());
JsonNode claimTxHash = jsonRpcResponse.get(0).get("result");

Assertions.assertNotNull(claimTxHash);

jsonRpcResponse = objectMapper.readTree(duplicatedClaimResponse.body().string());
JsonNode duplicatedClaimTxHash = jsonRpcResponse.get(0).get("result");

Assertions.assertNull(duplicatedClaimTxHash);
} catch (IOException | InterruptedException e) {
Assertions.fail(e);
}
}
);
}
}
4 changes: 3 additions & 1 deletion rskj-core/src/main/java/co/rsk/RskContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,9 @@ public synchronized BlockExecutor getBlockExecutor() {
blockExecutor = new BlockExecutor(
getRskSystemProperties().getActivationConfig(),
getRepositoryLocator(),
getTransactionExecutorFactory()
getTransactionExecutorFactory(),
getRskSystemProperties().getNetworkConstants(),
getBlockTxSignatureCache()
);
}

Expand Down
18 changes: 17 additions & 1 deletion rskj-core/src/main/java/co/rsk/core/bc/BlockExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import co.rsk.metrics.profilers.Profiler;
import co.rsk.metrics.profilers.ProfilerFactory;
import com.google.common.annotations.VisibleForTesting;
import org.ethereum.config.Constants;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ConsensusRule;
import org.ethereum.core.*;
Expand Down Expand Up @@ -62,13 +63,18 @@ public class BlockExecutor {
private final Map<Keccak256, ProgramResult> transactionResults = new HashMap<>();
private boolean registerProgramResults;

private final ClaimTransactionValidator claimTransactionValidator;

public BlockExecutor(
ActivationConfig activationConfig,
RepositoryLocator repositoryLocator,
TransactionExecutorFactory transactionExecutorFactory) {
TransactionExecutorFactory transactionExecutorFactory,
Constants constants,
SignatureCache signatureCache) {
this.repositoryLocator = repositoryLocator;
this.transactionExecutorFactory = transactionExecutorFactory;
this.activationConfig = activationConfig;
this.claimTransactionValidator = new ClaimTransactionValidator(signatureCache, constants);
}

/**
Expand Down Expand Up @@ -282,6 +288,16 @@ private BlockResult executeInternal(
for (Transaction tx : block.getTransactionsList()) {
logger.trace("apply block: [{}] tx: [{}] ", block.getNumber(), i);

if (claimTransactionValidator.isFeatureActive(activationConfig.forBlock(block.getNumber()))) {
if(claimTransactionValidator.isClaimTx(tx)
&& !claimTransactionValidator.hasLockedFunds(tx, track)) {
logger.warn("block: [{}] discarded claim tx: [{}], because the funds it tries to claim no longer exist in contract",
block.getNumber(),
tx.getHash());
continue;
Copy link

Choose a reason for hiding this comment

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

may cause an issue if just skipped. should be analyzed in more detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I used this approach is that it aligns with the existing logic in the BlockExecutor. Just like how invalid transactions are managed there, invalid Claim transactions are discarded if discardInvalidTxs is set to true. If not, the execution is stopped

For General Transactions, the handling is similar:

if (!acceptInvalidTransactions && !transactionExecuted) {
    if (discardInvalidTxs) {
        logger.warn("block: [{}] discarded tx: [{}]", block.getNumber(), tx.getHash());
        continue;
    } else {
        logger.warn("block: [{}] execution interrupted because of invalid tx: [{}]",
                    block.getNumber(), tx.getHash());
        profiler.stop(metric);
        return BlockResult.INTERRUPTED_EXECUTION_BLOCK_RESULT;
    }
}

Just as we discard invalid general transactions, we also discard invalid Claim transactions based on specific validation criteria. This ensures we maintain consistency in handling invalid transactions across the code
Let me know if you have any further questions!

Copy link

Choose a reason for hiding this comment

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

But Claim transactions that don't have locked funds !claimTransactionValidator.hasLockedFunds(tx, track)) would be just discarded without interrupting the execution regardless of discardInvalidTxs ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, invalid claim transactions (those without locked funds) are indeed discarded without interrupting block execution, regardless of the discardInvalidTxs value. This behavior is intentional and aligned with the design outlined in the TDD and Sergio's proposal

The logic was implemented to ensure that claim transactions, which don't meet the requirement of having locked funds, are simply discarded to prevent them from affecting the overall block processing. This ensures that the chain execution continues smoothly without being interrupted by these invalid claim transactions. The proposal specifies that invalid claims should not stop the block execution but rather be skipped to maintain stability and performance. You can find the detailed reasoning behind this in the initial Sergio's proposal document here

Happy to explain further if needed!

}
}

TransactionExecutor txExecutor = transactionExecutorFactory.newInstance(
tx,
txindex++,
Expand Down
Loading
Loading