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 Oct 30, 2020
1 parent 4e2c38c commit 104775c
Show file tree
Hide file tree
Showing 12 changed files with 191 additions and 46 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
47 changes: 34 additions & 13 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 @@ -118,7 +139,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
3 changes: 2 additions & 1 deletion src/main/scala/io/iohk/ethereum/ledger/BlockPreparator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class BlockPreparator(
* @param worldStateProxy the initial state
* @return the state after paying the appropriate reward to who corresponds
*/
// scalastyle:off method.length
private[ledger] def payBlockReward(
block: Block,
worldStateProxy: InMemoryWorldStateProxy
Expand Down Expand Up @@ -105,7 +106,7 @@ class BlockPreparator(
increaseAccountBalance(ommerAddress, UInt256(ommerReward))(ws)
}
}

// scalastyle:on
/**
* v0 ≡ Tg (Tx gas limit) * Tp (Tx gas price). See YP equation number (68)
*
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
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 @@ -12,12 +12,8 @@ import io.iohk.ethereum.crypto.kec256
import io.iohk.ethereum.domain._
import io.iohk.ethereum.jsonrpc.EthService._
import io.iohk.ethereum.jsonrpc.FilterManager.LogFilterLogs
import io.iohk.ethereum.jsonrpc.serialization.JsonSerializers.{
OptionNoneToJNullSerializer,
QuantitiesSerializer,
UnformattedDataJsonSerializer
}
import io.iohk.ethereum.jsonrpc.PersonalService._
import io.iohk.ethereum.jsonrpc.serialization.JsonSerializers.{OptionNoneToJNullSerializer, QuantitiesSerializer, UnformattedDataJsonSerializer}
import io.iohk.ethereum.ommers.OmmersPool
import io.iohk.ethereum.ommers.OmmersPool.Ommers
import io.iohk.ethereum.testing.ActorsTesting.simpleAutoPilot
Expand Down Expand Up @@ -110,6 +106,48 @@ class JsonRpcControllerEthSpec
response should haveResult(expectedBlockResponse)
}

it should "handle eth_getBlockByHash request (block with checkpoint)" in new JsonRpcControllerFixture {
val blockToRequest = blockWithCheckpoint
val blockTd = blockToRequest.header.difficulty

blockchain
.storeBlock(blockToRequest)
.and(blockchain.storeTotalDifficulty(blockToRequest.header.hash, blockTd))
.commit()

val request = newJsonRpcRequest(
"eth_getBlockByHash",
List(JString(s"0x${blockToRequest.header.hashAsHexString}"), JBool(false))
)
val response = jsonRpcController.handleRequest(request).futureValue

val expectedBlockResponse =
Extraction.decompose(BlockResponse(blockToRequest, fullTxs = false, totalDifficulty = Some(blockTd)))

response should haveResult(expectedBlockResponse)
}

it should "handle eth_getBlockByHash request (block with treasuryOptOut)" in new JsonRpcControllerFixture {
val blockToRequest = blockWithTreasuryOptOut
val blockTd = blockToRequest.header.difficulty

blockchain
.storeBlock(blockToRequest)
.and(blockchain.storeTotalDifficulty(blockToRequest.header.hash, blockTd))
.commit()

val request = newJsonRpcRequest(
"eth_getBlockByHash",
List(JString(s"0x${blockToRequest.header.hashAsHexString}"), JBool(false))
)
val response = jsonRpcController.handleRequest(request).futureValue

val expectedBlockResponse =
Extraction.decompose(BlockResponse(blockToRequest, fullTxs = false, totalDifficulty = Some(blockTd)))

response should haveResult(expectedBlockResponse)
}

it should "handle eth_getBlockByNumber request" in new JsonRpcControllerFixture {
val blockToRequest = Block(Fixtures.Blocks.Block3125369.header, Fixtures.Blocks.Block3125369.body)
val blockTd = blockToRequest.header.difficulty
Expand All @@ -131,6 +169,48 @@ class JsonRpcControllerEthSpec
response should haveResult(expectedBlockResponse)
}

it should "handle eth_getBlockByNumber request (block with treasuryOptOut)" in new JsonRpcControllerFixture {
val blockToRequest = blockWithTreasuryOptOut
val blockTd = blockToRequest.header.difficulty

blockchain
.storeBlock(blockToRequest)
.and(blockchain.storeTotalDifficulty(blockToRequest.header.hash, blockTd))
.commit()

val request = newJsonRpcRequest(
"eth_getBlockByNumber",
List(JString(s"0x${Hex.toHexString(blockToRequest.header.number.toByteArray)}"), JBool(false))
)
val response = jsonRpcController.handleRequest(request).futureValue

val expectedBlockResponse =
Extraction.decompose(BlockResponse(blockToRequest, fullTxs = false, totalDifficulty = Some(blockTd)))

response should haveResult(expectedBlockResponse)
}

it should "handle eth_getBlockByNumber request (block with checkpoint)" in new JsonRpcControllerFixture {
val blockToRequest = blockWithCheckpoint
val blockTd = blockToRequest.header.difficulty

blockchain
.storeBlock(blockToRequest)
.and(blockchain.storeTotalDifficulty(blockToRequest.header.hash, blockTd))
.commit()

val request = newJsonRpcRequest(
"eth_getBlockByNumber",
List(JString(s"0x${Hex.toHexString(blockToRequest.header.number.toByteArray)}"), JBool(false))
)
val response = jsonRpcController.handleRequest(request).futureValue

val expectedBlockResponse =
Extraction.decompose(BlockResponse(blockToRequest, fullTxs = false, totalDifficulty = Some(blockTd)))

response should haveResult(expectedBlockResponse)
}

it should "handle eth_getUncleByBlockHashAndIndex request" in new JsonRpcControllerFixture {
val uncle = Fixtures.Blocks.DaoForkBlock.header
val blockToRequest = Block(Fixtures.Blocks.Block3125369.header, BlockBody(Nil, Seq(uncle)))
Expand Down
Loading

0 comments on commit 104775c

Please sign in to comment.