Skip to content

Commit

Permalink
Merge pull request #1915 from rsksmart/eth-call-fix
Browse files Browse the repository at this point in the history 
Fixed issue with eth_call triggered for pending block
  • Loading branch information
@Vovchyk
Vovchyk authored Nov 10, 2022
2 parents 4533383 + 45a7a49 commit d440fb8
Show file tree
Hide file tree
Showing 17 changed files with 425 additions and 241 deletions.
7 changes: 4 additions & 3 deletions rskj-core/src/main/java/co/rsk/RskContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,8 @@ public synchronized List<InternalService> buildInternalServices() {
getBlocksBloomStore(),
getStateRootsStore()));

internalServices.add(getExecutionBlockRetriever());

return Collections.unmodifiableList(internalServices);
}

Expand Down Expand Up @@ -2081,10 +2083,9 @@ private GasLimitCalculator getGasLimitCalculator() {
private ExecutionBlockRetriever getExecutionBlockRetriever() {
if (executionBlockRetriever == null) {
executionBlockRetriever = new ExecutionBlockRetriever(
getMiningMainchainView(),
getBlockchain(),
getMinerServer(),
getBlockToMineBuilder()
getBlockToMineBuilder(),
getCompositeEthereumListener()
);
}

Expand Down
7 changes: 2 additions & 5 deletions rskj-core/src/main/java/co/rsk/core/ReversibleTransactionExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public ProgramResult executeTransaction(
byte[] value,
byte[] data,
RskAddress fromAddress) {
return executeTransaction_workaround(
return executeTransaction(
repositoryLocator.snapshotAt(executionBlock.getHeader()),
executionBlock,
coinbase,
Expand All @@ -79,10 +79,7 @@ public ProgramResult executeTransaction(
);
}



@Deprecated
public ProgramResult executeTransaction_workaround(
public ProgramResult executeTransaction(
RepositorySnapshot snapshot,
Block executionBlock,
RskAddress coinbase,
Expand Down
42 changes: 33 additions & 9 deletions rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nonnull;
import java.math.BigInteger;
import java.util.*;

Expand Down Expand Up @@ -101,6 +102,24 @@ public BlockToMineBuilder(
this.minerUtils = minerUtils;
}

/**
* Creates a pending block based on the parent block header. Pending block is temporary, not connected to the chain and
* includes txs from the mempool.
*
* @param parentHeader block header a "pending" block is based on.
*
* @return "pending" block result.
* */
@Nonnull
public BlockResult buildPending(@Nonnull BlockHeader parentHeader) {
Coin minimumGasPrice = minimumGasPriceCalculator.calculate(parentHeader.getMinimumGasPrice());
List<BlockHeader> uncles = getUnclesHeaders(parentHeader);
List<Transaction> txs = getTransactions(new ArrayList<>(), parentHeader, minimumGasPrice);
Block newBlock = createBlock(Collections.singletonList(parentHeader), uncles, txs, minimumGasPrice, null);

return executor.executeAndFill(newBlock, parentHeader);
}

/**
* Creates a new block to mine based on the previous mainchain blocks.
*
Expand All @@ -109,26 +128,31 @@ public BlockToMineBuilder(
*/
public BlockResult build(List<BlockHeader> mainchainHeaders, byte[] extraData) {
BlockHeader newBlockParentHeader = mainchainHeaders.get(0);
List<BlockHeader> uncles = getUnclesHeaders(newBlockParentHeader);

Coin minimumGasPrice = minimumGasPriceCalculator.calculate(newBlockParentHeader.getMinimumGasPrice());

final List<Transaction> txsToRemove = new ArrayList<>();
final List<Transaction> txs = getTransactions(txsToRemove, newBlockParentHeader, minimumGasPrice);
final Block newBlock = createBlock(mainchainHeaders, uncles, txs, minimumGasPrice, extraData);

removePendingTransactions(txsToRemove);
return executor.executeAndFill(newBlock, newBlockParentHeader);
}

private List<BlockHeader> getUnclesHeaders(BlockHeader newBlockParentHeader) {
List<BlockHeader> uncles = FamilyUtils.getUnclesHeaders(
blockStore,
newBlockParentHeader.getNumber() + 1,
newBlockParentHeader.getHash(),
miningConfig.getUncleGenerationLimit()
);


if (uncles.size() > miningConfig.getUncleListLimit()) {
uncles = uncles.subList(0, miningConfig.getUncleListLimit());
}

Coin minimumGasPrice = minimumGasPriceCalculator.calculate(newBlockParentHeader.getMinimumGasPrice());

final List<Transaction> txsToRemove = new ArrayList<>();
final List<Transaction> txs = getTransactions(txsToRemove, newBlockParentHeader, minimumGasPrice);
final Block newBlock = createBlock(mainchainHeaders, uncles, txs, minimumGasPrice, extraData);

removePendingTransactions(txsToRemove);
return executor.executeAndFill(newBlock, newBlockParentHeader);
return uncles;
}

private List<Transaction> getTransactions(List<Transaction> txsToRemove, BlockHeader parentHeader, Coin minGasPrice) {
Expand Down
161 changes: 110 additions & 51 deletions rskj-core/src/main/java/co/rsk/rpc/ExecutionBlockRetriever.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,78 +18,57 @@

package co.rsk.rpc;

import co.rsk.core.Coin;
import co.rsk.config.InternalService;
import co.rsk.core.bc.BlockResult;
import co.rsk.core.bc.MiningMainchainView;
import co.rsk.mine.BlockToMineBuilder;
import co.rsk.mine.MinerServer;
import co.rsk.trie.Trie;
import com.google.common.annotations.VisibleForTesting;
import org.ethereum.core.Block;
import org.ethereum.core.BlockHeader;
import org.ethereum.core.Blockchain;
import org.ethereum.core.Transaction;
import org.ethereum.core.TransactionReceipt;
import org.ethereum.listener.CompositeEthereumListener;
import org.ethereum.listener.EthereumListener;
import org.ethereum.listener.EthereumListenerAdapter;
import org.ethereum.util.Utils;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicReference;

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

/**
* Encapsulates the logic to retrieve or create an execution block
* for Web3 calls.
*/
public class ExecutionBlockRetriever {
public class ExecutionBlockRetriever implements InternalService {
private static final String LATEST_ID = "latest";
private static final String PENDING_ID = "pending";

private final MiningMainchainView miningMainchainView;
private final Object pendingBlockLock = new Object();
private final Blockchain blockchain;
private final MinerServer minerServer;
private final BlockToMineBuilder builder;
private final CompositeEthereumListener emitter;
private final EthereumListener listener = new CachedResultCleaner();

@Nullable
private Block cachedBlock;
@Nullable
private BlockResult cachedResult;
private final AtomicReference<Result> cachedPendingBlockResult = new AtomicReference<>();

public ExecutionBlockRetriever(MiningMainchainView miningMainchainView,
Blockchain blockchain,
MinerServer minerServer,
BlockToMineBuilder builder) {
this.miningMainchainView = miningMainchainView;
public ExecutionBlockRetriever(Blockchain blockchain, BlockToMineBuilder builder, CompositeEthereumListener emitter) {
this.blockchain = blockchain;
this.minerServer = minerServer;
this.builder = builder;
this.emitter = emitter;
}

public BlockResult retrieveExecutionBlock(String bnOrId) {
public Result retrieveExecutionBlock(String bnOrId) {
if (LATEST_ID.equals(bnOrId)) {
return newBlockResult(blockchain.getBestBlock());
return Result.ofBlock(blockchain.getBestBlock());
}

if (PENDING_ID.equals(bnOrId)) {
Optional<Block> latestBlock = minerServer.getLatestBlock();
if (latestBlock.isPresent()) {
return newBlockResult(latestBlock.get());
}

Block bestBlock = blockchain.getBestBlock();
if (cachedBlock == null || !bestBlock.isParentOf(cachedBlock)) {

// If the miner server is not running there is no one to update the mining mainchain view,
// thus breaking eth_call with 'pending' parameter
//
// This is just a provisional fix not intended to remain in the long run
if (!minerServer.isRunning()) {
miningMainchainView.addBest(bestBlock.getHeader());
}

List<BlockHeader> mainchainHeaders = miningMainchainView.get();
cachedResult = builder.build(mainchainHeaders, null);
}

return cachedResult;
return getPendingBlockResult();
}

// Is the block specifier either a hexadecimal or decimal number?
Expand All @@ -106,7 +85,7 @@ public BlockResult retrieveExecutionBlock(String bnOrId) {
if (executionBlock == null) {
throw invalidParamError(String.format("Invalid block number %d", executionBlockNumber.get()));
}
return newBlockResult(executionBlock);
return Result.ofBlock(executionBlock);
}

// If we got here, the specifier given is unsupported
Expand All @@ -116,14 +95,94 @@ public BlockResult retrieveExecutionBlock(String bnOrId) {
bnOrId));
}

private static BlockResult newBlockResult(Block block) {
return new BlockResult(
block,
Collections.emptyList(),
Collections.emptyList(),
0,
Coin.ZERO,
null
);
@Nonnull
private Result getPendingBlockResult() {
Block bestBlock = blockchain.getBestBlock();

// optimistic check without the lock
Result result = getCachedResultFor(bestBlock);
if (result != null) {
return result;
}

synchronized (pendingBlockLock) {
// build a new pending block, but before that just in case check if one hasn't been built while being locked
bestBlock = blockchain.getBestBlock();
result = getCachedResultFor(bestBlock);
if (result != null) {
return result;
}

result = Result.ofBlockResult(builder.buildPending(bestBlock.getHeader()));
cachedPendingBlockResult.set(result);

return result;
}
}

@Nullable
private Result getCachedResultFor(@Nonnull Block bestBlock) {
Result result = cachedPendingBlockResult.get();
if (result != null && result.getBlock().getParentHash().equals(bestBlock.getHash())) {
return result;
}
return null;
}

@VisibleForTesting
Result getCachedPendingBlockResult() {
return cachedPendingBlockResult.get();
}

@Override
public void start() {
emitter.addListener(listener);
}

@Override
public void stop() {
emitter.removeListener(listener);
}

public static class Result {
private final Block block;
private final Trie finalState;

public Result(@Nonnull Block block, @Nullable Trie finalState) {
this.block = block;
this.finalState = finalState;
}

static Result ofBlock(Block block) {
return new Result(block, null);
}

static Result ofBlockResult(BlockResult blockResult) {
return new Result(blockResult.getBlock(), blockResult.getFinalState());
}

public Block getBlock() {
return block;
}

public Trie getFinalState() {
return finalState;
}
}

private class CachedResultCleaner extends EthereumListenerAdapter {
@Override
public void onPendingTransactionsReceived(List<Transaction> transactions) {
cleanCachedResult();
}

@Override
public void onBestBlock(Block block, List<TransactionReceipt> receipts) {
cleanCachedResult();
}

private void cleanCachedResult() {
cachedPendingBlockResult.set(null);
}
}
}
23 changes: 12 additions & 11 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 @@ -23,12 +23,12 @@
import co.rsk.core.ReversibleTransactionExecutor;
import co.rsk.core.RskAddress;
import co.rsk.core.bc.AccountInformationProvider;
import co.rsk.core.bc.BlockResult;
import co.rsk.db.RepositoryLocator;
import co.rsk.peg.BridgeState;
import co.rsk.peg.BridgeSupport;
import co.rsk.peg.BridgeSupportFactory;
import co.rsk.rpc.ExecutionBlockRetriever;
import co.rsk.trie.Trie;
import co.rsk.trie.TrieStoreImpl;
import co.rsk.util.HexUtils;
import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -120,12 +120,14 @@ public Map<String, Object> bridgeState() throws IOException, BlockStoreException
public String call(CallArguments args, String bnOrId) {
String hReturn = null;
try {
BlockResult blockResult = executionBlockRetriever.retrieveExecutionBlock(bnOrId);
ExecutionBlockRetriever.Result result = executionBlockRetriever.retrieveExecutionBlock(bnOrId);
Block block = result.getBlock();
Trie finalState = result.getFinalState();
ProgramResult res;
if (blockResult.getFinalState() != null) {
res = callConstant_workaround(args, blockResult);
if (finalState != null) {
res = callConstantWithState(args, block, finalState);
} else {
res = callConstant(args, blockResult.getBlock());
res = callConstant(args, block);
}

if (res.isRevert()) {
Expand Down Expand Up @@ -290,13 +292,12 @@ public static Optional<String> decodeRevertReason(ProgramResult res) {
return decode != null && decode.length > 0 ? Optional.of((String) decode[0]) : Optional.empty();
}

@Deprecated
private ProgramResult callConstant_workaround(CallArguments args, BlockResult executionBlock) {
private ProgramResult callConstantWithState(CallArguments args, Block executionBlock, Trie state) {
CallArgumentsToByteArray hexArgs = new CallArgumentsToByteArray(args);
return reversibleTransactionExecutor.executeTransaction_workaround(
new MutableRepository(new TrieStoreImpl(new HashMapDB()), executionBlock.getFinalState()),
executionBlock.getBlock(),
executionBlock.getBlock().getCoinbase(),
return reversibleTransactionExecutor.executeTransaction(
new MutableRepository(new TrieStoreImpl(new HashMapDB()), state),
executionBlock,
executionBlock.getCoinbase(),
hexArgs.getGasPrice(),
hexArgs.getGasLimit(),
hexArgs.getToAddress(),
Expand Down
Loading

0 comments on commit d440fb8

Please sign in to comment.