search

dtr-org / unit-e

A digital currency for a new era of decentralized trust
https://unit-e.io
MIT License
45 stars 15 forks source link

Adjust difficulty & drift #908

Closed AM5800 closed 5 years ago

AM5800 commented 5 years ago

All credit for the function goes to @Ruteri. I am merely prettifying/adding tests/merging.

Fixes #285

AM5800 commented 5 years ago

Ready for reviews!

AM5800 commented 5 years ago

@thothd @Gnappuraz @cmihai I have implemented sliding window. WDYT?

thothd commented 5 years ago

Looks good in general, since it's a well isolated function - let's see how we can get and ACK by running it on the Testnet

thothd commented 5 years ago

@AM5800 I agree with @Gnappuraz & @Ruteri concerns about the time related topic. The window function looks good but our default MAX_FUTURE_BLOCK_TIME (2 hours) is way too high. In an era of NTP services its reasonable to expect way better time sync especially if you be a proposer. On the Testnet we rather use a smaller drift as possible (e.g 15 seconds) to bootstrap the network, it also makes sense since new nodes joining will have to adjust. I've changed the PR title since the changes are quite coupled and we should have them together.

AM5800 commented 5 years ago

So, I made several changes: 1) We now return max_difficulty_value if something goes wrong. So I removed asserts 2) Adjusted max_future_block_time_seconds to 15 seconds, like in Particl

AM5800 commented 5 years ago

I had to set max_future_block_time_seconds to 20 for the regtest. The reason is that in some tests we generate lots of blocks. This happens in an instant, but to satisfy validation their block times should grow. And this leads to blocks too far in the future. Also running functional tests. More fixes might be required

castarco commented 5 years ago

I had to set max_future_block_time_seconds to 20 for the regtest. The reason is that in some tests we generate lots of blocks. This happens in an instant, but to satisfy validation their block times should grow. And this leads to blocks too far in the future. Also running functional tests. More fixes might be required

Yeah, in regtest we should not be worried about having very large values for that parameter.

AM5800 commented 5 years ago

My intent regarding regtest was to keep the value close to the same value in the testnet. So we would not miss any issues that arise. But it seems impossible because of the aforementioned issue. We generate lots of blocks and they keep having time in the future. Moreover, it can be a source of flakiness, if generation is slow (because block times will be even bigger in that case). So I rolled back the value to 2 hours (regtest only!)

Should be green now...

scravy commented 5 years ago

My intent regarding regtest was to keep the value close to the same value in the testnet. So we would not miss any issues that arise. But it seems impossible because of the aforementioned issue. We generate lots of blocks and they keep having time in the future. Moreover, it can be a source of flakiness, if generation is slow (because block times will be even bigger in that case). So I rolled back the value to 2 hours (regtest only!)

I think if you want to test the difficulty function in regtest using a functional test you should test it specifically. You can do that by injecting chainparameters. See rpc_getchainparams func test.

AM5800 commented 5 years ago

@scravy thanks, I think that existing tests are enough. Just a bit sad that values in regtest and testnet differ so wildly

thothd commented 5 years ago

@scravy thanks, I think that existing tests are enough. Just a bit sad that values in regtest and testnet differ so wildly

I don't think it's an issue actually but more essential, if they would be the same it'll make the regtest less usable. it's really about the purpose of the network, the regtest is actually not just for automatic tests but a real network that you wanna use, to bootstrap an ad-hoc network. there you wouldn't wanna have the same restrictions on something like the difficulty as with the testnet. also you can always change the regtest value if you want