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

[EC-231] ECIP 1017 Geth compatibility fixes and tests #307

Merged
merged 10 commits into from
Sep 26, 2017
640 changes: 640 additions & 0 deletions src/it/resources/txExecTest/ecip1017Test/bodies.txt

Large diffs are not rendered by default.

Empty file.
Empty file.
640 changes: 640 additions & 0 deletions src/it/resources/txExecTest/ecip1017Test/headers.txt

Large diffs are not rendered by default.

640 changes: 640 additions & 0 deletions src/it/resources/txExecTest/ecip1017Test/receipts.txt

Large diffs are not rendered by default.

1,073 changes: 1,073 additions & 0 deletions src/it/resources/txExecTest/ecip1017Test/stateTree.txt

Large diffs are not rendered by default.

63 changes: 63 additions & 0 deletions src/it/scala/io/iohk/ethereum/txExecTest/ECIP1017Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package io.iohk.ethereum.txExecTest

import akka.util.ByteString
import io.iohk.ethereum.domain.{BlockchainImpl, Receipt, UInt256}
import io.iohk.ethereum.ledger.LedgerImpl
import io.iohk.ethereum.txExecTest.util.FixtureProvider
import io.iohk.ethereum.utils.{BlockchainConfig, DaoForkConfig, MonetaryPolicyConfig}
import io.iohk.ethereum.validators._
import io.iohk.ethereum.vm.VM
import org.scalatest.{FlatSpec, Matchers}

class ECIP1017Test extends FlatSpec with Matchers {

val EraDuration = 3

val blockchainConfig = new BlockchainConfig {
override val monetaryPolicyConfig: MonetaryPolicyConfig = MonetaryPolicyConfig(EraDuration, 0.2, 5000000000000000000L)

// unused
override val chainId: Byte = 0x3d
override val frontierBlockNumber: BigInt = 0
override val homesteadBlockNumber: BigInt = 1150000
override val eip150BlockNumber: BigInt = 2500000
override val eip160BlockNumber: BigInt = 3000000
override val eip155BlockNumber: BigInt = 3000000
override val eip106BlockNumber: BigInt = Long.MaxValue
override val customGenesisFileOpt: Option[String] = None
override val daoForkConfig: Option[DaoForkConfig] = None
override val difficultyBombPauseBlockNumber: BigInt = Long.MaxValue
override val difficultyBombContinueBlockNumber: BigInt = Long.MaxValue
override val accountStartNonce: UInt256 = UInt256.Zero
}

val noErrors = a[Right[_, Seq[Receipt]]]

val validators = new Validators {
val blockValidator: BlockValidator = BlockValidator
val blockHeaderValidator: BlockHeaderValidator = new BlockHeaderValidatorImpl(blockchainConfig)
val ommersValidator: OmmersValidator = new OmmersValidatorImpl(blockchainConfig)
val signedTransactionValidator: SignedTransactionValidator = new SignedTransactionValidatorImpl(blockchainConfig)
}

/**
* Tests the block reward calculation through out all the monetary policy through all the eras till block
* mining reward goes to zero. Block mining reward is tested till era 200 (that starts at block number 602)
* as the reward reaches zero at era 193 (which starts at block number 579), given an eraDuration of 3.
*/
"Ledger" should "execute blocks with respect to block reward changed by ECIP 1017" in {
val fixtures: FixtureProvider.Fixture = FixtureProvider.loadFixtures("/txExecTest/ecip1017Test")

val startBlock = 1
val endBlock = 602
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 maybe add a comment about era 193 going zero, and how we want to go past that point.


(startBlock to endBlock) foreach { blockToExecute =>
val storages = FixtureProvider.prepareStorages(blockToExecute - 1, fixtures)
val blockchain = BlockchainImpl(storages)
val ledger = new LedgerImpl(VM, blockchain, blockchainConfig)

ledger.executeBlock(fixtures.blockByNumber(blockToExecute), validators) shouldBe noErrors
}
}

}
160 changes: 115 additions & 45 deletions src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainActor.scala
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package io.iohk.ethereum.txExecTest.util

import java.io.FileWriter
import java.net.URI

import akka.actor.{Actor, ActorRef, _}
import akka.util.ByteString
import io.iohk.ethereum.crypto.kec256
import io.iohk.ethereum.domain.{BlockHeader, Receipt}
import io.iohk.ethereum.network.Peer
import io.iohk.ethereum.network.{Peer, PeerManagerActor}
import io.iohk.ethereum.network.PeerActor.SendMessage
import io.iohk.ethereum.network.PeerManagerActor.{GetPeers, Peers}
import io.iohk.ethereum.network.p2p.messages.PV62._
Expand All @@ -24,65 +25,82 @@ import scala.collection.immutable.HashMap
import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration._
import scala.language.postfixOps

class DumpChainActor(peerManager: ActorRef, peerMessageBus: ActorRef, startBlock: BigInt, maxBlocks: BigInt) extends Actor {
import DumpChainActor._

/**
* Actor used for obtaining all the blockchain data (blocks, receipts, nodes) from the blocks [startBlock, maxBlocks]
* from a peer bootstrapNode.
* The bootstrapNode is assumed to respond to all the messages and properly, so no validation of the received data is done.
*/
class DumpChainActor(peerManager: ActorRef, peerMessageBus: ActorRef, startBlock: BigInt,
maxBlocks: BigInt, bootstrapNode: String) extends Actor {
var stateNodesHashes: Set[ByteString] = Set.empty
var contractNodesHashes: Set[ByteString] = Set.empty
var evmCodeHashes: Set[ByteString] = Set.empty

//Temporary storages used to store the received data
var blockHeadersStorage: Map[ByteString, BlockHeader] = HashMap.empty
var blockHeadersHashes: Seq[(BigInt,ByteString)] = Nil
var blockBodyStorage: Map[ByteString, BlockBody] = HashMap.empty
var blockReceiptsStorage: Map[ByteString, Seq[Receipt]] = HashMap.empty
var stateStorage: Map[ByteString, MptNode] = HashMap.empty
var contractStorage: Map[ByteString, MptNode] = HashMap.empty
var evmCodeStorage: Map[ByteString, ByteString] = HashMap.empty

//Pending data to request
var blockHeaderToRequest: BigInt = 0
var receiptsToRequest: Seq[ByteString] = Nil
var blockBodiesToRequest: Seq[ByteString] = Nil
var nodesToRequest: Seq[ByteString] = Nil

var receiptsRequested: Seq[ByteString] = Nil
var blockBodiesRequested: Seq[ByteString] = Nil

var peers: Seq[Peer] = Nil

override def preStart(): Unit = {
context.system.scheduler.scheduleOnce(4 seconds, () => peerManager ! GetPeers)
}

//Periodically try to connect to bootstrap peer in case the connection failed before dump termination
val connectToBootstrapTimeout: Cancellable = context.system.scheduler.schedule(0 seconds, 4 seconds, () =>
peerManager ! PeerManagerActor.ConnectToPeer(new URI(bootstrapNode)))

val assignWorkTimeout: Cancellable = context.system.scheduler.schedule(0 seconds, 2 seconds, () => assignWork())

// scalastyle:off
override def receive: Receive = {
case Peers(p) =>
peers = p.keys.toSeq
peers.headOption.foreach { peer =>
peerMessageBus ! Subscribe(MessageClassifier(Set(BlockHeaders.code, BlockBodies.code, Receipts.code, NodeData.code), PeerSelector.WithId(peer.id)))
peer.ref ! SendMessage(GetBlockHeaders(block = Left(startBlock), maxHeaders = maxBlocks, skip = 0, reverse = false))
}

case MessageFromPeer(m: BlockHeaders, _) =>
println(s"Received ${m.headers.size} headers")
val mptRoots: Seq[ByteString] = m.headers.map(_.stateRoot)

blockHeadersHashes = m.headers.map(e => (e.number, e.hash))
m.headers.foreach { h =>
blockHeadersStorage = blockHeadersStorage + (h.hash -> h)
}

peers.headOption.foreach { case Peer(_, actor, _) =>
actor ! SendMessage(GetBlockBodies(m.headers.map(_.hash)))
actor ! SendMessage(GetReceipts(blockHeadersHashes.filter { case (n, _) => n > 0 }.map { case (_, h) => h }))
actor ! SendMessage(GetNodeData(mptRoots))
stateNodesHashes = stateNodesHashes ++ mptRoots.toSet
}
blockBodiesToRequest = blockBodiesToRequest ++ m.headers.map(_.hash)
receiptsToRequest = receiptsToRequest ++ m.headers.map(_.hash)
nodesToRequest = nodesToRequest ++ mptRoots
stateNodesHashes = stateNodesHashes ++ mptRoots.toSet

case MessageFromPeer(m: BlockBodies, _) =>
m.bodies.zip(blockHeadersHashes).foreach { case (b, (_, h)) =>
println(s"Received ${m.bodies.size} bodies")
m.bodies.zip(blockBodiesRequested).foreach { case (b, h) =>
blockBodyStorage = blockBodyStorage + (h -> b)
}
val bodiesFile = new FileWriter("bodies.txt", true)
blockBodyStorage.foreach{case (h,v) => bodiesFile.write(s"${Hex.toHexString(h.toArray[Byte])} ${Hex.toHexString(v.toBytes)}\n")}
bodiesFile.close()
blockBodiesRequested = Nil

case MessageFromPeer(m: Receipts, _) =>
m.receiptsForBlocks.zip(blockHeadersHashes.filter { case (n, _) => n > 0 }).foreach { case (r, (_, h)) =>
println(s"Received ${m.receiptsForBlocks.size} receipts lists")
m.receiptsForBlocks.zip(receiptsRequested).foreach { case (r, h) =>
blockReceiptsStorage = blockReceiptsStorage + (h -> r)
}
val receiptsFile = new FileWriter("receipts.txt", true)
blockReceiptsStorage.foreach { case (h, v: Seq[Receipt]) => receiptsFile.write(s"${Hex.toHexString(h.toArray[Byte])} ${Hex.toHexString(v.toBytes)}\n") }
receiptsFile.close()
receiptsRequested = Nil

case MessageFromPeer(m: NodeData, _) =>

Expand All @@ -95,8 +113,8 @@ class DumpChainActor(peerManager: ActorRef, peerMessageBus: ActorRef, startBlock
val children = nodes.flatMap {
case n: BranchNode => n.children.collect { case Some(Left(h)) => h }
case ExtensionNode(_, Left(h), _, _) => Seq(h)
case n: LeafNode => Seq.empty
case _ => Seq.empty
case _: LeafNode => Seq.empty
case _ => Seq.empty
}

var contractChildren: Seq[ByteString] = Nil
Expand Down Expand Up @@ -144,37 +162,89 @@ class DumpChainActor(peerManager: ActorRef, peerMessageBus: ActorRef, startBlock
contractStorage = contractStorage + (ByteString(n.hash) -> n)
}

if (children.isEmpty && contractChildren.isEmpty && evmTorequest.isEmpty) {
val headersFile = new FileWriter("headers.txt", true)
val stateTreeFile = new FileWriter("stateTree.txt", true)
val contractTreesFile = new FileWriter("contractTrees.txt", true)
val evmCodeFile = new FileWriter("evmCode.txt", true)
nodesToRequest = nodesToRequest ++ children ++ contractChildren ++ evmTorequest

def dumpToFile(fw: FileWriter, element: (ByteString, ByteString)): Unit = element match {
case (h, v) => fw.write(s"${Hex.toHexString(h.toArray[Byte])} ${Hex.toHexString(v.toArray)}\n")
}
}

blockHeadersStorage.foreach{ case (headerHash, header) => dumpToFile(headersFile, headerHash -> header.toBytes) }
stateStorage.foreach(s => dumpToFile(stateTreeFile, s._1 -> s._2.toBytes))
contractStorage.foreach(c => dumpToFile(contractTreesFile, c._1 -> c._2.toBytes))
evmCodeStorage.foreach { case (h, v) => evmCodeFile.write(s"${Hex.toHexString(h.toArray[Byte])} ${Hex.toHexString(v.toArray[Byte])}\n") }

headersFile.close()
stateTreeFile.close()
contractTreesFile.close()
evmCodeFile.close()
println("chain dumped to file")
} else {
peers.headOption.foreach { case Peer(_, actor, _) =>
actor ! SendMessage(GetNodeData(children ++ contractChildren ++ evmTorequest))
private def assignWork(): Unit = {
if(!anyRequestsRemaining()) {
dumpChainToFile()
println("Finished download, dumped chain to file")
assignWorkTimeout.cancel()
connectToBootstrapTimeout.cancel()
context stop self
} else {
if(peers.nonEmpty) {
val peerToRequest = peers.head
//Block headers are only requested once the pending receipts and bodies requests were finished
if(blockHeaderToRequest < maxBlocks && receiptsRequested.isEmpty && blockBodiesRequested.isEmpty &&
blockBodiesToRequest.isEmpty && receiptsToRequest.isEmpty) {
val headersRemaining = maxBlocks - blockHeaderToRequest
peerToRequest.ref ! SendMessage(
GetBlockHeaders(block = Left(blockHeaderToRequest), maxHeaders = headersRemaining.max(MaxHeadersPerRequest), skip = 0, reverse = false))
blockHeaderToRequest = blockHeaderToRequest + MaxHeadersPerRequest

} else if (nodesToRequest.nonEmpty) {
val (currentNodesToRequest, remainingNodesToRequest) = nodesToRequest.splitAt(MaxNodesPerRequest)
nodesToRequest = remainingNodesToRequest
peerToRequest.ref ! SendMessage(GetNodeData(currentNodesToRequest))

} else if (blockBodiesToRequest.nonEmpty) {
val (currentBlockBodiesToRequest, remainingBodiesToRequest) = blockBodiesToRequest.splitAt(MaxBodiesPerRequest)
blockBodiesToRequest = remainingBodiesToRequest
blockBodiesRequested = currentBlockBodiesToRequest
peerToRequest.ref ! SendMessage(GetBlockBodies(currentBlockBodiesToRequest))

} else if (receiptsToRequest.nonEmpty) {
val (currentReceiptsToRequest, remainingReceiptsToRequest) = receiptsToRequest.splitAt(MaxReceiptsPerRequest)
receiptsToRequest = remainingReceiptsToRequest
receiptsRequested = currentReceiptsToRequest
peerToRequest.ref ! SendMessage(GetReceipts(currentReceiptsToRequest))
}
}
}
}

private def anyRequestsRemaining(): Boolean =
nodesToRequest.nonEmpty || blockBodiesToRequest.nonEmpty || receiptsToRequest.nonEmpty || (blockHeaderToRequest < maxBlocks)

private def dumpChainToFile(): Unit = {
val headersFile = new FileWriter("headers.txt", true)
val stateTreeFile = new FileWriter("stateTree.txt", true)
val contractTreesFile = new FileWriter("contractTrees.txt", true)
val evmCodeFile = new FileWriter("evmCode.txt", true)
val receiptsFile = new FileWriter("receipts.txt", true)
val bodiesFile = new FileWriter("bodies.txt", true)

def dumpToFile(fw: FileWriter, element: (ByteString, ByteString)): Unit = element match {
case (h, v) => fw.write(s"${Hex.toHexString(h.toArray[Byte])} ${Hex.toHexString(v.toArray)}\n")
}

blockHeadersStorage.foreach{ case (headerHash, header) => dumpToFile(headersFile, headerHash -> header.toBytes) }
stateStorage.foreach(s => dumpToFile(stateTreeFile, s._1 -> s._2.toBytes))
contractStorage.foreach(c => dumpToFile(contractTreesFile, c._1 -> c._2.toBytes))
evmCodeStorage.foreach { case (h, v) => evmCodeFile.write(s"${Hex.toHexString(h.toArray[Byte])} ${Hex.toHexString(v.toArray[Byte])}\n") }
blockReceiptsStorage.foreach { case (h, v) => receiptsFile.write(s"${Hex.toHexString(h.toArray[Byte])} ${Hex.toHexString(v.toBytes)}\n") }
blockBodyStorage.foreach{case (h,v) => bodiesFile.write(s"${Hex.toHexString(h.toArray[Byte])} ${Hex.toHexString(v.toBytes)}\n")}

headersFile.close()
stateTreeFile.close()
contractTreesFile.close()
evmCodeFile.close()
receiptsFile.close()
bodiesFile.close()
}
}

object DumpChainActor {
def props(peerManager: ActorRef, peerMessageBus: ActorRef, startBlock: BigInt, maxBlocks: BigInt): Props =
Props(new DumpChainActor(peerManager, peerMessageBus: ActorRef, startBlock: BigInt, maxBlocks: BigInt))
val MaxHeadersPerRequest = 128
val MaxBodiesPerRequest = 64
val MaxNodesPerRequest = 128
val MaxReceiptsPerRequest = 128

def props(peerManager: ActorRef, peerMessageBus: ActorRef, startBlock: BigInt,
maxBlocks: BigInt, bootstrapNode: String): Props =
Props(new DumpChainActor(peerManager, peerMessageBus: ActorRef, startBlock: BigInt, maxBlocks: BigInt, bootstrapNode))
val emptyStorage = ByteString(Hex.decode("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421"))
val emptyEvm = ByteString(Hex.decode("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"))
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ object DumpChainApp extends App with NodeKeyBuilder with SecureRandomBuilder wit
handshaker = handshaker,
authHandshaker = authHandshaker,
messageDecoder = EthereumMessageDecoder), "peer-manager")
actorSystem.actorOf(DumpChainActor.props(peerManager,peerMessageBus,startBlock,maxBlocks), "dumper")
peerManager ! PeerManagerActor.StartConnecting

actorSystem.actorOf(DumpChainActor.props(peerManager,peerMessageBus,startBlock,maxBlocks, node), "dumper")
}

class BlockchainMock(genesisHash: ByteString) extends Blockchain {
Expand Down
41 changes: 29 additions & 12 deletions src/main/scala/io/iohk/ethereum/ledger/BlockRewardCalculator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import io.iohk.ethereum.utils.MonetaryPolicyConfig

/**
* Calculates rewards for mining blocks and ommers.
* Avoids floating point arithmetic. Because of that the formulas may look a bit unintuitive, but the important
* thing here is that we want to defer any division to be a single and final operation
* To be compatible with Geth division was not defered to be a single and final operation, which produces expected
* rounding errors.
* A Geth issue (ethereumproject/go-ethereum#352) and a comment on the ECIP (https://github.com/ethereumproject/ECIPs/issues/15#issuecomment-330660976)
* were created to check whether this should be changed
*/
class BlockRewardCalculator(config: MonetaryPolicyConfig) {
/** Era duration in blocks */
Expand Down Expand Up @@ -41,12 +43,9 @@ class BlockRewardCalculator(config: MonetaryPolicyConfig) {

def calcBlockMinerReward(blockNumber: BigInt, ommersCount: Int): BigInt = {
val era = eraNumber(blockNumber)
val eraMultiplier = rewardReductionRateNumer.pow(era)
val eraDivisor = rewardReductionRateDenom.pow(era)

val baseReward = (firstEraBlockReward * eraMultiplier) / eraDivisor
val ommersReward = (firstEraBlockReward * ommersCount * ommerInclusionRewardNumer * eraMultiplier) /
(ommerInclusionRewardDenom * eraDivisor)
val baseReward = calcMinerBaseReward(era)
val ommersReward = calcMinerRewardPerOmmer(era) * ommersCount
Copy link
Contributor

Choose a reason for hiding this comment

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

Take a note of ethereumproject/go-ethereum#352 and ethereumproject/ECIPs#15 (comment). Maybe we should at least add comments with those links until the issue is resolved. Ideally we should hold on merging this until we get some feedback (hopefully soon)

Copy link
Contributor

Choose a reason for hiding this comment

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

If we go with this implementation, comments need to be updated to reflect that these are separate rewards that have already been rounded down.

baseReward + ommersReward
}

Expand All @@ -56,13 +55,31 @@ class BlockRewardCalculator(config: MonetaryPolicyConfig) {
if (era == 0) {
val numer = firstEraOmmerMiningRewardMaxNumer - (blockNumber - ommerNumber - 1)
(firstEraBlockReward * numer) / firstEraOmmerMiningRewardDenom
} else {
val eraMultiplier = rewardReductionRateNumer.pow(era)
val eraDivisor = rewardReductionRateDenom.pow(era)
(firstEraBlockReward * ommerMiningRewardNumer * eraMultiplier) / (ommerMiningRewardDenom * eraDivisor)
}
} else
calcMinerBaseReward(era) * ommerMiningRewardNumer / ommerMiningRewardDenom
Copy link
Contributor

Choose a reason for hiding this comment

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

You're multiplying the result of division, thus introducing rounding errors

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this could cause rounding error, however as this seems to be the more closer approach to how it is calculated in Geth and ommerMiningRewardNumer=1, I decided to leave it this way.
Do you think it would be better to change this?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, if geth does it then it's fine. I thought about it and it's not devoid of logic to have it rounded down prior to another division.

}

/**
* Calculates the miner base reward (without considering the ommers included)
*
* @param era to which the mined block belongs
* @return miner base reward
*/
private def calcMinerBaseReward(era: Int): BigInt = {
val eraMultiplier = rewardReductionRateNumer.pow(era)
val eraDivisor = rewardReductionRateDenom.pow(era)
firstEraBlockReward * eraMultiplier / eraDivisor
}

/**
* Calculates reward given to the miner for each ommer included in the block
*
* @param era to which the mined block belongs
* @return reward given to the miner for each ommer included
*/
private def calcMinerRewardPerOmmer(era: Int): BigInt =
calcMinerBaseReward(era) * ommerInclusionRewardNumer / ommerInclusionRewardDenom

/** era number counting from 0 */
private def eraNumber(blockNumber: BigInt): Int =
((blockNumber - 1) / eraDuration).toInt
Expand Down
Loading