Set max epoch number constant #212
Conversation
| @@ -49,6 +49,9 @@ extern "C" { | |||
| #define ETHASH_FULL_DATASET_ITEM_SIZE 128 | |||
| #define ETHASH_NUM_DATASET_ACCESSES 64 | |||
|
|
|||
| /// The maximum epoch number supported by this implementation. | |||
| #define ETHASH_MAX_EPOCH_NUMBER 32639 | |||
There was a problem hiding this comment.
How did you arrive at this number?
There was a problem hiding this comment.
The max value where light cache size is under 4GB. Later some uint32 overflows start to happen.
There was a problem hiding this comment.
Can you document this?
Also can you put an estimate in terms of mainnet block numbers (I know it depends on difficulty, but assume a linear yearly difficulty increase). I also understand this is moot with PoS, but still would be nice to have an idea.
Or at least document the current block + epoch number, as that gives an idea how many epochs we progressed over 6 years.
There was a problem hiding this comment.
@AndreaLanfranchi @chfast Thanks for the explanation, this looks good now.
Codecov Report
@@ Coverage Diff @@
## master #212 +/- ##
==========================================
+ Coverage 98.63% 98.65% +0.01%
==========================================
Files 17 17
Lines 1323 1340 +17
==========================================
+ Hits 1305 1322 +17
Misses 18 18
Flags with carried forward coverage won't be shown. Click here to find out more.
|
lib/ethash/ethash.cpp
Outdated
| if (epoch_number > max_epoch_number) | ||
| return 0; | ||
|
|
There was a problem hiding this comment.
I think this might cause a cascading problem in create_epoch_context around here
const size_t alloc_size = context_alloc_size + light_cache_size + full_dataset_size;
char* const alloc_data = static_cast<char*>(std::calloc(1, alloc_size));
if (!alloc_data)
return nullptr; // Signal out-of-memory by returning null pointer.
hash512* const light_cache = reinterpret_cast<hash512*>(alloc_data + context_alloc_size);
If beyond max epoch then alloc_size == 512+0+0 which means
hash512* const light_cache = reinterpret_cast<hash512*>(alloc_data + context_alloc_size); reinterpret a location beyond previous allocation
There was a problem hiding this comment.
Probably is worth to make this function to return nullptr in case epoch requested is above limit. This way higher level code are somehow notified
There was a problem hiding this comment.
The create_epoch_context() has the same check and returns nullptr. Now I also added a check for epoch_number < 0.
The calculate_X_num_items() have the check duplicated because they are public functions.
There was a problem hiding this comment.
Right sorry. Was comparing master branch while reading PR branch. My bad
chfast
force-pushed
the
max_epoch_number
branch
from
7731aa3
to
bb18916
Compare
include/ethash/ethash.h
Outdated
| /// The DAG size in the last epoch is 252GB and the block number is above 979M | ||
| /// what gives over 60 years of blocks assuming 2s block times. |
There was a problem hiding this comment.
| /// The DAG size in the last epoch is 252GB and the block number is above 979M | |
| /// what gives over 60 years of blocks assuming 2s block times. | |
| /// The DAG size in the last epoch is 252GB and the block number is above 979M, | |
| /// which gives over 60 years of blocks assuming 2s block times. |
test/unittests/test_ethash.cpp
Outdated
| @@ -268,19 +273,30 @@ TEST(ethash, find_epoch_number_double_descending) | |||
| TEST(ethash, find_epoch_number_sequential) | |||
| { | |||
| hash256 seed = {}; | |||
| for (int i = 0; i < 30000; ++i) | |||
| for (int i = 0; i < max_epoch_number; ++i) | |||
test/unittests/test_ethash.cpp
Outdated
| TEST(ethash, find_epoch_number_sequential_gap) | ||
| { | ||
| constexpr int start_epoch = 200; | ||
| hash256 seed = calculate_epoch_seed(start_epoch); | ||
| for (int i = start_epoch; i < 30000; ++i) | ||
| for (int i = start_epoch; i < max_epoch_number; ++i) |
test/unittests/test_ethash.cpp
Outdated
| @@ -324,7 +340,7 @@ TEST(ethash_multithreaded, find_epoch_number_sequential) | |||
| { | |||
| auto fn = [] { | |||
| hash256 seed = {}; | |||
| for (int i = 0; i < 30000; ++i) | |||
| for (int i = 0; i < max_epoch_number; ++i) | |||
Define the maximum value of epoch number this Ethash library supports.
chfast
force-pushed
the
max_epoch_number
branch
from
bb18916
to
acd8e96
Compare
Define the maximum value of epoch number this Ethash library supports.