Core: fix an issue in BlockchainPerformance

semuxproject · Jul 6, 2019 · c29a2cb · c29a2cb
1 parent e7cdf8e
commit c29a2cb
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 29 deletions.
diff --git a/src/main/java/org/semux/consensus/SemuxBft.java b/src/main/java/org/semux/consensus/SemuxBft.java
@@ -35,7 +35,6 @@
 import org.semux.core.Transaction;
 import org.semux.core.TransactionExecutor;
 import org.semux.core.TransactionResult;
-import org.semux.core.TransactionType;
 import org.semux.core.state.AccountState;
 import org.semux.core.state.DelegateState;
 import org.semux.crypto.Hash;
@@ -772,7 +771,8 @@ protected Block proposeBlock() {
         byte[] data = chain.constructBlockData();
 
         // fetch pending transactions
-        final List<PendingManager.PendingTransaction> pending = pendingMgr.getPendingTransactions(config.poolBlockGasLimit());
+        final List<PendingManager.PendingTransaction> pending = pendingMgr
+                .getPendingTransactions(config.poolBlockGasLimit());
         final List<Transaction> pendingTxs = new ArrayList<>();
         final List<TransactionResult> pendingResults = new ArrayList<>();
 
@@ -792,7 +792,8 @@ protected Block proposeBlock() {
         for (PendingManager.PendingTransaction pendingTx : pending) {
             Transaction tx = pendingTx.transaction;
             if (tx.isVMTransaction()) {
-                // Note: transactions that exceed the remaining block gas limit are ignored; however,
+                // Note: transactions that exceed the remaining block gas limit are ignored;
+                // however,
                 // this doesn't mean they're invalid because the gas usage will be less
                 // than gas limit.
                 if (tx.getGas() <= myRemainingBlockGas) {
@@ -854,7 +855,7 @@ protected boolean validateBlockProposal(Block block) {
 
         // [1] check block header
         Block latest = chain.getLatestBlock();
-        if (!block.validateHeader(latest.getHeader(), header)) {
+        if (!block.validateHeader(header, latest.getHeader())) {
             logger.warn("Invalid block header");
             return false;
         }

diff --git a/src/main/java/org/semux/consensus/SemuxSync.java b/src/main/java/org/semux/consensus/SemuxSync.java
@@ -540,7 +540,7 @@ protected boolean validateBlock(Block block, AccountState asSnapshot, DelegateSt
 
         // [1] check block header
         Block latest = chain.getLatestBlock();
-        if (!block.validateHeader(latest.getHeader(), header)) {
+        if (!block.validateHeader(header, latest.getHeader())) {
             logger.error("Invalid block header");
             return false;
         }

diff --git a/src/main/java/org/semux/core/Block.java b/src/main/java/org/semux/core/Block.java
@@ -139,10 +139,10 @@ public void setVotes(List<Signature> votes) {
      * Validates block header.
      *
      * @param header
-     * @param previous
+     * @param parentHeader
      * @return
      */
-    public boolean validateHeader(BlockHeader previous, BlockHeader header) {
+    public boolean validateHeader(BlockHeader header, BlockHeader parentHeader) {
         if (header == null) {
             logger.warn("Header was null.");
             return false;
@@ -153,17 +153,17 @@ public boolean validateHeader(BlockHeader previous, BlockHeader header) {
             return false;
         }
 
-        if (header.getNumber() != previous.getNumber() + 1) {
+        if (header.getNumber() != parentHeader.getNumber() + 1) {
             logger.warn("Header number was not one greater than previous block.");
             return false;
         }
 
-        if (!Arrays.equals(header.getParentHash(), previous.getHash())) {
+        if (!Arrays.equals(header.getParentHash(), parentHeader.getHash())) {
             logger.warn("Header parent hash was not equal to previous block hash.");
             return false;
         }
 
-        if (header.getTimestamp() <= previous.getTimestamp()) {
+        if (header.getTimestamp() <= parentHeader.getTimestamp()) {
             logger.warn("Header timestamp was before previous block.");
             return false;
         }
@@ -229,7 +229,7 @@ public boolean validateTransactions(BlockHeader header, Collection<Transaction>
      * @param results
      * @return
      */
-    public boolean validateResults(BlockHeader header, List<TransactionResult> results) {
+    public static boolean validateResults(BlockHeader header, List<TransactionResult> results) {
         // validate results
         for (TransactionResult result : results) {
             if (result.getCode().isRejected()) {

diff --git a/src/main/java/org/semux/core/PendingManager.java b/src/main/java/org/semux/core/PendingManager.java
@@ -215,7 +215,8 @@ public synchronized List<PendingTransaction> getPendingTransactions(long blockGa
         while (it.hasNext() && blockGasLimit > 0) {
             PendingTransaction tx = it.next();
 
-            long gasUsage = tx.transaction.isVMTransaction() ? tx.result.getGasUsed() : kernel.getConfig().spec().nonVMTransactionGasCost();
+            long gasUsage = tx.transaction.isVMTransaction() ? tx.result.getGasUsed()
+                    : kernel.getConfig().spec().nonVMTransactionGasCost();
             if (blockGasLimit > gasUsage) {
                 txs.add(tx);
                 blockGasLimit -= gasUsage;

diff --git a/src/test/java/org/semux/bench/BlockchainPerformance.java b/src/test/java/org/semux/bench/BlockchainPerformance.java
@@ -46,10 +46,8 @@ public static Block testBlockCreation() {
         List<Transaction> txs = new ArrayList<>();
         List<TransactionResult> res = new ArrayList<>();
 
-        // TODO: use block gas limit
-
-        int total = 0;
-        for (int i = 0;; i++) {
+        long remainingBlockGas = config.spec().maxBlockGasLimit();
+        for (int i = 0; remainingBlockGas >= config.spec().nonVMTransactionGasCost(); i++) {
             Network network = config.network();
             TransactionType type = TransactionType.TRANSFER;
             byte[] to = Bytes.random(20);
@@ -59,14 +57,10 @@ public static Block testBlockCreation() {
             long timestamp = TimeUtil.currentTimeMillis();
             byte[] data = Bytes.EMPTY_BYTES;
             Transaction tx = new Transaction(network, type, to, value, fee, nonce, timestamp, data).sign(key);
-
-            if (total + tx.size() > 1024 * 1024) {
-                break;
-            }
-
             txs.add(tx);
             res.add(new TransactionResult());
-            total += tx.size();
+
+            remainingBlockGas -= config.spec().nonVMTransactionGasCost();
         }
 
         long number = 1;
@@ -101,12 +95,12 @@ public static Block testBlockCreation() {
     }
 
     public static void testBlockValidation(Block block) {
-        Genesis gen = Genesis.load(Network.DEVNET);
+        Genesis genesis = Genesis.load(Network.DEVNET);
 
         long t1 = System.nanoTime();
-        block.validateHeader(gen.getHeader(), block.getHeader());
-        block.validateTransactions(gen.getHeader(), block.getTransactions(), config.network());
-        block.validateResults(gen.getHeader(), block.getResults());
+        block.validateHeader(block.getHeader(), genesis.getHeader());
+        block.validateTransactions(block.getHeader(), block.getTransactions(), config.network());
+        block.validateResults(block.getHeader(), block.getResults());
         // block votes validation skipped
         long t2 = System.nanoTime();
 

diff --git a/src/test/java/org/semux/core/BlockTest.java b/src/test/java/org/semux/core/BlockTest.java
@@ -118,7 +118,7 @@ public void testValidateTransactions() {
                 resultsRoot, stateRoot, data);
         Block block = new Block(header, transactions);
 
-        assertTrue(block.validateHeader(previousHeader, header));
+        assertTrue(block.validateHeader(header, previousHeader));
         assertTrue(block.validateTransactions(previousHeader, transactions, Network.DEVNET));
         assertTrue(block.validateResults(previousHeader, results));
     }
@@ -131,7 +131,7 @@ public void testValidateTransactionsSparse() {
                 resultsRoot, stateRoot, data);
         Block block = new Block(header, transactions);
 
-        assertTrue(block.validateHeader(previousHeader, header));
+        assertTrue(block.validateHeader(header, previousHeader));
         assertTrue(block.validateTransactions(previousHeader, Collections.singleton(transactions.get(0)), transactions,
                 Network.DEVNET));
         assertTrue(block.validateResults(previousHeader, results));

diff --git a/src/test/java/org/semux/integration/SyncingTest.java b/src/test/java/org/semux/integration/SyncingTest.java
@@ -140,7 +140,7 @@ public void testSync() throws IOException {
         for (int i = 1; i <= targetHeight(); i++) {
             Block block = kernel4.getBlockchain().getBlock(i);
             Block previousBlock = kernel4.getBlockchain().getBlock(i - 1);
-            assertTrue(block.validateHeader(previousBlock.getHeader(), block.getHeader()));
+            assertTrue(block.validateHeader(block.getHeader(), previousBlock.getHeader()));
             assertTrue(block.validateTransactions(previousBlock.getHeader(), block.getTransactions(),
                     kernel4.getConfig().network()));
             assertTrue(block.validateResults(previousBlock.getHeader(), block.getResults()));