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

Signature cache #1876

Merged
merged 18 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 24 additions & 17 deletions rskj-core/src/main/java/co/rsk/RskContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* 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.bitcoinj.core.NetworkParameters;
Expand Down Expand Up @@ -402,7 +401,7 @@ public TxQuotaChecker getTxQuotaChecker() {
checkIfNotClosed();

if (this.txQuotaChecker == null) {
this.txQuotaChecker = new TxQuotaChecker(System::currentTimeMillis);
this.txQuotaChecker = new TxQuotaChecker(System::currentTimeMillis, getReceivedTxSignatureCache());
}
return txQuotaChecker;
}
Expand All @@ -417,6 +416,16 @@ public synchronized ReceivedTxSignatureCache getReceivedTxSignatureCache() {
return receivedTxSignatureCache;
}

public BlockTxSignatureCache getBlockTxSignatureCache() {
checkIfNotClosed();

if (blockTxSignatureCache == null) {
blockTxSignatureCache = new BlockTxSignatureCache(getReceivedTxSignatureCache());
}

return blockTxSignatureCache;
}

public synchronized RepositoryLocator getRepositoryLocator() {
checkIfNotClosed();

Expand Down Expand Up @@ -485,7 +494,10 @@ public synchronized PrecompiledContracts getPrecompiledContracts() {
checkIfNotClosed();

if (precompiledContracts == null) {
precompiledContracts = new PrecompiledContracts(getRskSystemProperties(), getBridgeSupportFactory());
precompiledContracts = new PrecompiledContracts(
getRskSystemProperties(),
getBridgeSupportFactory(),
getBlockTxSignatureCache());
}

return precompiledContracts;
Expand All @@ -497,7 +509,7 @@ public synchronized BridgeSupportFactory getBridgeSupportFactory() {
if (bridgeSupportFactory == null) {
bridgeSupportFactory = new BridgeSupportFactory(getBtcBlockStoreFactory(),
getRskSystemProperties().getNetworkConstants().getBridgeConstants(),
getRskSystemProperties().getActivationConfig());
getRskSystemProperties().getActivationConfig(), getBlockTxSignatureCache());
}

return bridgeSupportFactory;
Expand Down Expand Up @@ -815,7 +827,8 @@ public synchronized TraceModule getTraceModule() {
getBlockStore(),
getReceiptStore(),
getBlockExecutor(),
getExecutionBlockRetriever()
getExecutionBlockRetriever(),
getBlockTxSignatureCache()
);
}

Expand All @@ -836,7 +849,7 @@ public synchronized TxPoolModule getTxPoolModule() {
checkIfNotClosed();

if (txPoolModule == null) {
txPoolModule = new TxPoolModuleImpl(getTransactionPool());
txPoolModule = new TxPoolModuleImpl(getTransactionPool(), getReceivedTxSignatureCache());
}

return txPoolModule;
Expand Down Expand Up @@ -1101,7 +1114,7 @@ public synchronized BlockParentDependantValidationRule getBlockParentDependantVa
if (blockParentDependantValidationRule == null) {
Constants commonConstants = getRskSystemProperties().getNetworkConstants();
blockParentDependantValidationRule = new BlockParentCompositeRule(
new BlockTxsFieldsValidationRule(),
new BlockTxsFieldsValidationRule(getBlockTxSignatureCache()),
new BlockTxsValidationRule(getRepositoryLocator(), getBlockTxSignatureCache()),
new PrevMinGasPriceRule(),
new BlockParentNumberRule(),
Expand Down Expand Up @@ -1288,7 +1301,8 @@ protected synchronized Web3 buildWeb3() {
getBuildInfo(),
getBlocksBloomStore(),
getWeb3InformationRetriever(),
getSyncProcessor());
getSyncProcessor(),
getBlockTxSignatureCache());
}

protected synchronized Web3InformationRetriever getWeb3InformationRetriever() {
Expand Down Expand Up @@ -1442,14 +1456,6 @@ private void initializeNativeLibs() {
AbstractAltBN128.init();
}

private BlockTxSignatureCache getBlockTxSignatureCache() {
if (blockTxSignatureCache == null) {
blockTxSignatureCache = new BlockTxSignatureCache(getReceivedTxSignatureCache());
}

return blockTxSignatureCache;
}

private KeyValueDataSource getBlocksBloomDataSource() {
if (this.blocksBloomDataSource == null) {
this.blocksBloomDataSource = this.buildBlocksBloomDataSource();
Expand Down Expand Up @@ -1766,7 +1772,8 @@ private BlockToMineBuilder getBlockToMineBuilder() {
getBlockFactory(),
getBlockExecutor(),
new MinimumGasPriceCalculator(Coin.valueOf(getMiningConfig().getMinGasPriceTarget())),
new MinerUtils()
new MinerUtils(),
getBlockTxSignatureCache()
);
}

Expand Down
20 changes: 8 additions & 12 deletions rskj-core/src/main/java/co/rsk/core/bc/PendingState.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,13 @@
* 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.core.bc;

import co.rsk.core.Coin;
import co.rsk.core.RskAddress;
import co.rsk.crypto.Keccak256;
import co.rsk.db.RepositorySnapshot;
import org.ethereum.core.Repository;
import org.ethereum.core.Transaction;
import org.ethereum.core.TransactionExecutor;
import org.ethereum.core.TransactionSet;
import org.ethereum.core.*;
import org.ethereum.util.ByteUtil;
import org.ethereum.vm.DataWord;
import org.slf4j.Logger;
Expand All @@ -46,12 +42,13 @@ public class PendingState implements AccountInformationProvider {
private final TransactionExecutorFactory transactionExecutorFactory;
private final TransactionSet pendingTransactions;
private boolean executed = false;
private final SignatureCache signatureCache;


public PendingState(RepositorySnapshot repository, TransactionSet pendingTransactions, TransactionExecutorFactory transactionExecutorFactory) {
public PendingState(RepositorySnapshot repository, TransactionSet pendingTransactions, TransactionExecutorFactory transactionExecutorFactory, SignatureCache signatureCache) {
this.pendingRepository = repository.startTracking();
this.pendingTransactions = pendingTransactions;
this.transactionExecutorFactory = transactionExecutorFactory;
this.signatureCache = signatureCache;
}

@Override
Expand Down Expand Up @@ -117,13 +114,13 @@ public BigInteger getNonce(RskAddress addr) {

// Note that this sort doesn't return the best solution, it is an approximation algorithm to find approximate
// solution. (No trivial solution)
public static List<Transaction> sortByPriceTakingIntoAccountSenderAndNonce(List<Transaction> transactions) {
public static List<Transaction> sortByPriceTakingIntoAccountSenderAndNonce(List<Transaction> transactions, SignatureCache signatureCache) {

//Priority heap, and list of transactions are ordered by descending gas price.
Comparator<Transaction> gasPriceComparator = reverseOrder(Comparator.comparing(Transaction::getGasPrice));

//First create a map to separate txs by each sender.
Map<RskAddress, List<Transaction>> senderTxs = transactions.stream().collect(Collectors.groupingBy(Transaction::getSender));
Map<RskAddress, List<Transaction>> senderTxs = transactions.stream().collect(Collectors.groupingBy(transaction -> transaction.getSender(signatureCache)));

//For each sender, order all txs by nonce and then by hash,
//finally we order by price in cases where nonce are equal, and then by hash to disambiguate
Expand Down Expand Up @@ -151,7 +148,7 @@ public static List<Transaction> sortByPriceTakingIntoAccountSenderAndNonce(List<
while (txsCount > 0) {
Transaction nextTxToAdd = candidateTxs.remove();
sortedTxs.add(nextTxToAdd);
List<Transaction> txs = senderTxs.get(nextTxToAdd.getSender());
List<Transaction> txs = senderTxs.get(nextTxToAdd.getSender(signatureCache));
if (!txs.isEmpty()) {
Transaction tx = txs.remove(0);
candidateTxs.add(tx);
Expand All @@ -172,8 +169,7 @@ private <T> T postExecutionReturn(PostExecutionAction<T> action) {
}

private void executeTransactions(Repository currentRepository, List<Transaction> pendingTransactions) {

PendingState.sortByPriceTakingIntoAccountSenderAndNonce(pendingTransactions)
PendingState.sortByPriceTakingIntoAccountSenderAndNonce(pendingTransactions, signatureCache)
.forEach(pendingTransaction -> executeTransaction(currentRepository, pendingTransaction));
}

Expand Down
27 changes: 14 additions & 13 deletions rskj-core/src/main/java/co/rsk/core/bc/TransactionPoolImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* 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.core.bc;

import co.rsk.config.RskSystemProperties;
Expand Down Expand Up @@ -52,8 +51,8 @@
public class TransactionPoolImpl implements TransactionPool {
private static final Logger logger = LoggerFactory.getLogger("txpool");

private final TransactionSet pendingTransactions = new TransactionSet();
private final TransactionSet queuedTransactions = new TransactionSet();
private final TransactionSet pendingTransactions;
private final TransactionSet queuedTransactions;

private final Map<Keccak256, Long> transactionBlocks = new HashMap<>();
private final Map<Keccak256, Long> transactionTimes = new HashMap<>();
Expand Down Expand Up @@ -95,7 +94,10 @@ public TransactionPoolImpl(RskSystemProperties config, RepositoryLocator reposit
this.quotaChecker = txQuotaChecker;
this.gasPriceTracker = gasPriceTracker;

this.validator = new TxPendingValidator(config.getNetworkConstants(), config.getActivationConfig(), config.getNumOfAccountSlots());
pendingTransactions = new TransactionSet(this.signatureCache);
queuedTransactions = new TransactionSet(this.signatureCache);

this.validator = new TxPendingValidator(config.getNetworkConstants(), config.getActivationConfig(), config.getNumOfAccountSlots(), signatureCache);

if (this.outdatedTimeout > 0) {
this.cleanerTimer = Executors.newSingleThreadScheduledExecutor(r -> new Thread(r, "TransactionPoolCleanerTimer"));
Expand Down Expand Up @@ -158,7 +160,7 @@ public PendingState getPendingState() {

private PendingState getPendingState(RepositorySnapshot currentRepository) {
removeObsoleteTransactions(this.outdatedThreshold, this.outdatedTimeout);
return new PendingState(currentRepository, new TransactionSet(pendingTransactions), (repository, tx) -> transactionExecutorFactory.newInstance(tx, 0, bestBlock.getCoinbase(), repository, createFakePendingBlock(bestBlock), 0));
return new PendingState(currentRepository, new TransactionSet(pendingTransactions, signatureCache), (repository, tx) -> transactionExecutorFactory.newInstance(tx, 0, bestBlock.getCoinbase(), repository, createFakePendingBlock(bestBlock), 0), signatureCache);
}

private RepositorySnapshot getCurrentRepository() {
Expand Down Expand Up @@ -215,7 +217,7 @@ public synchronized List<Transaction> addTransactions(final List<Transaction> tx
private Optional<Transaction> getQueuedSuccessor(Transaction tx) {
BigInteger next = tx.getNonceAsInteger().add(BigInteger.ONE);

List<Transaction> txsaccount = this.queuedTransactions.getTransactionsWithSender(tx.getSender());
List<Transaction> txsaccount = this.queuedTransactions.getTransactionsWithSender(tx.getSender(signatureCache));

if (txsaccount == null) {
return Optional.empty();
Expand Down Expand Up @@ -243,7 +245,7 @@ private TransactionPoolAddResult internalAddTransaction(final Transaction tx) {
Keccak256 hash = tx.getHash();
logger.trace("add transaction {} {}", toBI(tx.getNonce()), tx.getHash());

Optional<Transaction> replacedTx = pendingTransactions.getTransactionsWithSender(tx.getSender()).stream().filter(t -> t.getNonceAsInteger().equals(tx.getNonceAsInteger())).findFirst();
Optional<Transaction> replacedTx = pendingTransactions.getTransactionsWithSender(tx.getSender(signatureCache)).stream().filter(t -> t.getNonceAsInteger().equals(tx.getNonceAsInteger())).findFirst();
if (replacedTx.isPresent() && !isBumpingGasPriceForSameNonceTx(tx, replacedTx.get())) {
return TransactionPoolAddResult.withError("gas price not enough to bump transaction");
}
Expand All @@ -253,7 +255,7 @@ private TransactionPoolAddResult internalAddTransaction(final Transaction tx) {
final long timestampSeconds = this.getCurrentTimeInSeconds();
transactionTimes.put(hash, timestampSeconds);

BigInteger currentNonce = getPendingState(currentRepository).getNonce(tx.getSender());
BigInteger currentNonce = getPendingState(currentRepository).getNonce(tx.getSender(signatureCache));
BigInteger txNonce = tx.getNonceAsInteger();
if (txNonce.compareTo(currentNonce) > 0) {
this.addQueuedTransaction(tx);
Expand Down Expand Up @@ -454,8 +456,7 @@ private Block createFakePendingBlock(Block best) {
}

private TransactionValidationResult shouldAcceptTx(Transaction tx, RepositorySnapshot currentRepository) {
AccountState state = currentRepository.getAccountState(tx.getSender(signatureCache));
return validator.isValid(tx, bestBlock, state);
return validator.isValid(tx, bestBlock, currentRepository.getAccountState(tx.getSender(signatureCache)));
}

/**
Expand All @@ -464,15 +465,15 @@ private TransactionValidationResult shouldAcceptTx(Transaction tx, RepositorySna
* @return whether the sender balance is enough to pay for all pending transactions + newTx
*/
private boolean senderCanPayPendingTransactionsAndNewTx(Transaction newTx, RepositorySnapshot currentRepository) {
List<Transaction> transactions = pendingTransactions.getTransactionsWithSender(newTx.getSender());
List<Transaction> transactions = pendingTransactions.getTransactionsWithSender(newTx.getSender(signatureCache));

Coin accumTxCost = Coin.ZERO;
for (Transaction t : transactions) {
accumTxCost = accumTxCost.add(getTxBaseCost(t));
}

Coin costWithNewTx = accumTxCost.add(getTxBaseCost(newTx));
return costWithNewTx.compareTo(currentRepository.getBalance(newTx.getSender())) <= 0;
return costWithNewTx.compareTo(currentRepository.getBalance(newTx.getSender(signatureCache))) <= 0;
}

private Coin getTxBaseCost(Transaction tx) {
Expand All @@ -486,7 +487,7 @@ private Coin getTxBaseCost(Transaction tx) {
}

private long getTransactionCost(Transaction tx, long number) {
return tx.transactionCost(config.getNetworkConstants(), config.getActivationConfig().forBlock(number));
return tx.transactionCost(config.getNetworkConstants(), config.getActivationConfig().forBlock(number), signatureCache);
}

}
10 changes: 7 additions & 3 deletions rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ public class BlockToMineBuilder {

private final ForkDetectionDataCalculator forkDetectionDataCalculator;

private final SignatureCache signatureCache;

public BlockToMineBuilder(
ActivationConfig activationConfig,
MiningConfig miningConfig,
Expand All @@ -85,7 +87,8 @@ public BlockToMineBuilder(
BlockFactory blockFactory,
BlockExecutor blockExecutor,
MinimumGasPriceCalculator minimumGasPriceCalculator,
MinerUtils minerUtils) {
MinerUtils minerUtils,
SignatureCache signatureCache) {
this.activationConfig = Objects.requireNonNull(activationConfig);
this.miningConfig = Objects.requireNonNull(miningConfig);
this.repositoryLocator = Objects.requireNonNull(repositoryLocator);
Expand All @@ -100,6 +103,7 @@ public BlockToMineBuilder(
this.executor = blockExecutor;
this.minimumGasPriceCalculator = minimumGasPriceCalculator;
this.minerUtils = minerUtils;
this.signatureCache = signatureCache;
}

/**
Expand Down Expand Up @@ -157,7 +161,7 @@ private List<BlockHeader> getUnclesHeaders(BlockHeader newBlockParentHeader) {

private List<Transaction> getTransactions(List<Transaction> txsToRemove, BlockHeader parentHeader, Coin minGasPrice) {
logger.debug("getting transactions from pending state");
List<Transaction> txs = minerUtils.getAllTransactions(transactionPool);
List<Transaction> txs = minerUtils.getAllTransactions(transactionPool, signatureCache);
logger.debug("{} transaction(s) collected from pending state", txs.size());

final long blockNumber = parentHeader.getNumber() + 1;
Expand All @@ -170,7 +174,7 @@ private List<Transaction> getTransactions(List<Transaction> txsToRemove, BlockHe

final boolean isRskip252Enabled = activationConfig.isActive(ConsensusRule.RSKIP252, blockNumber);

return minerUtils.filterTransactions(txsToRemove, txs, accountNonces, originalRepo, minGasPrice, isRskip252Enabled);
return minerUtils.filterTransactions(txsToRemove, txs, accountNonces, originalRepo, minGasPrice, isRskip252Enabled, signatureCache);
}

private void removePendingTransactions(List<Transaction> transactions) {
Expand Down
11 changes: 5 additions & 6 deletions rskj-core/src/main/java/co/rsk/mine/MinerUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* 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.mine;

import co.rsk.bitcoinj.core.BtcTransaction;
Expand All @@ -32,6 +31,7 @@
import org.bouncycastle.util.Arrays;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ConsensusRule;
import org.ethereum.core.SignatureCache;
import org.ethereum.core.Transaction;
import org.ethereum.core.TransactionPool;
import org.slf4j.Logger;
Expand All @@ -48,7 +48,6 @@
import java.util.function.Function;

public class MinerUtils {

private static final Logger logger = LoggerFactory.getLogger("minerserver");

public static co.rsk.bitcoinj.core.BtcTransaction getBitcoinMergedMiningCoinbaseTransaction(co.rsk.bitcoinj.core.NetworkParameters params, MinerWork work) {
Expand Down Expand Up @@ -163,21 +162,21 @@ public static byte[] buildMerkleProof(
}
}

public List<org.ethereum.core.Transaction> getAllTransactions(TransactionPool transactionPool) {
public List<org.ethereum.core.Transaction> getAllTransactions(TransactionPool transactionPool, SignatureCache signatureCache) {

List<Transaction> txs = transactionPool.getPendingTransactions();

return PendingState.sortByPriceTakingIntoAccountSenderAndNonce(txs);
return PendingState.sortByPriceTakingIntoAccountSenderAndNonce(txs, signatureCache);
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this method be static?

}

public List<org.ethereum.core.Transaction> filterTransactions(List<Transaction> txsToRemove, List<Transaction> txs, Map<RskAddress, BigInteger> accountNonces, RepositorySnapshot originalRepo, Coin minGasPrice, boolean isRskip252Enabled) {
public List<org.ethereum.core.Transaction> filterTransactions(List<Transaction> txsToRemove, List<Transaction> txs, Map<RskAddress, BigInteger> accountNonces, RepositorySnapshot originalRepo, Coin minGasPrice, boolean isRskip252Enabled, SignatureCache signatureCache) {
List<org.ethereum.core.Transaction> txsResult = new ArrayList<>();
for (org.ethereum.core.Transaction tx : txs) {
try {
Keccak256 hash = tx.getHash();
Coin txValue = tx.getValue();
BigInteger txNonce = new BigInteger(1, tx.getNonce());
RskAddress txSender = tx.getSender();
RskAddress txSender = tx.getSender(signatureCache);
Copy link
Contributor

Choose a reason for hiding this comment

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

and this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will update this as requested. 💪🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

logger.debug("Examining tx={} sender: {} value: {} nonce: {}", hash, txSender, txValue, txNonce);

BigInteger expectedNonce;
Expand Down
Loading