Skip to content

Bug/etcm 797 #974

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Apr 26, 2021
Merged

Bug/etcm 797 #974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6acde52
fix bug seed calculation
bsuieric Apr 26, 2021
d91a129
[ETCM-797] add testcase for ecip 1099 seed bug
Apr 26, 2021
48b39b5
[ETCM-797] add debug endpoint to request a specific block (by number)…
Apr 26, 2021
aef19cc
refactor EthashUtils.seed and add a comment
Apr 26, 2021
26d03a0
Revert "[ETCM-797] add debug endpoint to request a specific block (by…
Apr 26, 2021
1cc751b
remove ecip1099ActivationBlock parameter from EthashDAGManager
Apr 26, 2021
f40aa21
Merge remote-tracking branch 'origin/develop' into bug/ETCM-797
Apr 26, 2021
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
2 changes: 1 addition & 1 deletion src/main/scala/io/iohk/ethereum/consensus/pow/EthashDAGManager.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class EthashDAGManager(blockCreator: EthashBlockCreator) extends Logger {
(currentEpoch, currentEpochDag, currentEpochDagSize) match {
case (Some(`epoch`), Some(dag), Some(dagSize)) => (dag, dagSize)
case _ =>
val seed = EthashUtils.seed(blockNumber)
val seed = EthashUtils.seed(blockNumber, blockCreator.blockchainConfig.ecip1099BlockNumber.toLong)
val dagSize = EthashUtils.dagSize(epoch)
val dagNumHashes = (dagSize / EthashUtils.HASH_BYTES).toInt
val dag =
Expand Down
11 changes: 8 additions & 3 deletions src/main/scala/io/iohk/ethereum/consensus/pow/EthashUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,15 @@ object EthashUtils {

// scalastyle:on magic.number

private def epochBeforeEcip1099(blockNumber: Long): Long = blockNumber / EPOCH_LENGTH_BEFORE_ECIP_1099
// computes seed for epoch of given blockNumber
// this also involves the non-ECIP1099 epoch of the first blocks of the
// ECIP1099 epoch, to make sure every block in the latter results in the same
// seed being calculated, would there be a cache miss.
def seed(blockNumber: Long, ecip1099ActivationBlock: Long): ByteString = {
val epochLength = calcEpochLength(blockNumber, ecip1099ActivationBlock)
val startBlock = (blockNumber / epochLength) * epochLength + 1
val epoch = startBlock / EPOCH_LENGTH_BEFORE_ECIP_1099

def seed(blockNumber: Long): ByteString = {
val epoch = epochBeforeEcip1099(blockNumber)
(BigInt(0) until epoch)
.foldLeft(ByteString(Hex.decode("00" * 32))) { case (b, _) => kec256(b) }
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/io/iohk/ethereum/consensus/pow/validators/EthashBlockHeaderValidator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class EthashBlockHeaderValidator(blockchainConfig: BlockchainConfig) {
}

val epoch = EthashUtils.epoch(blockHeader.number.toLong, blockchainConfig.ecip1099BlockNumber.toLong)
val seed = EthashUtils.seed(blockHeader.number.toLong)
val seed = EthashUtils.seed(blockHeader.number.toLong, blockchainConfig.ecip1099BlockNumber.toLong)
val powCacheData = getPowCacheData(epoch, seed)

val proofOfWork = hashimotoLight(
Expand Down
4 changes: 3 additions & 1 deletion src/main/scala/io/iohk/ethereum/jsonrpc/EthMiningService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import scala.collection.concurrent.{TrieMap, Map => ConcurrentMap}
import scala.concurrent.duration.FiniteDuration
import scala.language.existentials
import io.iohk.ethereum.transactions.TransactionPicker
import io.iohk.ethereum.utils.BlockchainConfig

object EthMiningService {

Expand All @@ -45,6 +46,7 @@ object EthMiningService {

class EthMiningService(
blockchain: Blockchain,
blockchainConfig: BlockchainConfig,
ledger: Ledger,
jsonRpcConfig: JsonRpcConfig,
ommersPool: ActorRef,
Expand Down Expand Up @@ -92,7 +94,7 @@ class EthMiningService(
Right(
GetWorkResponse(
powHeaderHash = ByteString(kec256(BlockHeader.getEncodedWithoutNonce(pb.block.header))),
dagSeed = EthashUtils.seed(pb.block.header.number.toLong),
dagSeed = EthashUtils.seed(pb.block.header.number.toLong, blockchainConfig.ecip1099BlockNumber.toLong),
target = ByteString((BigInt(2).pow(256) / pb.block.header.difficulty).toByteArray)
)
)
Expand Down
2 changes: 2 additions & 0 deletions src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ trait EthInfoServiceBuilder {

trait EthMiningServiceBuilder {
self: BlockchainBuilder
with BlockchainConfigBuilder
with LedgerBuilder
with JSONRpcConfigBuilder
with OmmersPoolBuilder
Expand All @@ -414,6 +415,7 @@ trait EthMiningServiceBuilder {

lazy val ethMiningService = new EthMiningService(
blockchain,
blockchainConfig,
ledger,
jsonRpcConfig,
ommersPool,
Expand Down
36 changes: 19 additions & 17 deletions src/test/scala/io/iohk/ethereum/consensus/pow/EthashUtilsSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import org.scalatest.matchers.should.Matchers

import scala.annotation.tailrec
import io.iohk.ethereum.SuperSlow
import io.iohk.ethereum.utils.ByteStringUtils
import org.scalatest.prop.TableFor2

class EthashUtilsSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyChecks with SuperSlow {

Expand All @@ -18,8 +20,21 @@ class EthashUtilsSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyC
val ecip1099forkBlockNumber: Long = 11460000

"Ethash" should "generate correct hash" in {
forAll(Gen.choose[Long](0, 15000000L)) { blockNumber =>
seed(blockNumber) shouldBe seedForBlockReference(blockNumber)
val seedEpoch0 = ByteStringUtils.string2hash("0000000000000000000000000000000000000000000000000000000000000000")
val seedEpoch1 = ByteStringUtils.string2hash("290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563")
val seedEpoch382 = ByteStringUtils.string2hash("d3d0aa11197dcdcfcb3ad3c73d415af47299bddb47fda6081d31d9dd06462f6a")
val seedEpoch383 = ByteStringUtils.string2hash("bf532874eb434842e7a3e4acd113fe454541651872760d9b95d11d7f90ca25dc")
Copy link
Contributor

@dzajkowski dzajkowski Apr 26, 2021

Choose a reason for hiding this comment

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

where did these hashes come from? maybe this would be a good inspiration https://github.com/iquidus/ecip-1099-data ?

Copy link
Contributor

@jvdp jvdp Apr 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

These are the seeds that correspond to the blocks, calculated with the Mantis code found to be working properly (valid blocks are valid.)

But the strings themselves are not so interesting for the purposes of this bug, just that they are the same across different blocks within a (ECIP-1099) epoch. (For the implementation itself the hashes are important of course.)

Copy link
Contributor

@jvdp jvdp Apr 26, 2021

Choose a reason for hiding this comment

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

(Also, the hash being all zeroes for epoch 0 is a good sign?)

val table: TableFor2[Long, ByteString] = Table(
("blockNumber", "referenceSeed"),
(0, seedEpoch0),
(1, seedEpoch0),
(30_000, seedEpoch1),
(ecip1099forkBlockNumber, seedEpoch382),
(ecip1099forkBlockNumber + 30_000, seedEpoch382),
(ecip1099forkBlockNumber + 60_000, seedEpoch383)
)
forAll(table) { (blockNumber, referenceSeed) =>
seed(blockNumber, ecip1099forkBlockNumber) shouldBe referenceSeed
}
}

Expand Down Expand Up @@ -55,7 +70,7 @@ class EthashUtilsSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyC

val blockNumber = 486382
val _epoch = epoch(blockNumber, ecip1099forkBlockNumber)
val _seed = seed(blockNumber)
val _seed = seed(blockNumber, ecip1099forkBlockNumber)
val cache = makeCache(_epoch, _seed)
val proofOfWork = hashimotoLight(hash, nonce, dagSize(_epoch), cache)

Expand Down Expand Up @@ -129,25 +144,12 @@ class EthashUtilsSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyC

forAll(table) { (blockNumber, hashWithoutNonce, nonce, mixHash) =>
val _epoch = epoch(blockNumber, ecip1099forkBlockNumber)
val _seed = seed(blockNumber)
val _seed = seed(blockNumber, ecip1099forkBlockNumber)
val cache = makeCache(_epoch, _seed)
val proofOfWork =
hashimotoLight(Hex.decode(hashWithoutNonce), Hex.decode(nonce), dagSize(_epoch), cache)
proofOfWork.mixHash shouldBe ByteString(Hex.decode(mixHash))
}
}
}

def seedForBlockReference(blockNumber: BigInt): ByteString = {
@tailrec
def go(current: BigInt, currentHash: ByteString): ByteString = {
if (current < EPOCH_LENGTH_BEFORE_ECIP_1099) {
currentHash
} else {
go(current - EPOCH_LENGTH_BEFORE_ECIP_1099, kec256(currentHash))
}
}

go(blockNumber, ByteString(Hex.decode("00" * 32)))
}
}
1 change: 1 addition & 0 deletions src/test/scala/io/iohk/ethereum/jsonrpc/EthMiningServiceSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ class EthMiningServiceSpec

lazy val ethMiningService = new EthMiningService(
blockchain,
blockchainConfig,
ledger,
jsonRpcConfig,
ommersPool.ref,
Expand Down
1 change: 1 addition & 0 deletions src/test/scala/io/iohk/ethereum/jsonrpc/JsonRpcControllerFixture.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class JsonRpcControllerFixture(implicit system: ActorSystem)

val ethMiningService = new EthMiningService(
blockchain,
blockchainConfig,
ledger,
config,
ommersPool.ref,
Expand Down