Skip to content

Commit

Permalink
ETCM-284 block with checkpoint will not generate coinbase reward
Browse files Browse the repository at this point in the history
  • Loading branch information
pslaski committed Nov 2, 2020
1 parent 4e2c38c commit 32768dc
Show file tree
Hide file tree
Showing 15 changed files with 206 additions and 62 deletions.
8 changes: 4 additions & 4 deletions src/it/scala/io/iohk/ethereum/txExecTest/ContractTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ContractTest extends AnyFlatSpec with Matchers {
//block only with ether transfers
val blockValidation = new BlockValidation(consensus, blockchain, BlockQueue(blockchain, syncConfig))
val blockExecution = new BlockExecution(blockchain, blockchainConfig, consensus.blockPreparator, blockValidation)
blockExecution.executeBlock(fixtures.blockByNumber(1)) shouldBe noErrors
blockExecution.executeAndValidateBlock(fixtures.blockByNumber(1)) shouldBe noErrors
}

it should "deploy contract" in new ScenarioSetup {
Expand All @@ -37,7 +37,7 @@ class ContractTest extends AnyFlatSpec with Matchers {
//contract creation
val blockValidation = new BlockValidation(consensus, blockchain, BlockQueue(blockchain, syncConfig))
val blockExecution = new BlockExecution(blockchain, blockchainConfig, consensus.blockPreparator, blockValidation)
blockExecution.executeBlock(fixtures.blockByNumber(2)) shouldBe noErrors
blockExecution.executeAndValidateBlock(fixtures.blockByNumber(2)) shouldBe noErrors
}

it should "execute contract call" in new ScenarioSetup {
Expand All @@ -48,7 +48,7 @@ class ContractTest extends AnyFlatSpec with Matchers {
//block with ether transfers and contract call
val blockValidation = new BlockValidation(consensus, blockchain, BlockQueue(blockchain, syncConfig))
val blockExecution = new BlockExecution(blockchain, blockchainConfig, consensus.blockPreparator, blockValidation)
blockExecution.executeBlock(fixtures.blockByNumber(3)) shouldBe noErrors
blockExecution.executeAndValidateBlock(fixtures.blockByNumber(3)) shouldBe noErrors
}

it should "execute contract that pays 2 accounts" in new ScenarioSetup {
Expand All @@ -59,6 +59,6 @@ class ContractTest extends AnyFlatSpec with Matchers {
//block contains contract paying 2 accounts
val blockValidation = new BlockValidation(consensus, blockchain, BlockQueue(blockchain, syncConfig))
val blockExecution = new BlockExecution(blockchain, blockchainConfig, consensus.blockPreparator, blockValidation)
blockExecution.executeBlock(fixtures.blockByNumber(3)) shouldBe noErrors
blockExecution.executeAndValidateBlock(fixtures.blockByNumber(3)) shouldBe noErrors
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class ECIP1017Test extends AnyFlatSpec with Matchers {
val blockchain = BlockchainImpl(storages)
val blockValidation = new BlockValidation(consensus, blockchain, BlockQueue(blockchain, syncConfig))
val blockExecution = new BlockExecution(blockchain, blockchainConfig, consensus.blockPreparator, blockValidation)
blockExecution.executeBlock(fixtures.blockByNumber(blockToExecute)) shouldBe noErrors
blockExecution.executeAndValidateBlock(fixtures.blockByNumber(blockToExecute)) shouldBe noErrors
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/it/scala/io/iohk/ethereum/txExecTest/ForksTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class ForksTest extends AnyFlatSpec with Matchers {
val blockchain = BlockchainImpl(storages)
val blockValidation = new BlockValidation(consensus, blockchain, BlockQueue(blockchain, syncConfig))
val blockExecution = new BlockExecution(blockchain, blockchainConfig, consensus.blockPreparator, blockValidation)
blockExecution.executeBlock(fixtures.blockByNumber(blockToExecute)) shouldBe noErrors
blockExecution.executeAndValidateBlock(fixtures.blockByNumber(blockToExecute)) shouldBe noErrors
}
}

Expand Down
15 changes: 14 additions & 1 deletion src/main/scala/io/iohk/ethereum/jsonrpc/BlockResponse.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package io.iohk.ethereum.jsonrpc

import akka.util.ByteString
import io.iohk.ethereum.domain.{Block, BlockHeader, BlockBody}
import io.iohk.ethereum.crypto.ECDSASignature
import io.iohk.ethereum.domain.{Block, BlockBody, BlockHeader}

case class CheckpointResponse(signatures: Seq[ECDSASignature], signers: Seq[ByteString])

case class BlockResponse(
number: BigInt,
Expand All @@ -21,6 +24,8 @@ case class BlockResponse(
gasLimit: BigInt,
gasUsed: BigInt,
timestamp: BigInt,
checkpoint: Option[CheckpointResponse],
treasuryOptOut: Option[Boolean],
transactions: Either[Seq[ByteString], Seq[TransactionResponse]],
uncles: Seq[ByteString]
)
Expand All @@ -41,6 +46,12 @@ object BlockResponse {
else
Left(block.body.transactionList.map(_.hash))

val checkpoint = block.header.checkpoint.map { checkpoint =>
val signers = checkpoint.signatures.flatMap(_.publicKey(block.header.parentHash))

CheckpointResponse(checkpoint.signatures, signers)
}

BlockResponse(
number = block.header.number,
hash = if (pendingBlock) None else Some(block.header.hash),
Expand All @@ -59,6 +70,8 @@ object BlockResponse {
gasLimit = block.header.gasLimit,
gasUsed = block.header.gasUsed,
timestamp = block.header.unixTimestamp,
checkpoint = checkpoint,
treasuryOptOut = block.header.treasuryOptOut,
transactions = transactions,
uncles = block.body.uncleNodesList.map(_.hash)
)
Expand Down
54 changes: 39 additions & 15 deletions src/main/scala/io/iohk/ethereum/ledger/BlockExecution.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,36 @@ class BlockExecution(
blockValidation: BlockValidation
) extends Logger {

/** Executes a block
/** Executes and validate a block
*
* @param alreadyValidated should we skip pre-execution validation (if the block has already been validated,
* eg. in the importBlock method)
*/
def executeBlock(block: Block, alreadyValidated: Boolean = false): Either[BlockExecutionError, Seq[Receipt]] = {
def executeAndValidateBlock(
block: Block,
alreadyValidated: Boolean = false
): Either[BlockExecutionError, Seq[Receipt]] = {
val preExecValidationResult =
if (alreadyValidated) Right(block) else blockValidation.validateBlockBeforeExecution(block)

val blockExecResult = for {
_ <- preExecValidationResult
execResult <- executeBlockTransactions(block)
BlockResult(resultingWorldStateProxy, gasUsed, receipts) = execResult
worldToPersist = blockPreparator.payBlockReward(block, resultingWorldStateProxy)
// State root hash needs to be up-to-date for validateBlockAfterExecution
worldPersisted = InMemoryWorldStateProxy.persistState(worldToPersist)
_ <- blockValidation.validateBlockAfterExecution(block, worldPersisted.stateRootHash, receipts, gasUsed)

} yield receipts
val blockExecResult = {
if (block.hasCheckpoint) {
// block with checkpoint is not executed normally - it's not need to do after execution validation
preExecValidationResult
.map(_ => Seq.empty[Receipt])
} else {
for {
_ <- preExecValidationResult
result <- executeBlock(block)
_ <- blockValidation.validateBlockAfterExecution(
block,
result.worldState.stateRootHash,
result.receipts,
result.gasUsed
)
} yield result.receipts
}
}

if (blockExecResult.isRight) {
log.debug(s"Block ${block.header.number} (with hash: ${block.header.hashAsHexString}) executed correctly")
Expand All @@ -41,6 +52,16 @@ class BlockExecution(
blockExecResult
}

/** Executes a block (executes transactions and pays rewards) */
private def executeBlock(block: Block): Either[BlockExecutionError, BlockResult] = {
for {
execResult <- executeBlockTransactions(block)
worldToPersist = blockPreparator.payBlockReward(block, execResult.worldState)
// State root hash needs to be up-to-date for validateBlockAfterExecution
worldPersisted = InMemoryWorldStateProxy.persistState(worldToPersist)
} yield execResult.copy(worldState = worldPersisted)
}

/** This function runs transactions
*
* @param block the block with transactions to run
Expand Down Expand Up @@ -97,14 +118,17 @@ class BlockExecution(
}
}

/** Executes a list of blocks, storing the results in the blockchain.
/** Executes and validates a list of blocks, storing the results in the blockchain.
*
* @param blocks blocks to be executed
* @param parentTd transaction difficulty of the parent
*
* @return a list of blocks that were correctly executed and an optional [[BlockExecutionError]]
*/
def executeBlocks(blocks: List[Block], parentTd: BigInt): (List[BlockData], Option[BlockExecutionError]) = {
def executeAndValidateBlocks(
blocks: List[Block],
parentTd: BigInt
): (List[BlockData], Option[BlockExecutionError]) = {
@tailrec
def go(
executedBlocks: List[BlockData],
Expand All @@ -118,7 +142,7 @@ class BlockExecution(
(executedBlocks, error)
} else {
val blockToExecute = remainingBlocks.head
executeBlock(blockToExecute, alreadyValidated = true) match {
executeAndValidateBlock(blockToExecute, alreadyValidated = true) match {
case Right(receipts) =>
val td = parentTd + blockToExecute.header.difficulty
val newBlockData = BlockData(blockToExecute, receipts, td)
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class BlockImport(
val executionResult = for {
topBlock <- blockQueue.enqueueBlock(block, bestBlockNumber)
topBlocks = blockQueue.getBranch(topBlock.hash, dequeue = true)
(executed, errors) = blockExecution.executeBlocks(topBlocks, currentTd)
(executed, errors) = blockExecution.executeAndValidateBlocks(topBlocks, currentTd)
} yield (executed, errors, topBlocks)

executionResult match {
Expand Down Expand Up @@ -190,7 +190,7 @@ class BlockImport(
parentTd: BigInt,
oldBlocksData: List[BlockData]
): Either[BlockExecutionError, (List[Block], List[Block])] = {
val (executedBlocks, maybeError) = blockExecution.executeBlocks(newBranch, parentTd)
val (executedBlocks, maybeError) = blockExecution.executeAndValidateBlocks(newBranch, parentTd)
maybeError match {
case None =>
Right(oldBlocksData.map(_.block), executedBlocks.map(_.block))
Expand Down
2 changes: 1 addition & 1 deletion src/snappy/scala/io/iohk/ethereum/snappy/SnappyTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class SnappyTest extends AnyFreeSpec with Matchers with Logger {
val blockValidation = new BlockValidation(consensus, blockchain, BlockQueue(blockchain, syncConfig))
val blockExecution =
new BlockExecution(blockchain, blockchainConfig, consensus.blockPreparator, blockValidation)
blockExecution.executeBlock(block)
blockExecution.executeAndValidateBlock(block)

case None =>
// this seems to discard failures, for better errors messages we might want to implement a different method (simulateBlock?)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,6 @@ import org.bouncycastle.crypto.AsymmetricCipherKeyPair
import io.iohk.ethereum.crypto.ECDSASignatureImplicits.ECDSASignatureOrdering

object CheckpointingTestHelpers {
def createBlockWithCheckpoint(
parentHeader: BlockHeader,
checkpoint: Checkpoint
): Block = {
Block(createBlockHeaderWithCheckpoint(parentHeader, checkpoint), BlockBody(Nil, Nil))
}

def createBlockHeaderWithCheckpoint(
parentHeader: BlockHeader,
Expand Down
20 changes: 10 additions & 10 deletions src/test/scala/io/iohk/ethereum/consensus/BlockGeneratorSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
fullBlock.header,
blockchain.getBlockHeaderByHash
) shouldBe Right(BlockHeaderValid)
blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.header.extraData shouldBe headerExtraData
}

Expand All @@ -67,7 +67,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
fullBlock.header,
blockchain.getBlockHeaderByHash
) shouldBe Right(BlockHeaderValid)
blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.header.extraData shouldBe headerExtraData
}

Expand Down Expand Up @@ -135,7 +135,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
blockchain.getBlockHeaderByHash
) shouldBe Right(BlockHeaderValid)

blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.body.transactionList shouldBe Seq(signedTransaction.tx)
fullBlock.header.extraData shouldBe headerExtraData
}
Expand Down Expand Up @@ -166,7 +166,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
fullBlock.header,
blockchain.getBlockHeaderByHash
) shouldBe Right(BlockHeaderValid)
blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.body.transactionList shouldBe Seq(signedTransaction.tx)
fullBlock.header.extraData shouldBe headerExtraData
}
Expand Down Expand Up @@ -231,7 +231,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
fullBlock.header,
blockchain.getBlockHeaderByHash
) shouldBe Right(BlockHeaderValid)
blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.body.transactionList shouldBe Seq(generalTx)
fullBlock.header.extraData shouldBe headerExtraData
}
Expand Down Expand Up @@ -289,7 +289,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
val generatedBlock =
blockGenerator.generateBlock(bestBlock, Seq(generalTx), Address(testAddress), blockGenerator.emptyX)

blockExecution.executeBlock(generatedBlock.block, true) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(generatedBlock.block, true) shouldBe a[Right[_, Seq[Receipt]]]
}

it should "generate block after eip155 and allow both chain specific and general transactions" in new TestSetup {
Expand All @@ -315,7 +315,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
validators.blockHeaderValidator.validate(fullBlock.header, blockchain.getBlockHeaderByHash) shouldBe Right(
BlockHeaderValid
)
blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.body.transactionList shouldBe Seq(signedTransaction.tx, generalTx)
fullBlock.header.extraData shouldBe headerExtraData
}
Expand Down Expand Up @@ -344,7 +344,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
validators.blockHeaderValidator.validate(fullBlock.header, blockchain.getBlockHeaderByHash) shouldBe Right(
BlockHeaderValid
)
blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.body.transactionList shouldBe Seq(signedTransaction.tx, nextTransaction)
fullBlock.header.extraData shouldBe headerExtraData
}
Expand Down Expand Up @@ -386,7 +386,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
validators.blockHeaderValidator.validate(fullBlock.header, blockchain.getBlockHeaderByHash) shouldBe Right(
BlockHeaderValid
)
blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.body.transactionList shouldBe Seq(signedTransaction.tx, nextTransaction)
fullBlock.header.extraData shouldBe headerExtraData
}
Expand Down Expand Up @@ -415,7 +415,7 @@ class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
validators.blockHeaderValidator.validate(fullBlock.header, blockchain.getBlockHeaderByHash) shouldBe Right(
BlockHeaderValid
)
blockExecution.executeBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
blockExecution.executeAndValidateBlock(fullBlock) shouldBe a[Right[_, Seq[Receipt]]]
fullBlock.body.transactionList shouldBe Seq(signedTransaction.tx)
fullBlock.header.extraData shouldBe headerExtraData
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class BlockchainSpec extends AnyFlatSpec with Matchers {
val parent = Fixtures.Blocks.Genesis.block
blockchain.storeBlock(parent)

val validBlock = CheckpointingTestHelpers.createBlockWithCheckpoint(parent.header, checkpoint)
val validBlock = new CheckpointBlockGenerator().generate(parent, checkpoint)

blockchain.save(validBlock, Seq.empty, BigInt(0), saveAsBestBlock = true)

Expand Down
Loading

0 comments on commit 32768dc

Please sign in to comment.