Skip to content

Commit

Permalink
Repaired case of ugly tx processing abort on localCall==true
Browse files Browse the repository at this point in the history
  • Loading branch information
@SergioDemianLerner
SergioDemianLerner authored and fedejinich committed Nov 23, 2021
1 parent 81b7d0d commit bda349c
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 7 deletions.
5 changes: 5 additions & 0 deletions rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ public String estimateGas(CallArguments args) {
try {
ProgramResult res = callConstant(args, blockchain.getBestBlock());

// IMPORTANT: currently, due to localCall=true argument,
// res.getDeductedRefund() will always return zero. This is ok.
// However this method is prepared for the time where localCall sematic
// is changed so that it does process refunds, which is what one would expect.

// gasUsed cannot be greater than the gas passed, which should not
// be higher than the block gas limit, so we don't expect any overflow
// in these operations unless the user provides a malicius gasLimit value.
Expand Down
29 changes: 22 additions & 7 deletions rskj-core/src/main/java/org/ethereum/core/TransactionExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ private boolean transactionAddressesAreValid() {
logger.warn("Tx Included in the following block: {}", this.executionBlock);

panicProcessor.panic("invalidsignature",
String.format("Transaction %s signature not accepted: %s",
tx.getHash(), tx.getSignature()));
String.format("Transaction %s signature not accepted: %s",
tx.getHash(), tx.getSignature()));
execError(String.format("Transaction signature not accepted: %s", tx.getSignature()));

return false;
Expand Down Expand Up @@ -487,7 +487,7 @@ public TransactionReceipt getReceipt() {
receipt.setTransaction(tx);
receipt.setLogInfoList(getVMLogs());
receipt.setGasUsed(getGasUsed());
receipt.setStatus(executionError.isEmpty()?TransactionReceipt.SUCCESS_STATUS:TransactionReceipt.FAILED_STATUS);
receipt.setStatus(executionError.isEmpty() ? TransactionReceipt.SUCCESS_STATUS : TransactionReceipt.FAILED_STATUS);
}
return receipt;
}
Expand All @@ -496,6 +496,15 @@ public TransactionReceipt getReceipt() {
private void finalization() {
// RSK if local call gas balances must not be changed
if (localCall) {
// This should change in the future.
// It's preferable that if localCall==true it continues executing normally,
// performing the expected commit() and returning the correct program result.
// Now it will have invalid deletedAccounts data.
// It's better that in this case the track used is a cache that does not
// allow flush, so that guarantees no side effects.

// Informing the CallWithValue is required for gas estimation
informOfCallWithValuePerformed();
return;
}

Expand All @@ -516,15 +525,14 @@ private void finalization() {
.logs(notRejectedLogInfos)
.result(result.getHReturn());


// Accumulate refunds for suicides
result.addFutureRefund(GasCost.multiply(result.getDeleteAccounts().size(), GasCost.SUICIDE_REFUND));
// The actual gas subtracted is equal to half of the future refund
long gasRefund = Math.min(result.getFutureRefund(), result.getGasUsed() / 2);
result.addDeductedRefund(gasRefund);

if ((program != null) && (program.getCallWithValuePerformed())) {
result.markCallWithValuePerformed();
}
informOfCallWithValuePerformed();

mEndGas = activations.isActive(ConsensusRule.RSKIP136) ?
GasCost.add(mEndGas, gasRefund) :
Expand Down Expand Up @@ -552,7 +560,7 @@ private void finalization() {
Coin summaryFee = summary.getFee();

//TODO: REMOVE THIS WHEN THE LocalBLockTests starts working with REMASC
if(enableRemasc) {
if (enableRemasc) {
logger.trace("Adding fee to remasc contract account");
track.addBalance(PrecompiledContracts.REMASC_ADDR, summaryFee);
} else {
Expand All @@ -573,6 +581,13 @@ private void finalization() {
logger.trace("tx finalization done");
}

public void informOfCallWithValuePerformed() {
if ((program != null) && (program.getCallWithValuePerformed())) {
result.markCallWithValuePerformed();
}
}


/**
* This extracts the trace to an object in memory.
* Refer to {@link org.ethereum.vm.VMUtils#saveProgramTraceFile} for a way to saving the trace to a file.
Expand Down
191 changes: 191 additions & 0 deletions rskj-core/src/test/java/co/rsk/mine/TransactionModuleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import co.rsk.net.TransactionGateway;
import co.rsk.peg.BridgeSupportFactory;
import co.rsk.peg.RepositoryBtcBlockStoreWithCache;
import co.rsk.peg.performance.PrecompiledContractPerformanceTestCase;
import co.rsk.rpc.ExecutionBlockRetriever;
import co.rsk.rpc.Web3RskImpl;
import co.rsk.rpc.modules.debug.DebugModule;
Expand All @@ -44,8 +45,11 @@
import co.rsk.trie.TrieStore;
import co.rsk.validators.BlockUnclesValidationRule;
import co.rsk.validators.ProofOfWorkRule;
import org.bouncycastle.util.BigIntegers;
import org.bouncycastle.util.encoders.Hex;
import org.ethereum.core.*;
import org.ethereum.crypto.ECKey;
import org.ethereum.crypto.HashUtil;
import org.ethereum.crypto.Keccak256Helper;
import org.ethereum.datasource.HashMapDB;
import org.ethereum.db.BlockStore;
Expand All @@ -64,13 +68,16 @@
import org.ethereum.util.BuildInfo;
import org.ethereum.util.ByteUtil;
import org.ethereum.vm.PrecompiledContracts;
import org.ethereum.vm.program.invoke.ProgramInvokeFactoryImpl;
import org.junit.Assert;
import org.junit.Test;
import org.mockito.Mockito;

import java.math.BigInteger;
import java.time.Clock;

import static org.hamcrest.CoreMatchers.is;

public class TransactionModuleTest {
private final TestSystemProperties config = new TestSystemProperties();
private final BlockFactory blockFactory = new BlockFactory(config.getActivationConfig());
Expand Down Expand Up @@ -219,6 +226,83 @@ public void sendRawTransactionWithoutAutoMining() {
Assert.assertEquals(txHash, transactionPool.getPendingTransactions().get(0).getHash().toJsonString());
}

@Test
public void testGasEstimation() {
World world = new World();
BlockChainImpl blockchain = world.getBlockChain();

TrieStore trieStore = world.getTrieStore();

RepositoryLocator repositoryLocator = world.getRepositoryLocator();
RepositorySnapshot repository = repositoryLocator.snapshotAt(blockchain.getBestBlock().getHeader());
//RskAddress c = new RskAddress("0101010101010101010101010101010101010101");
//Repository repo2 = repository.startTracking();
//.setupContract(c);

BlockStore blockStore = world.getBlockStore();

TransactionPool transactionPool = new TransactionPoolImpl(config, repositoryLocator, blockStore, blockFactory, null, buildTransactionExecutorFactory(blockStore, null), 10, 100);

Web3Impl web3 = createEnvironment(blockchain, null, trieStore, transactionPool, blockStore, true);
RskAddress srcAddr = new RskAddress(ECKey.fromPrivate(Keccak256Helper.keccak256("cow".getBytes())).getAddress());

// Create the transaction that creates the destination contract
String tx = sendContractCreationTransaction(srcAddr, web3, repository);
// Compute contract destination address

BigInteger nonce = repository.getAccountState(srcAddr).getNonce();
RskAddress contractAddress = new RskAddress(HashUtil.calcNewAddr(srcAddr.getBytes(), nonce.toByteArray()));
int gasLimit = 5000000; // start with 5M
int consumed = checkEstimateGas(callCallWithValue, 33557,gasLimit,srcAddr,contractAddress,web3, repository);
// Now that I know the estimation, call again using the estimated value
// it should not fail. We set the gasLimit to the expected value plus 1 to
// differentiate betweem OOG and success.
int consumed2 = checkEstimateGas(callCallWithValue,33557,consumed+1, srcAddr,contractAddress,web3, repository);
Assert.assertEquals(consumed,consumed2);

consumed = checkEstimateGas(callUnfill, 46942,
gasLimit,srcAddr,contractAddress,web3, repository);
consumed2 = checkEstimateGas(callUnfill, 46942,
consumed+1,srcAddr,contractAddress,web3, repository);
Assert.assertEquals(consumed,consumed2);
}

// We check that the transaction does not fail!
// This is clearly missing for estimateGas. It should return a tuple
// (success,gasConsumed)
public int checkEstimateGas(int method,int expectedValue,int gasLimit,
RskAddress srcAddr,RskAddress contractAddress,Web3Impl web3,RepositorySnapshot repository) {
// If expected value given is the gasLimit we must fail because estimateGas cannot
// differentiate between transaction failure (OOG) and success.
Assert.assertNotEquals(expectedValue,gasLimit);

Web3.CallArguments args = getContractCallTransactionParameters(method,gasLimit,srcAddr,contractAddress,web3, repository);
String gas = web3.eth_estimateGas(args);
byte[] gasReturnedBytes = Hex.decode(gas.substring("0x".length()));
BigInteger gasReturned =BigIntegers.fromUnsignedByteArray(gasReturnedBytes);
int gasReturnedInt = gasReturned.intValueExact();
Assert.assertNotEquals(gasReturnedInt,gasLimit);
Assert.assertEquals(gasReturnedInt, expectedValue);
return gasReturnedInt;
}

private String getGasEstimationTransactionRawData(Web3Impl web3,int gasLimit) {
Account sender = new AccountBuilder().name("cow").build();
Account receiver = new AccountBuilder().name("addr2").build();

Transaction tx = new TransactionBuilder()
.sender(sender)
.receiver(receiver)
.gasPrice(BigInteger.valueOf(8))
.gasLimit(BigInteger.valueOf(gasLimit))
.value(BigInteger.valueOf(7))
.nonce(0)
.build();

String rawData = Hex.toHexString(tx.getEncoded());
return rawData;
}

private String sendRawTransaction(Web3Impl web3) {
Account sender = new AccountBuilder().name("cow").build();
Account receiver = new AccountBuilder().name("addr2").build();
Expand Down Expand Up @@ -249,12 +333,119 @@ private Transaction getTransactionFromBlockWhichWasSend(BlockChainImpl blockchai
return txInBlock;
}

private String sendContractCreationTransaction(RskAddress srcaddr,Web3Impl web3, RepositorySnapshot repository) {

Web3.CallArguments args = getContractCreationTransactionParameters(srcaddr,web3, repository);

return web3.eth_sendTransaction(args);
}

private String sendTransaction(Web3Impl web3, RepositorySnapshot repository) {
CallArguments args = getTransactionParameters(web3, repository);

return web3.eth_sendTransaction(args);
}

////////////////////////////////////////////////
// This is the contract that is being created
// by getContractCreationTransactionParameters
/* pragma solidity ^0.5.8;
contract GasExactimation {
mapping(uint256 => uint256) filled;
constructor() public payable {
fill();
}
function fill() public {
filled[1] = 1;
filled[2] = 2;
filled[3] = 3;
filled[4] = 4;
filled[5] = 5;
}
function unfill() public {
filled[1] = 0;
filled[2] = 0;
filled[3] = 0;
filled[4] = 0;
filled[5] = 0;
}
function getFilled() public view returns(uint) {
return filled[1];
}
function () external payable {
}
function callWithValue() public payable {
address(this).transfer(100);
}
function callAndUnfill() public {
address(this).transfer(100);
unfill();
}
}
//////////////////////////////// */
private Web3.CallArguments getContractCreationTransactionParameters(
RskAddress addr1,Web3Impl web3, RepositorySnapshot repository) {

BigInteger value = BigInteger.valueOf(7);
BigInteger gasPrice = BigInteger.valueOf(8);
BigInteger gasLimit = BigInteger.valueOf(500000);
String data = "0x608060405261001261001760201b60201c565b610096565b6001600080600181526020019081526020016000208190555060026000806002815260200190815260200160002081905550600360008060038152602001908152602001600020819055506004600080600481526020019081526020016000208190555060056000806005815260200190815260200160002081905550565b6102a7806100a56000396000f3fe60806040526004361061004a5760003560e01c8063742392c51461004c5780639a1e180f14610077578063c3cefd361461008e578063d9c55ce114610098578063dfd2d2c2146100af575b005b34801561005857600080fd5b506100616100c6565b6040518082815260200191505060405180910390f35b34801561008357600080fd5b5061008c6100e1565b005b610096610133565b005b3480156100a457600080fd5b506100ad61017d565b005b3480156100bb57600080fd5b506100c46101fc565b005b60008060006001815260200190815260200160002054905090565b3073ffffffffffffffffffffffffffffffffffffffff166108fc60649081150290604051600060405180830381858888f19350505050158015610128573d6000803e3d6000fd5b506101316101fc565b565b3073ffffffffffffffffffffffffffffffffffffffff166108fc60649081150290604051600060405180830381858888f1935050505015801561017a573d6000803e3d6000fd5b50565b6001600080600181526020019081526020016000208190555060026000806002815260200190815260200160002081905550600360008060038152602001908152602001600020819055506004600080600481526020019081526020016000208190555060056000806005815260200190815260200160002081905550565b600080600060018152602001908152602001600020819055506000806000600281526020019081526020016000208190555060008060006003815260200190815260200160002081905550600080600060048152602001908152602001600020819055506000806000600581526020019081526020016000208190555056fea165627a7a72305820545214f6b1b9d3a4928fb579044851ba06a9ff28b7d588b175847b7116d7b7c00029";

Web3.CallArguments args = new Web3.CallArguments();
args.from = TypeConverter.toJsonHex(addr1.getBytes());
args.to = ""; // null?
args.data = data;
args.gas = TypeConverter.toQuantityJsonHex(gasLimit);
args.gasPrice = TypeConverter.toQuantityJsonHex(gasPrice);
args.value = value.toString();
args.nonce = repository.getAccountState(addr1).getNonce().toString();

return args;
}
public final int callUnfill =0;
public final int callCallWithValue = 1;

private Web3.CallArguments getContractCallTransactionParameters(
int methodToCall,int gasLimitInt,RskAddress addr1,RskAddress destContract,Web3Impl web3, RepositorySnapshot repository) {

BigInteger value;
BigInteger gasPrice = BigInteger.valueOf(8);
BigInteger gasLimit = BigInteger.valueOf(gasLimitInt);
String data ="";
byte[] encoded = null;
if (methodToCall==callUnfill) {
value = BigInteger.valueOf(0);
CallTransaction.Function func = CallTransaction.Function.fromSignature(
"unfill",
new String[]{},
new String[]{});

encoded = func.encode();
} else {
value = BigInteger.valueOf(101);
CallTransaction.Function func = CallTransaction.Function.fromSignature(
"callWithValue",
new String[]{},
new String[]{});

encoded = func.encode();
}
data = Hex.toHexString(encoded);
Web3.CallArguments args = new Web3.CallArguments();
args.from = TypeConverter.toJsonHex(addr1.getBytes());
args.to = TypeConverter.toJsonHex(destContract.getBytes());
args.data = data;
args.gas = TypeConverter.toQuantityJsonHex(gasLimit);
args.gasPrice = TypeConverter.toQuantityJsonHex(gasPrice);
args.value = value.toString();
args.nonce = repository.getAccountState(addr1).getNonce().toString();

return args;
}

private CallArguments getTransactionParameters(Web3Impl web3, RepositorySnapshot repository) {
RskAddress addr1 = new RskAddress(ECKey.fromPrivate(Keccak256Helper.keccak256("cow".getBytes())).getAddress());
String addr2 = web3.personal_newAccountWithSeed("addr2");
Expand Down

0 comments on commit bda349c

Please sign in to comment.