diff --git a/src/main/java/org/semux/api/v2/TypeFactory.java b/src/main/java/org/semux/api/v2/TypeFactory.java index 75ec688fb..8a0f2e011 100644 --- a/src/main/java/org/semux/api/v2/TypeFactory.java +++ b/src/main/java/org/semux/api/v2/TypeFactory.java @@ -157,9 +157,7 @@ public static TransactionType transactionType(Transaction tx) { public static TransactionResultType transactionResultType(Transaction tx, TransactionResult result, long number) { // gas price is in nano sem, not wei - boolean isVMTransaction = (tx.getType() == org.semux.core.TransactionType.CREATE - || tx.getType() == org.semux.core.TransactionType.CALL); - Amount fee = isVMTransaction ? Amount.mul(result.getGasPrice(), result.getGasUsed()) : tx.getFee(); + Amount fee = tx.isVMTransaction() ? Amount.mul(result.getGasPrice(), result.getGasUsed()) : tx.getFee(); return new TransactionResultType() .blockNumber(Long.toString(number)) diff --git a/src/main/java/org/semux/consensus/SemuxBft.java b/src/main/java/org/semux/consensus/SemuxBft.java index 123a7a29e..659c52270 100644 --- a/src/main/java/org/semux/consensus/SemuxBft.java +++ b/src/main/java/org/semux/consensus/SemuxBft.java @@ -783,17 +783,20 @@ protected Block proposeBlock() { BlockHeader tempHeader = new BlockHeader(height, coinbase.toAddress(), prevHash, timestamp, new byte[0], new byte[0], new byte[0], data); - // only propose gas used up to configured block gas limit - long remainingBlockGas = config.poolBlockGasLimit(); SemuxBlock semuxBlock = new SemuxBlock(tempHeader, config.spec().maxBlockGasLimit()); + // only propose gas used up to configured block gas limit + long remainingBlockGas = semuxBlock.getGasLimit(); + long myRemainingBlockGas = config.poolBlockGasLimit(); + for (PendingManager.PendingTransaction pendingTx : pending) { Transaction tx = pendingTx.transaction; - boolean isVMTransaction = tx.getType() == TransactionType.CALL || tx.getType() == TransactionType.CREATE; - - if (isVMTransaction) { - // transactions that exceed the remaining block gas limit are ignored - if (tx.getGas() <= remainingBlockGas) { + if (tx.isVMTransaction()) { + // 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) { + // FIXME: transaction executor does not know the remaining gas in block TransactionResult result = exec.execute(tx, as, ds, semuxBlock, chain); // only include transaction that's acceptable @@ -801,15 +804,20 @@ protected Block proposeBlock() { pendingResults.add(result); pendingTxs.add(tx); - remainingBlockGas -= result.getGasUsed(); + myRemainingBlockGas -= result.getGasUsed(); } } } else { - if (config.spec().nonVMTransactionGasCost() <= remainingBlockGas + // FIXME: this is wrong because the results from pending manager may be wrong!!! + if (config.spec().nonVMTransactionGasCost() <= myRemainingBlockGas && pendingTx.result.getCode().isAcceptable()) { - pendingResults.add(pendingTx.result); - pendingTxs.add(pendingTx.transaction); - remainingBlockGas -= config.spec().nonVMTransactionGasCost(); + TransactionResult result = pendingTx.result; + + if (result.getCode().isAcceptable()) { + pendingResults.add(pendingTx.result); + pendingTxs.add(pendingTx.transaction); + myRemainingBlockGas -= config.spec().nonVMTransactionGasCost(); + } } } } @@ -954,7 +962,7 @@ protected void applyBlock(Block block) { List results = exec.execute(transactions, as, ds, new SemuxBlock(block.getHeader(), config.spec().maxBlockGasLimit()), chain); if (!block.validateResults(header, results)) { - logger.debug("Invalid transactions"); + logger.warn("Invalid transactions"); return; } diff --git a/src/main/java/org/semux/core/PendingManager.java b/src/main/java/org/semux/core/PendingManager.java index cef997e31..8ef5fedb4 100644 --- a/src/main/java/org/semux/core/PendingManager.java +++ b/src/main/java/org/semux/core/PendingManager.java @@ -209,22 +209,16 @@ public synchronized long getNonce(byte[] address) { * @return */ public synchronized List getPendingTransactions(long blockGasLimit) { - int byteLimit = 1024 * 1024; - - // TODO: include transactions based on the block gas limit - List txs = new ArrayList<>(); Iterator it = transactions.iterator(); - int size = 0; - while (it.hasNext()) { + while (it.hasNext() && blockGasLimit > 0) { PendingTransaction tx = it.next(); - size += tx.transaction.size(); - if (size > byteLimit) { - break; - } else { + long gasUsage = tx.transaction.isVMTransaction() ? tx.result.getGasUsed() : kernel.getConfig().spec().nonVMTransactionGasCost(); + if (blockGasLimit > gasUsage) { txs.add(tx); + blockGasLimit -= gasUsage; } } @@ -314,14 +308,13 @@ protected ProcessingResult processTransaction(Transaction tx, boolean relay) { int cnt = 0; long now = TimeUtil.currentTimeMillis(); - boolean isVMTransaction = tx.getType() == TransactionType.CALL || tx.getType() == TransactionType.CREATE; // reject VM transactions that come in before fork - if (isVMTransaction && !kernel.getBlockchain().isForkActivated(Fork.VIRTUAL_MACHINE)) { + if (tx.isVMTransaction() && !kernel.getBlockchain().isForkActivated(Fork.VIRTUAL_MACHINE)) { return new ProcessingResult(0, TransactionResult.Code.INVALID_TYPE); } // reject VM transaction with low gas price - if (isVMTransaction && tx.getGasPrice().lt(kernel.getConfig().poolMinGasPrice())) { + if (tx.isVMTransaction() && tx.getGasPrice().lt(kernel.getConfig().poolMinGasPrice())) { return new ProcessingResult(0, TransactionResult.Code.INVALID_FEE); } @@ -356,8 +349,8 @@ protected ProcessingResult processTransaction(Transaction tx, boolean relay) { Block prevBlock = chain.getLatestBlock(); BlockHeader blockHeader = new BlockHeader( prevBlock.getNumber() + 1, - new Key().toAddress(), prevBlock.getHash(), System.currentTimeMillis(), new byte[0], - new byte[0], new byte[0], new byte[0]); + new Key().toAddress(), prevBlock.getHash(), System.currentTimeMillis(), Bytes.EMPTY_BYTES, + Bytes.EMPTY_BYTES, Bytes.EMPTY_BYTES, Bytes.EMPTY_BYTES); SemuxBlock block = new SemuxBlock(blockHeader, kernel.getConfig().spec().maxBlockGasLimit()); // execute transactions diff --git a/src/main/java/org/semux/core/Transaction.java b/src/main/java/org/semux/core/Transaction.java index 9abd3a900..5df063747 100644 --- a/src/main/java/org/semux/core/Transaction.java +++ b/src/main/java/org/semux/core/Transaction.java @@ -125,6 +125,10 @@ public Transaction(Network network, TransactionType type, byte[] toAddress, Amou this(network, type, toAddress, value, fee, nonce, timestamp, data, 0, Amount.ZERO); } + public boolean isVMTransaction() { + return type == TransactionType.CREATE || type == TransactionType.CALL; + } + /** * Sign this transaction. * diff --git a/src/main/java/org/semux/core/TransactionExecutor.java b/src/main/java/org/semux/core/TransactionExecutor.java index 84199b886..e2586ca71 100644 --- a/src/main/java/org/semux/core/TransactionExecutor.java +++ b/src/main/java/org/semux/core/TransactionExecutor.java @@ -95,6 +95,7 @@ public List execute(List txs, AccountState as, D SemuxBlock block, Blockchain chain) { List results = new ArrayList<>(); + // FIXME: gas usage should be provided by caller long gasUsedInBlock = 0; for (Transaction tx : txs) { TransactionResult result = new TransactionResult(); @@ -118,16 +119,12 @@ public List execute(List txs, AccountState as, D continue; } - boolean isVMTransaction = type == TransactionType.CREATE || type == TransactionType.CALL; - // check fee (call and create use gas instead) - if (isVMTransaction) { + if (tx.isVMTransaction()) { // applying a very strict check to avoid mistakes boolean valid = fee.equals(Amount.ZERO) && tx.getGas() >= 21_000 && tx.getGas() <= config.spec().maxBlockGasLimit() - && tx.getGasPrice().getNano() >= 1 && tx.getGasPrice().getNano() <= Integer.MAX_VALUE; // a - // theoretical - // limit + && tx.getGasPrice().getNano() >= 1 && tx.getGasPrice().getNano() <= Integer.MAX_VALUE; if (!valid) { result.setCode(Code.INVALID_FEE); continue; @@ -145,6 +142,8 @@ public List execute(List txs, AccountState as, D continue; } + // TODO: check remaining gas + switch (type) { case TRANSFER: { if (fee.lte(available) && value.lte(available) && sum(value, fee).lte(available)) { @@ -232,12 +231,12 @@ public List execute(List txs, AccountState as, D } if (result.getCode().isAcceptable()) { - if (!isVMTransaction) { + if (!tx.isVMTransaction()) { // CREATEs and CALLs manages the nonce inside the VM as.increaseNonce(from); } - if (isVMTransaction) { + if (tx.isVMTransaction()) { gasUsedInBlock += result.getGasUsed(); } else { gasUsedInBlock += config.spec().nonVMTransactionGasCost();