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

[ETCM-284] Pay no money to coinbase address when processing checkpoint blocks #767

Merged
merged 1 commit into from
Nov 3, 2020
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
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],
ntallar marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -2,43 +2,10 @@ package io.iohk.ethereum.checkpointing

import akka.util.ByteString
import io.iohk.ethereum.crypto.ECDSASignature
import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields.HefPostEcip1097
import io.iohk.ethereum.domain._
import io.iohk.ethereum.ledger.BloomFilter
import org.bouncycastle.crypto.AsymmetricCipherKeyPair
import io.iohk.ethereum.crypto.ECDSASignatureImplicits.ECDSASignatureOrdering
import org.bouncycastle.crypto.AsymmetricCipherKeyPair

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

Choose a reason for hiding this comment

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

Could we also remove the one below?


def createBlockHeaderWithCheckpoint(
parentHeader: BlockHeader,
checkpoint: Checkpoint
): BlockHeader = {
BlockHeader(
parentHash = parentHeader.hash,
beneficiary = BlockHeader.EmptyBeneficiary,
stateRoot = parentHeader.stateRoot,
ommersHash = BlockHeader.EmptyOmmers,
transactionsRoot = BlockHeader.EmptyMpt,
receiptsRoot = BlockHeader.EmptyMpt,
logsBloom = BloomFilter.EmptyBloomFilter,
difficulty = parentHeader.difficulty,
number = parentHeader.number + 1,
gasLimit = parentHeader.gasLimit,
gasUsed = UInt256.Zero,
unixTimestamp = parentHeader.unixTimestamp + 1,
extraData = ByteString.empty,
mixHash = ByteString.empty,
nonce = ByteString.empty,
extraFields = HefPostEcip1097(treasuryOptOut = false, checkpoint = Some(checkpoint))
)
}

def createCheckpointSignatures(
keys: Seq[AsymmetricCipherKeyPair],
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
5 changes: 2 additions & 3 deletions src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package io.iohk.ethereum.domain

import akka.util.ByteString
import io.iohk.ethereum.{Fixtures, ObjectGenerators}
import io.iohk.ethereum.blockchain.sync.EphemBlockchainTestSetup
import io.iohk.ethereum.checkpointing.CheckpointingTestHelpers
import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator
import io.iohk.ethereum.db.dataSource.EphemDataSource
import io.iohk.ethereum.db.storage.StateStorage
import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields.HefPostEcip1097
import io.iohk.ethereum.mpt.MerklePatriciaTrie
import io.iohk.ethereum.{Fixtures, ObjectGenerators}
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers

Expand Down Expand Up @@ -56,7 +55,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