From fb9671613da88b7871ca4fd366b69665a4c81739 Mon Sep 17 00:00:00 2001 From: Ilan Date: Wed, 23 Nov 2022 14:59:06 -0300 Subject: [PATCH] General fixes + refactor getSublistCandidate --- .../co/rsk/core/TransactionListExecutor.java | 72 +++++---- .../java/co/rsk/core/bc/BlockExecutor.java | 139 +++++++++++------- .../bc/ParallelizeTransactionHandler.java | 51 +++++-- .../java/org/ethereum/core/BlockHeader.java | 20 ++- .../ethereum/core/BlockHeaderExtension.java | 8 +- .../ethereum/core/BlockHeaderExtensionV1.java | 8 +- .../java/org/ethereum/core/BlockHeaderV0.java | 6 +- .../java/co/rsk/core/BlockFactoryTest.java | 4 +- .../co/rsk/core/BlockHeaderExtensionTest.java | 21 +++ .../java/co/rsk/core/BlockHeaderTest.java | 2 +- .../java/co/rsk/core/BlockHeaderV1Test.java | 2 +- .../bc/ParallelizeTransactionHandlerTest.java | 42 ++++++ 12 files changed, 266 insertions(+), 109 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/core/TransactionListExecutor.java b/rskj-core/src/main/java/co/rsk/core/TransactionListExecutor.java index 1cbcba0944c..e49da9efb5b 100644 --- a/rskj-core/src/main/java/co/rsk/core/TransactionListExecutor.java +++ b/rskj-core/src/main/java/co/rsk/core/TransactionListExecutor.java @@ -83,9 +83,7 @@ public Boolean call() { int numberOfTransactions = block.getTransactionsList().size(); boolean isRemascTransaction = tx.isRemascTransaction(this.i, numberOfTransactions); - if (this.remascEnabled && isRemascTransaction) { - addFeesToRemasc(); - } + addFeesToRemascIfEnabled(isRemascTransaction); TransactionExecutor txExecutor = transactionExecutorFactory.newInstance( tx, @@ -101,16 +99,7 @@ public Boolean call() { boolean transactionExecuted = txExecutor.executeTransaction(); if (!acceptInvalidTransactions && !transactionExecuted) { - // It's used just for testing, the last tx should be always the REMASC. - payToRemascWhenThereIsNoRemascTx(numberOfTransactions, isRemascTransaction); - if (!discardInvalidTxs) { - logger.warn("block: [{}] execution interrupted because of invalid tx: [{}]", - block.getNumber(), tx.getHash() - ); - return false; - } - - logger.warn("block: [{}] discarded tx: [{}]", block.getNumber(), tx.getHash()); + if (discardIfInvalid(tx, numberOfTransactions, isRemascTransaction)) return false; continue; } @@ -130,31 +119,21 @@ public Boolean call() { long txGasUsed = txExecutor.getGasUsed(); totalGasUsed += txGasUsed; - Coin txPaidFees = txExecutor.getPaidFees(); - if (txPaidFees != null) { - totalPaidFees = totalPaidFees.add(txPaidFees); - } + addPaidFeesToToal(txExecutor); // It's used just for testing, the last tx should be always the REMASC. payToRemascWhenThereIsNoRemascTx(numberOfTransactions, isRemascTransaction); deletedAccounts.addAll(txExecutor.getResult().getDeleteAccounts()); - TransactionReceipt receipt = new TransactionReceipt(); - receipt.setGasUsed(txGasUsed); - receipt.setCumulativeGas(totalGasUsed); - - receipt.setTxStatus(txExecutor.getReceipt().isSuccessful()); - receipt.setTransaction(tx); - receipt.setLogInfoList(txExecutor.getVMLogs()); - receipt.setStatus(txExecutor.getReceipt().getStatus()); + TransactionReceipt receipt = createTransactionReceipt(totalGasUsed, tx, txExecutor, txGasUsed); logger.trace("block: [{}] executed tx: [{}]", block.getNumber(), tx.getHash()); - logger.trace("tx[{}].receipt", i + 1); - i++; + logger.trace("tx[{}].receipt", i); + receipts.put(i, receipt); logger.trace("tx[{}] done", i); @@ -163,6 +142,45 @@ public Boolean call() { return true; } + private boolean discardIfInvalid(Transaction tx, int numberOfTransactions, boolean isRemascTransaction) { + // It's used just for testing, the last tx should be always the REMASC. + payToRemascWhenThereIsNoRemascTx(numberOfTransactions, isRemascTransaction); + if (!discardInvalidTxs) { + logger.warn("block: [{}] execution interrupted because of invalid tx: [{}]", + block.getNumber(), tx.getHash() + ); + return true; + } + + logger.warn("block: [{}] discarded tx: [{}]", block.getNumber(), tx.getHash()); + return false; + } + + private TransactionReceipt createTransactionReceipt(long totalGasUsed, Transaction tx, TransactionExecutor txExecutor, long txGasUsed) { + TransactionReceipt receipt = new TransactionReceipt(); + receipt.setGasUsed(txGasUsed); + receipt.setCumulativeGas(totalGasUsed); + + receipt.setTxStatus(txExecutor.getReceipt().isSuccessful()); + receipt.setTransaction(tx); + receipt.setLogInfoList(txExecutor.getVMLogs()); + receipt.setStatus(txExecutor.getReceipt().getStatus()); + return receipt; + } + + private void addPaidFeesToToal(TransactionExecutor txExecutor) { + Coin txPaidFees = txExecutor.getPaidFees(); + if (txPaidFees != null) { + totalPaidFees = totalPaidFees.add(txPaidFees); + } + } + + private void addFeesToRemascIfEnabled(boolean isRemascTransaction) { + if (this.remascEnabled && isRemascTransaction) { + addFeesToRemasc(); + } + } + private void payToRemascWhenThereIsNoRemascTx(int numberOfTransactions, boolean isRemascTransaction) { boolean isLastTx = this.i == numberOfTransactions - 1; if (this.remascEnabled && isLastTx && !isRemascTransaction) { diff --git a/rskj-core/src/main/java/co/rsk/core/bc/BlockExecutor.java b/rskj-core/src/main/java/co/rsk/core/bc/BlockExecutor.java index e7011f2d7d6..7f46e7f9a5a 100644 --- a/rskj-core/src/main/java/co/rsk/core/bc/BlockExecutor.java +++ b/rskj-core/src/main/java/co/rsk/core/bc/BlockExecutor.java @@ -340,9 +340,7 @@ private BlockResult executePreviousRSKIP144( for (Transaction tx : block.getTransactionsList()) { - if (this.isRemascEnabled() && tx.isRemascTransaction(block.getTransactionsList().size(), txindex)) { - addFeesToRemasc(totalPaidFees, track); - } + addFeesToRemascIfRemascTx(block, track, totalPaidFees, txindex, tx); loggingApplyBlockToTx(block, i); @@ -367,23 +365,11 @@ private BlockResult executePreviousRSKIP144( continue; } - executedTransactions.add(tx); - - if (this.registerProgramResults) { - this.transactionResults.put(tx.getHash(), txExecutor.getResult()); - } - - if (vmTrace) { - txExecutor.extractTrace(programTraceProcessor); - } - - loggingTxExecuted(); + registerExecutedTx(programTraceProcessor, vmTrace, executedTransactions, tx, txExecutor); long gasUsed = txExecutor.getGasUsed(); totalGasUsed += gasUsed; - Coin paidFees = txExecutor.getPaidFees(); - if (paidFees != null) { - totalPaidFees = totalPaidFees.add(paidFees); - } + + totalPaidFees = addTotalPaidFees(totalPaidFees, txExecutor); deletedAccounts.addAll(txExecutor.getResult().getDeleteAccounts()); @@ -416,6 +402,34 @@ private BlockResult executePreviousRSKIP144( return result; } + private void registerExecutedTx(ProgramTraceProcessor programTraceProcessor, boolean vmTrace, List executedTransactions, Transaction tx, TransactionExecutor txExecutor) { + executedTransactions.add(tx); + + if (this.registerProgramResults) { + this.transactionResults.put(tx.getHash(), txExecutor.getResult()); + } + + if (vmTrace) { + txExecutor.extractTrace(programTraceProcessor); + } + + loggingTxExecuted(); + } + + private void addFeesToRemascIfRemascTx(Block block, Repository track, Coin totalPaidFees, int txindex, Transaction tx) { + if (this.isRemascEnabled() && tx.isRemascTransaction(block.getTransactionsList().size(), txindex)) { + addFeesToRemasc(totalPaidFees, track); + } + } + + private Coin addTotalPaidFees(Coin totalPaidFees, TransactionExecutor txExecutor) { + Coin paidFees = txExecutor.getPaidFees(); + if (paidFees != null) { + totalPaidFees = totalPaidFees.add(paidFees); + } + return totalPaidFees; + } + private BlockResult executeParallel( @Nullable ProgramTraceProcessor programTraceProcessor, int vmTraceOptions, @@ -625,9 +639,7 @@ private BlockResult executeForMiningAfterRSKIP144( int numberOfTransactions = transactionsList.size(); boolean isRemascTransaction = tx.isRemascTransaction(txindex, numberOfTransactions); - if (this.isRemascEnabled() && isRemascTransaction) { - addFeesToRemasc(totalPaidFees, track); - } + addFeesToRemascIfRemascTxAndTrack(track, totalPaidFees, isRemascTransaction); TransactionExecutor txExecutor = transactionExecutorFactory.newInstance( tx, @@ -643,53 +655,32 @@ private BlockResult executeForMiningAfterRSKIP144( boolean transactionExecuted = txExecutor.executeTransaction(); if (!acceptInvalidTransactions && !transactionExecuted) { - payToRemascWhenThereIsNoRemascTx(track, totalPaidFees, txindex, numberOfTransactions, isRemascTransaction); - - if (!discardInvalidTxs) { + if (discardIfInvalid(block, discardInvalidTxs, track, totalPaidFees, txindex, tx, numberOfTransactions, isRemascTransaction)) { return getBlockResultAndLogExecutionInterrupted(block, metric, tx); } - - loggingDiscardedBlock(block, tx); txindex++; continue; } - Optional sublistGasAccumulated; - if (isRemascTransaction) { - sublistGasAccumulated = parallelizeTransactionHandler.addRemascTransaction(tx, txExecutor.getGasUsed()); - } else { - sublistGasAccumulated = parallelizeTransactionHandler.addTransaction(tx, readWrittenKeysTracker.getThisThreadReadKeys(), readWrittenKeysTracker.getThisThreadWrittenKeys(), txExecutor.getGasUsed()); - } + Optional sublistGasAccumulated = calculateSublistGasAccumulated(readWrittenKeysTracker, parallelizeTransactionHandler, tx, isRemascTransaction, txExecutor); if (!acceptInvalidTransactions && !sublistGasAccumulated.isPresent()) { - payToRemascWhenThereIsNoRemascTx(track, totalPaidFees, txindex, numberOfTransactions, isRemascTransaction); - - if (!discardInvalidTxs) { + if (discardIfInvalid(block, discardInvalidTxs, track, totalPaidFees, txindex, tx, numberOfTransactions, isRemascTransaction)) { return getBlockResultAndLogExecutionInterrupted(block, metric, tx); } - - loggingDiscardedBlock(block, tx); txindex++; continue; } - readWrittenKeysTracker.clear(); - - if (this.registerProgramResults) { - this.transactionResults.put(tx.getHash(), txExecutor.getResult()); - } + registerTxExecutedForMiningAfterRSKIP144(readWrittenKeysTracker, tx, txExecutor); - loggingTxExecuted(); long gasUsed = txExecutor.getGasUsed(); gasUsedInBlock += gasUsed; - Coin paidFees = txExecutor.getPaidFees(); - if (paidFees != null) { - totalPaidFees = totalPaidFees.add(paidFees); - } + totalPaidFees = addTotalPaidFees(totalPaidFees, txExecutor); payToRemascWhenThereIsNoRemascTx(track, totalPaidFees, txindex, numberOfTransactions, isRemascTransaction); - deletedAccounts.addAll(txExecutor.getResult().getDeleteAccounts()); + deletedAccounts.addAll(txExecutor.getResult().getDeleteAccounts()); //orElseGet is used for testing only when acceptInvalidTransactions is set. long cumulativeGas = sublistGasAccumulated @@ -712,11 +703,7 @@ private BlockResult executeForMiningAfterRSKIP144( List executedTransactions = parallelizeTransactionHandler.getTransactionsInOrder(); short[] sublistOrder = parallelizeTransactionHandler.getTransactionsPerSublistInOrder(); - List receipts = new ArrayList<>(); - - for (Transaction tx : executedTransactions) { - receipts.add(receiptsByTx.get(tx)); - } + List receipts = getTransactionReceipts(receiptsByTx, executedTransactions); BlockResult result = new BlockResult( block, @@ -732,6 +719,52 @@ private BlockResult executeForMiningAfterRSKIP144( return result; } + private void registerTxExecutedForMiningAfterRSKIP144(IReadWrittenKeysTracker readWrittenKeysTracker, Transaction tx, TransactionExecutor txExecutor) { + readWrittenKeysTracker.clear(); + + if (this.registerProgramResults) { + this.transactionResults.put(tx.getHash(), txExecutor.getResult()); + } + + loggingTxExecuted(); + } + + private List getTransactionReceipts(Map receiptsByTx, List executedTransactions) { + List receipts = new ArrayList<>(); + + for (Transaction tx : executedTransactions) { + receipts.add(receiptsByTx.get(tx)); + } + return receipts; + } + + private boolean discardIfInvalid(Block block, boolean discardInvalidTxs, Repository track, Coin totalPaidFees, int txindex, Transaction tx, int numberOfTransactions, boolean isRemascTransaction) { + payToRemascWhenThereIsNoRemascTx(track, totalPaidFees, txindex, numberOfTransactions, isRemascTransaction); + + if (!discardInvalidTxs) { + return true; + } + + loggingDiscardedBlock(block, tx); + return false; + } + + private Optional calculateSublistGasAccumulated(IReadWrittenKeysTracker readWrittenKeysTracker, ParallelizeTransactionHandler parallelizeTransactionHandler, Transaction tx, boolean isRemascTransaction, TransactionExecutor txExecutor) { + Optional sublistGasAccumulated; + if (isRemascTransaction) { + sublistGasAccumulated = parallelizeTransactionHandler.addRemascTransaction(tx, txExecutor.getGasUsed()); + } else { + sublistGasAccumulated = parallelizeTransactionHandler.addTransaction(tx, readWrittenKeysTracker.getThisThreadReadKeys(), readWrittenKeysTracker.getThisThreadWrittenKeys(), txExecutor.getGasUsed()); + } + return sublistGasAccumulated; + } + + private void addFeesToRemascIfRemascTxAndTrack(Repository track, Coin totalPaidFees, boolean isRemascTransaction) { + if (this.isRemascEnabled() && isRemascTransaction) { + addFeesToRemasc(totalPaidFees, track); + } + } + private void saveOrCommitTrackState(boolean saveState, Repository track) { logger.trace("End txs executions."); if (saveState) { diff --git a/rskj-core/src/main/java/co/rsk/core/bc/ParallelizeTransactionHandler.java b/rskj-core/src/main/java/co/rsk/core/bc/ParallelizeTransactionHandler.java index 0d3861cf058..93b6c655598 100644 --- a/rskj-core/src/main/java/co/rsk/core/bc/ParallelizeTransactionHandler.java +++ b/rskj-core/src/main/java/co/rsk/core/bc/ParallelizeTransactionHandler.java @@ -151,52 +151,73 @@ private Optional getAvailableSublistWithLessUsedGas(long txG return sublistCandidate; } - private TransactionSublist getSublistCandidates(Transaction tx, Set newReadKeys, Set newWrittenKeys) { Optional sublistCandidate = getSublistBySender(tx); if (sublistCandidate.isPresent() && sublistCandidate.get().isSequential()) { + // there is a tx with the same sender in the sequential sublist return getSequentialSublist(); } - // read - written + // analyze reads for (ByteArrayWrapper newReadKey : newReadKeys) { - if (sublistsHavingWrittenToKey.containsKey(newReadKey)) { - TransactionSublist sublist = sublistsHavingWrittenToKey.get(newReadKey); - sublistCandidate = Optional.of(sublistCandidate.map(sc -> returnsSequentialIfBothAreDifferent(sc, sublist)).orElse(sublist)); - } + // read - written + sublistCandidate = sublistCandidate + .map(sc -> getCandidateIfCollidesWithWrittenKeys(sc, newReadKey)) + .orElse(sublistCandidate); } if (sublistCandidate.isPresent() && sublistCandidate.get().isSequential()) { + // there is a tx with read-written collision in sequential sublist return sublistCandidate.get(); } + // analyze writes for (ByteArrayWrapper newWrittenKey : newWrittenKeys) { - // written - written, - if (sublistsHavingWrittenToKey.containsKey(newWrittenKey)) { - TransactionSublist sublist = sublistsHavingWrittenToKey.get(newWrittenKey); - sublistCandidate = Optional.of(sublistCandidate.map(sc -> returnsSequentialIfBothAreDifferent(sc, sublist)).orElse(sublist)); - } + // write - written, + sublistCandidate = sublistCandidate + .map(sc -> getCandidateIfCollidesWithWrittenKeys(sc, newWrittenKey)) + .orElse(sublistCandidate); if (sublistCandidate.isPresent() && sublistCandidate.get().isSequential()) { + // there is a tx with write-written collision in sequential sublist return sublistCandidate.get(); } - // read - written + + // write - read if (sublistsHavingReadFromKey.containsKey(newWrittenKey)) { Set sublist = sublistsHavingReadFromKey.get(newWrittenKey); if (sublist.size() > 1) { + // there is a write-read collision with multiple sublists return getSequentialSublist(); } - sublistCandidate = Optional.of(sublistCandidate.map(sc -> getTransactionSublistForReadKeys(sublist, sc)).orElse(getNextSublist(sublist))); + // there is only one colluded sublist + // if there is no candidate, take the colluded sublist + // otherwise, check if the sublist is different from the candidate and return the sequential + sublistCandidate = Optional.of(sublistCandidate + .map(sc -> getSequentialIfIncluded(sublist, sc)) + .orElse(getNextSublist(sublist))); } } - return sublistCandidate.orElseGet(() -> getAvailableSublistWithLessUsedGas(GasCost.toGas(tx.getGasLimit())).orElseGet(this::getSequentialSublist)); + // if there is no candidate use the sublist with more gas available + // if the is no more gas available in any parallel sublist use the sequential + return sublistCandidate + .orElseGet(() -> getAvailableSublistWithLessUsedGas(GasCost.toGas(tx.getGasLimit())) + .orElseGet(this::getSequentialSublist)); + } + + private Optional getCandidateIfCollidesWithWrittenKeys(TransactionSublist sublistCandidate, ByteArrayWrapper newReadKey) { + if (sublistsHavingWrittenToKey.containsKey(newReadKey)) { + TransactionSublist sublist = sublistsHavingWrittenToKey.get(newReadKey); + return Optional.of(returnsSequentialIfBothAreDifferent(sublistCandidate, sublist)); + } + return Optional.of(sublistCandidate); } - private TransactionSublist getTransactionSublistForReadKeys(Set sublist, TransactionSublist sc) { + private TransactionSublist getSequentialIfIncluded(Set sublist, TransactionSublist sc) { if (!sublist.contains(sc)) { return getSequentialSublist(); } diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java index 361af5155df..bea5356d9fa 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java @@ -1,3 +1,21 @@ +/* + * This file is part of RskJ + * Copyright (C) 2017 RSK Labs Ltd. + * (derived from ethereumJ library, Copyright (c) 2016 ) + * + * 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 . + */ package org.ethereum.core; import co.rsk.config.RskMiningConstants; @@ -118,7 +136,7 @@ public abstract class BlockHeader { /* Indicates if Block hash for merged mining should have the format described in RSKIP-110 */ private final boolean includeForkDetectionData; - public BlockHeader(byte[] parentHash, byte[] unclesHash, RskAddress coinbase, byte[] stateRoot, + protected BlockHeader(byte[] parentHash, byte[] unclesHash, RskAddress coinbase, byte[] stateRoot, byte[] txTrieRoot, byte[] receiptTrieRoot, BlockDifficulty difficulty, long number, byte[] gasLimit, long gasUsed, long timestamp, byte[] extraData, Coin paidFees, byte[] bitcoinMergedMiningHeader, byte[] bitcoinMergedMiningMerkleProof, diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderExtension.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderExtension.java index bf0fcd8537f..db154461919 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderExtension.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderExtension.java @@ -2,11 +2,11 @@ import org.ethereum.util.RLPList; -public abstract class BlockHeaderExtension { - public abstract byte getHeaderVersion(); - public abstract byte[] getEncoded(); +public interface BlockHeaderExtension { + byte getHeaderVersion(); + byte[] getEncoded(); - public static BlockHeaderExtension fromEncoded(RLPList encoded) { + static BlockHeaderExtension fromEncoded(RLPList encoded) { byte version = encoded.get(0).getRLPData()[0]; if (version == 0x1) return BlockHeaderExtensionV1.fromEncoded(encoded.getRLPData()); return null; diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderExtensionV1.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderExtensionV1.java index 93e07b63562..7cce383f03f 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderExtensionV1.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderExtensionV1.java @@ -9,7 +9,7 @@ import java.util.Arrays; import java.util.List; -public class BlockHeaderExtensionV1 extends BlockHeaderExtension { +public class BlockHeaderExtensionV1 implements BlockHeaderExtension { @Override public byte getHeaderVersion() { return 0x1; } @@ -27,7 +27,7 @@ public void setTxExecutionSublistsEdges(short[] edges) { public BlockHeaderExtensionV1(byte[] logsBloom, short[] edges) { this.logsBloom = logsBloom; - this.setTxExecutionSublistsEdges(edges); + this.txExecutionSublistsEdges = edges != null ? Arrays.copyOf(edges, edges.length) : null; } public byte[] getHash() { @@ -38,7 +38,9 @@ public byte[] getHash() { public byte[] getEncoded() { List fieldToEncodeList = Lists.newArrayList(RLP.encodeByte(this.getHeaderVersion()), RLP.encodeElement(this.getLogsBloom())); short[] txExecutionSublistsEdges = this.getTxExecutionSublistsEdges(); - if (txExecutionSublistsEdges != null) fieldToEncodeList.add(ByteUtil.shortsToRLP(this.getTxExecutionSublistsEdges())); + if (txExecutionSublistsEdges != null) { + fieldToEncodeList.add(ByteUtil.shortsToRLP(this.getTxExecutionSublistsEdges())); + } return RLP.encodeList(fieldToEncodeList.toArray(new byte[][]{})); } diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderV0.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderV0.java index 824ffedbf47..d172b632b7a 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderV0.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderV0.java @@ -31,9 +31,11 @@ public class BlockHeaderV0 extends BlockHeader { @Override public byte getVersion() { return 0x0; } @Override - public BlockHeaderExtension getExtension() { return null; } + public BlockHeaderExtension getExtension() { return null; } // block header v0 has no extension @Override - public void setExtension(BlockHeaderExtension extension) {} + public void setExtension(BlockHeaderExtension extension) { + // block header v0 has no extension + } private byte[] logsBloom; private short[] txExecutionSublistsEdges; diff --git a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java index 1ce61e475b6..00adc54fc44 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java @@ -299,7 +299,7 @@ public void headerIsVersion0BeforeActivation () { long number = 20L; enableRskip351At(number); BlockHeader header = factory.getBlockHeaderBuilder().setNumber(number - 1).build(); - Assertions.assertEquals(header.getVersion(), 0); + Assertions.assertEquals(0, header.getVersion()); } @Test @@ -307,7 +307,7 @@ public void headerIsVersion1AfterActivation () { long number = 20L; enableRskip351At(number); BlockHeader header = factory.getBlockHeaderBuilder().setNumber(number).build(); - Assertions.assertEquals(header.getVersion(), 1); + Assertions.assertEquals(1, header.getVersion()); } @Test diff --git a/rskj-core/src/test/java/co/rsk/core/BlockHeaderExtensionTest.java b/rskj-core/src/test/java/co/rsk/core/BlockHeaderExtensionTest.java index 6d1cb2ae9c8..3454ccb5fbe 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockHeaderExtensionTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockHeaderExtensionTest.java @@ -1,13 +1,17 @@ package co.rsk.core; +import com.google.common.collect.Lists; import org.ethereum.core.BlockHeaderExtension; import org.ethereum.core.BlockHeaderExtensionV1; import org.ethereum.core.Bloom; +import org.ethereum.util.ByteUtil; import org.ethereum.util.RLP; import org.ethereum.util.RLPList; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.util.List; + public class BlockHeaderExtensionTest { @Test public void decodeV1() { @@ -28,4 +32,21 @@ public void decodeV1() { Assertions.assertEquals(extension.getHeaderVersion(), decoded.getHeaderVersion()); Assertions.assertArrayEquals(extension.getHash(), extension.getHash()); } + + @Test + public void invalidDecode() { + byte version = 0; + byte[] logsBloom = new byte[Bloom.BLOOM_BYTES]; + short[] edges = { 1, 2, 3, 4 }; + + BlockHeaderExtension decoded = BlockHeaderExtension.fromEncoded( + new RLPList(RLP.encodeList( + RLP.encodeByte(version), + RLP.encodeElement(logsBloom), + ByteUtil.shortsToRLP(edges) + ), 0) + ); + + Assertions.assertNull(decoded); + } } diff --git a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java index 4a033579fe8..38a57d0c6cd 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java @@ -419,7 +419,7 @@ public void encodeForLogsBloomField() { Assertions.assertEquals(0x1, logsBloomField[0]); Assertions.assertArrayEquals(header.getExtension().getHash(), Arrays.copyOfRange(logsBloomField, 1, 33)); for (byte b:Arrays.copyOfRange(logsBloomField, 33, Bloom.BLOOM_BYTES)) Assertions.assertEquals(0x0, b); - Assertions.assertEquals(logsBloomField.length, Bloom.BLOOM_BYTES); + Assertions.assertEquals(Bloom.BLOOM_BYTES, logsBloomField.length); } private BlockHeaderV1 createV1FromV0(BlockHeaderV0 headerV0) { diff --git a/rskj-core/src/test/java/co/rsk/core/BlockHeaderV1Test.java b/rskj-core/src/test/java/co/rsk/core/BlockHeaderV1Test.java index 670702d67bf..b27cb9f0256 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockHeaderV1Test.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockHeaderV1Test.java @@ -77,7 +77,7 @@ void logsBloomFieldEncoded() { byte[] field = RLP.decode2(header.getLogsBloomFieldEncoded()).get(0).getRLPData(); Assertions.assertEquals((byte) 0x1, field[0]); for (int i = 33; i < 256; i++) Assertions.assertEquals((byte) 0x0, field[i]); - Assertions.assertEquals(field.length, Bloom.BLOOM_BYTES); + Assertions.assertEquals(Bloom.BLOOM_BYTES, field.length); } BlockHeaderV1 encodedHeaderWithRandomLogsBloom() { diff --git a/rskj-core/src/test/java/co/rsk/core/bc/ParallelizeTransactionHandlerTest.java b/rskj-core/src/test/java/co/rsk/core/bc/ParallelizeTransactionHandlerTest.java index 8ea027c2ac9..3cd9c52e6d6 100644 --- a/rskj-core/src/test/java/co/rsk/core/bc/ParallelizeTransactionHandlerTest.java +++ b/rskj-core/src/test/java/co/rsk/core/bc/ParallelizeTransactionHandlerTest.java @@ -714,6 +714,48 @@ void callGetGasUsedInWithAnInvalidSublistShouldThrowAnError2() { Assertions.assertTrue(true); } } + + @Test + void senderWritesAKeyAndReadsAnotherThatIsWrittenShouldGoToSequential() { + HashSet writeKeyX = createASetAndAddKeys(aWrappedKey); + HashSet writeKeyY = createASetAndAddKeys(aDifferentWrapperKey); + HashSet readKeyY = createASetAndAddKeys(aDifferentWrapperKey); + + Account senderA = new AccountBuilder().name("sender1").build(); + Account senderB = new AccountBuilder().name("sender2").build(); + + Transaction a_writes_x = new TransactionBuilder().nonce(1).sender(senderA).value(BigInteger.valueOf(0)).gasLimit(BigInteger.valueOf(16000)).build(); + Transaction b_writes_y = new TransactionBuilder().nonce(1).sender(senderB).value(BigInteger.valueOf(0)).gasLimit(BigInteger.valueOf(16000)).build(); + Transaction a_reads_y = new TransactionBuilder().nonce(2).sender(senderA).value(BigInteger.valueOf(0)).gasLimit(BigInteger.valueOf(16000)).build(); + + handler.addTransaction(a_writes_x, new HashSet<>(), writeKeyX, 1000); + handler.addTransaction(b_writes_y, new HashSet<>(), writeKeyY, 1000); + handler.addTransaction(a_reads_y, readKeyY, new HashSet<>(), 1000); + + Assertions.assertArrayEquals(new short[]{ 1, 2 }, handler.getTransactionsPerSublistInOrder()); + } + + @Test + void senderWritesAKeyAndReadsAnotherThatIsWrittenShouldGoToSequentialIfReadingOtherKeys() { + ByteArrayWrapper anotherKey = new ByteArrayWrapper(new byte[]{ 7, 7, 7 }); + HashSet writeKeyX = createASetAndAddKeys(aWrappedKey); + HashSet writeKeyYAndAnother = createASetAndAddKeys(anotherKey, aDifferentWrapperKey); + HashSet readKeyYAndAnother = createASetAndAddKeys(anotherKey, aDifferentWrapperKey); + + Account senderA = new AccountBuilder().name("sender1").build(); + Account senderB = new AccountBuilder().name("sender2").build(); + + Transaction a_writes_x = new TransactionBuilder().nonce(1).sender(senderA).value(BigInteger.valueOf(0)).gasLimit(BigInteger.valueOf(16000)).build(); + Transaction b_writes_y = new TransactionBuilder().nonce(1).sender(senderB).value(BigInteger.valueOf(0)).gasLimit(BigInteger.valueOf(16000)).build(); + Transaction a_reads_y = new TransactionBuilder().nonce(2).sender(senderA).value(BigInteger.valueOf(0)).gasLimit(BigInteger.valueOf(16000)).build(); + + handler.addTransaction(a_writes_x, new HashSet<>(), writeKeyX, 1000); + handler.addTransaction(b_writes_y, new HashSet<>(), writeKeyYAndAnother, 1000); + handler.addTransaction(a_reads_y, readKeyYAndAnother, new HashSet<>(), 1000); + + Assertions.assertArrayEquals(new short[]{ 1, 2 }, handler.getTransactionsPerSublistInOrder()); + } + private HashSet createASetAndAddKeys(ByteArrayWrapper... aKey) { return new HashSet<>(Arrays.asList(aKey)); }