[Fix] Fix handling of empty recipts int fast sync #702
Conversation
a071ad4 to
aa71fbe
Compare
| syncState = syncState.enqueueReceipts(remainingReceipts) | ||
| } | ||
| if (receipts.isEmpty) { | ||
| val reason = s"got empty receipts for known hashes: ${requestedHashes.map(h => Hex.toHexString(h.toArray[Byte]))}" |
There was a problem hiding this comment.
Very minor: we have helper method ByteStringUtils.hash2String
| sendReceipts(newBlocks.map(_.hash), Seq(), peer1) | ||
|
|
||
| // Peer will be blacklisted for empty response, so wait he is blacklisted | ||
| Thread.sleep(6.second.toMillis) |
There was a problem hiding this comment.
Why 6 seconds? Maybe we could tweak some test params? This test will take a lot of time
There was a problem hiding this comment.
I reduced sleeps in case of blacklist (and blacklist config).
I agree @kapke that mock clock would be perfect, but at current state sync state is almost black box and there is so much timing/scheduling happening inside that those whole testsuit would require little refactor and i would prefer to create separate task to do it
There was a problem hiding this comment.
Good idea. Maybe, for now, we could tag time-consuming tests like that and skip it by default for development purposes (and run on CI only). Now the whole test suite takes more than 30m. WDYT?
There was a problem hiding this comment.
I created task for both of this possible improvements - https://jira.iohk.io/browse/ETCM-147.
But on my machine whole testsuite takes 6min so it is not that much, I am not sure why tests on CI takes that long.
| @@ -2,18 +2,20 @@ package io.iohk.ethereum.consensus.ethash | |||
|
|
|||
| import akka.util.ByteString | |||
| import io.iohk.ethereum.crypto.kec256 | |||
| import org.scalacheck.Arbitrary | |||
| import org.scalacheck.{Gen} | |||
There was a problem hiding this comment.
Very minor: strange format of import
Description
Two fixes:
reducethrown exception -new UnsupportedOperationException("empty.reduceLeft")which killed sync. In general it means that our error hanlding in actor is less than stellar, but for now just handle empty case.EthashUtilsSpec, try to fix it by rewriting recursive function in tail recursive manner and by changing number generation logic