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

Tweaks to difficulty adjustment algorithm #1338

Merged

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Sep 5, 2016

This PR changes the difficulty algorithm to adjust from the average difficulty over the
block window instead of from the last difficulty. It also removes the special rules for the
testnet, which are incompatible with difficulty averaging.

Closes #147 again.

The min-difficulty blocks are incompatible with difficulty averaging.

Network difficulty is also now defined as the difficulty the network is
currently working to solve, rather than the last non-min-difficulty block
difficulty.
@str4d str4d added I-SECURITY Problems and improvements related to security. A-consensus Area: Consensus rules A-pow Area: Proof-of-Work and mining S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2016
@str4d str4d added this to the Beta 1 - complete RPC milestone Sep 5, 2016
@str4d
Copy link
Contributor Author

str4d commented Sep 5, 2016

Simulations of the algorithm after this tweak:

zcashd-diff-algo-beta1-1
zcashd-diff-algo-beta1-2

function getMedianTimePast(b) {
        var nMedianTimeSpan = 11;

        var times = [];
        var pindex = b;
        for (var i = 0; i < nMedianTimeSpan && pindex; i++) {
                times.push(pindex.time);
                pindex = pindex._prev();
        }

        times.sort();
        return times[Math.floor(times.length/2)];
}
btc.Blockchain.Block.prototype.difficulty_adjustment_period = 1;
btc.Blockchain.Block.prototype.difficultyAdjustment = function() {
        var nAveragingInterval = 17;
        var nAveragingTargetTimespan = nAveragingInterval * this.target_avg_between_blocks;

        var nMaxAdjustDownV3 = 16; // 16% adjustment down
        var nMaxAdjustUpV3 = 8; // 8% adjustment up

        var nMinActualTimespanV3 = nAveragingTargetTimespan * (100 - nMaxAdjustUpV3) / 100;
        var nMaxActualTimespanV3 = nAveragingTargetTimespan * (100 + nMaxAdjustDownV3) / 100;

        var avgDiff;
        var first = this;
        for (var i=0; first && i<nAveragingInterval; i++) {
                if (i == 0) {
                        avgDiff = first.difficulty;
                } else {
                        avgDiff = ((avgDiff * i) + first.difficulty) / (i + 1);
                }
                first = first._prev();
        }
        if (!first) {
                this.difficulty = this.proof_of_work_limit; // not enough blocks available
                console.log("(h=" + this.h + ") difficulty at minimum");
                return;
        }

        // Limit adjustment step
        // Use medians to prevent time-warp attacks
        var nActualTimespan = getMedianTimePast(this) - getMedianTimePast(first);
        nActualTimespan = nAveragingTargetTimespan + (nActualTimespan - nAveragingTargetTimespan)/4
        if (nActualTimespan < nMinActualTimespanV3) {
                nActualTimespan = nMinActualTimespanV3
        }
        if (nActualTimespan > nMaxActualTimespanV3) {
                nActualTimespan = nMaxActualTimespanV3
        }

        // Global retarget
        this.difficulty = avgDiff * nAveragingTargetTimespan / nActualTimespan;

        if (this.difficulty < this.proof_of_work_limit) {
                this.difficulty = this.proof_of_work_limit;
        }

        console.log("(h=" + this.h + ") difficulty adjustment " + (nAveragingTargetTimespan / nActualTimespan) + "x")
};

@str4d
Copy link
Contributor Author

str4d commented Sep 5, 2016

We may still want to make the algorithm more aggressive by reducing the dampening and/or limits, because it still appears to take tens of blocks (up to 100 in the simulation) to adjust to a mining pool turning on or off. But on a 100-block average, the simulation appears to be pretty stable.

I'd be happy with running with this for Beta 1, and examining the resulting behaviour.

@@ -10,33 +10,6 @@
#include "streams.h"
#include "utilstrencodings.h"

TEST(rpc, GetDifficultyTestnetRules) {
SelectParams(CBaseChainParams::TESTNET);
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than just deleting this test, can we run the test for the mainnet parameters also with the testnet parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test doesn't make sense without the min-difficulty rules. If there's another aspect you want to test, I can add that (my) tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, is there no mainnet test corresponding to this? I just meant have a test that would fail without this commit.

@daira
Copy link
Contributor

daira commented Sep 5, 2016

Doesn't this change significantly slow the effective adjustment rate? Suppose for the sake of argument you start with a constant difficulty over the averaging period. Then you see something that would cause a maximum adjustment upward under the old algorithm. This algorithm will perform that adjustment for the first block, but then the next adjustment will be relative to almost the same average (and is constrained by the same maximum adjustment relative to that average), so it's not possible to achieve the maximum adjustment for more than one block in a row.

@daira
Copy link
Contributor

daira commented Sep 5, 2016

Ignore that comment; I had misunderstood what was being averaged.

@daira
Copy link
Contributor

daira commented Sep 5, 2016

Oh, there aren't any tests of the difficulty adjustment at all. Eek.

Yes there are tests: str4d@284e125 . Why aren't they failing with this change?

@str4d
Copy link
Contributor Author

str4d commented Sep 5, 2016

@daira those tests check the boundaries and time-based adjustment, which is unchanged in this PR; all that changes is that in those tests, nBits is now interpreted as an average difficulty. I'll update the test comments to indicate this. I still need to add tests to check the difficulty averaging portion.

@str4d
Copy link
Contributor Author

str4d commented Sep 5, 2016

Also note that by increasing the bounds for beta 1 as suggested by @daira in today's engineering meeting, those existing tests will change.

@str4d
Copy link
Contributor Author

str4d commented Sep 5, 2016

In our pairing, @daira and I simulated doubling the bounds to make the algorithm slightly more aggressive:

zcashd-diff-algo-beta1-aggr-1
zcashd-diff-algo-beta1-aggr-2

We're pretty happy with the stability of the result, and will be going with this for beta 1 (and very likely for 1.0 unless the beta 1 blockchain shows any severe issues).

Additionally, I will be adding a restriction in this PR that powLimit must be smaller than 2^256 / 17, since otherwise we can get overflows in the difficulty averaging code. This does mean that the next genesis block we generate (for beta 1) will take significantly longer to find, but I don't think there are any other changes for beta 1 that will require a genesis block change, so we can just do it in this PR.

@zawy12
Copy link

zawy12 commented Sep 6, 2016

What happens if someone applies 10x hash power for 10 minutes? They get 15 blocks and everyone else gets 9 blocks in 50 minutes? By 1.08^30 = 10x, plus 2 to get it started and 4 more by picking a bottom, it seems they could go up to 36 blocks in 30 minutes, outside it's averaging window so that block issuance is ahead of schedule. For this reason It would be better to remove the limits. The limits were to prevent self-made oscillations which are not present if Sum of difficulties is over the same period as sum of block times (ActualTimespan). The number of steal-able blocks is why I suggested 8 instead of 17. Removing the limits also prevents the block release from being able to get ahead of time. N=8 will not look pretty on difficulty and time variation, but it stops cheating better.

Concerning Time warp and medians, can there be a protocol requirement that block timestamps are sequential by requiring ActualTimespan to be the sum of the N block timestamps?

On my algo, it appears [edit: no, str4d corrected me on this] you were multiplying by the sum of previous N block difficulties instead of N=2, so it was 10x to 20x too high. I would get my algo running, then compare the results of a 10x attack and other quickly-varying hash powers. Although I can't quite understand the above median methodology. I was just summing difference between each block timestamps, not even thinking about subtracting first from last.

It appears someone's been turning on a 5x or 10x system for 1 block since block 1500 every time a solve time exceeded 6 minutes in order to "attack" a difficulty ="1" condition. No block since then has taken over 330 seconds when 20% should be. If they want, they can get 30% of all coins at a difficulty 5x less than their hashrate and probably no algorithm can stop it. I see no theoretical way to dis-incentivize it without slowing down block issuance.

@str4d
Copy link
Contributor Author

str4d commented Sep 6, 2016

@zawy12

It appears someone's been turning on a 5x or 10x system for 1 block since block 1500 every time a solve time exceeded 6 minutes in order to "attack" a difficulty ="1" condition. No block since then has taken over 330 seconds when 20% should be.

IMHO that was entirely caused by the special testnet-only difficulty rules, which allowed difficulty=1 if a block hadn't been found in over 5 minutes / 300s. The latest the testnet would adjust to that is 330s because if miners have only just started another solver run slightly before the 5 minutes is up, their next solution won't appear until 30s later at the earliest (for the current Equihash impl).

These rules are removed in this PR as they are incompatible with difficulty averaging, so all that behaviour should disappear in beta 1.

@zawy12
Copy link

zawy12 commented Sep 6, 2016

This is an idea to protect against the problems I mentioned above. This assumes the limits have been removed. In words: If last 6 blocks were faster than 98% of the Poisson expectations (6/0.02 = once per 300 blocks), the averaging window switches to 6 (which is effectively saying difficulty immediately rises ~3x). If this has occurred, do not switch back to N=17 unless there is 80% chance the higher difficulty is getting lower. This means it begins to come down quickly if it was an accident, but regrettably can't come down immediately because of a possible "on/off" attack. This also keeps block issuance on time.

For pools joining and increasing hash rate 30%, this is unlikely to get activated, so it should give the same charts as above, but cause about 4 spikes over those 2 days that resolve fairly quickly.

If the limits are removed, and the above code basically says:
difficulty = avgDiff[17] * TargetTimespan[17] / nActualTimespan[17]

Then use:

if ( ActualTimespan[6] / TargetTimespan[6] < 0.33 ) {   N=6;  attack=yes;   }
if ( attack == 'yes' && ActualTimespan[6] / TargetTimespan[6] > 1.5 ) { attack=no }
if (attack == 'yes' )  { N=6 } else { N=17 }
difficulty = avgDiff[N] * TargetTimespan[N] / nActualTimespan[N]

If median of 6 is required and problematic, use 7.

@str4d
Copy link
Contributor Author

str4d commented Sep 7, 2016

@ebfull this should be ready now 😄

@ebfull
Copy link
Contributor

ebfull commented Sep 7, 2016

ACK but I don't think (the old) CalculateNextWorkRequired should be left for "testing purposes", it should either be moved into the tests directly or removed.

GetNextWorkRequired(&blocks[lastBlk], nullptr, params));
// Result should be unchanged
// TODO: This should be 0x1e7fffff, and just before GetNextWorkRequired()
// returns, it is. Somehow it ends up off by one....
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait what? Please try to figure out why this is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already did try, and my findings are what I documented in the comment. Just before GetNextWorkRequired returns the result, the result is correct. After it returned the result, the macro sees it is off-by-one. The only think I didn't check is what the result from GetNextWorkRequired looks like before entering the macro; logically they should be identical.

Previously, if the two random numbers happened to be equal, the block time
would not be updated, and subsequent checks would fail.
@str4d
Copy link
Contributor Author

str4d commented Sep 8, 2016

@ebfull the blocking comment by @daira has been addressed.

@ebfull
Copy link
Contributor

ebfull commented Sep 8, 2016

ACK

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Sep 8, 2016

📌 Commit 622ced8 has been approved by ebfull

zkbot pushed a commit that referenced this pull request Sep 8, 2016
…, r=ebfull

Tweaks to difficulty adjustment algorithm

This PR changes the difficulty algorithm to adjust from the average difficulty over the
block window instead of from the last difficulty. It also removes the special rules for the
testnet, which are incompatible with difficulty averaging.

Closes #147 again.
@zkbot
Copy link
Contributor

zkbot commented Sep 8, 2016

⌛ Testing commit 622ced8 with merge 2271d3f...

@zkbot
Copy link
Contributor

zkbot commented Sep 8, 2016

☀️ Test successful - zcash

@zkbot zkbot merged commit 622ced8 into zcash:zc.v0.11.2.latest Sep 8, 2016
@daira
Copy link
Contributor

daira commented Sep 8, 2016

Post-hoc ACK on changes since my review.

@zawy12
Copy link

zawy12 commented Sep 9, 2016

I implemented a Poisson generator in a spreadsheet and my above code did not work better. Any complex variations I tried did not work better than a simple average.

The charts below show what happens when the 8% up and 16% down limits are removed. Blue areas are where solve times will be too long, where turning on an EC2 server will be more profitable, or how attacks will get their profit. Every red-only area is where constant-on miners will have to pay the price. For sinusoids less than 20 blocks, there is not a clear benefit to removing the limits. The step functions are attacks or a pool joining or leaving, it shows 4 tests each of 8x hash rate increasefor 8 blocks on then 8 blocks off, and then 8x for 4 blocks on 4 blocks off, followed by a 6x pool joining and leaving. Removing the limits shines on medium-term changes (50 to 150 block sinusoids), and on longer terms they are the same. Large sinusoids on short time scales < 30 minutes result in more solve time variability (aka delays) when the limits are removed. Twice the std dev under specific conditions was the worst I saw if the limits are removed.

Tech Details: The blue is the network hash rate scaled to show what the ideal difficulty should be at each block. The 1st chart is with the 8% up and 16% down limits. The 2nd is without. Rolling average is 17 blocks. My Poisson process is the time-to-solution -150*ln(rand()). Solve times recorded are this Poisson result times (Difficulty chosen) / (ideal Difficulty). The network hash rate sinusoid shown is H.R. = A * (1+0.5 * sin(2 pi * N / F)) where N is a number of 2.5 minutes that have passed, F is intervals per cycle and A is amplitude of whatever the average difficulty is (D=100 to D=1^10 is all the same). The cycle is 4 hours, 96 clocks. It is not a smooth sinusoid because x axis is block number instead of time.

These are based on average past times-to-solve, not median because when I put in median my average solve time was 30% too high, so the Zcash code above is doing a different and better median than the median of block solve time.

screenshot from 2016-09-09 06-55-51

The improved protection against hash rate variation comes at a cost under certain circumstances: the std dev of time-to-solve when there was 50% hash rate variation in cycles less than 1 hour was about 50%. The ideal attack will result in maximum solve time variation. The best protection such as a shorter averaging window will result in the highest solve time std dev when the network is stable. In general, any good algorithm will follow the rule:
(% stolen blocks) * (solve time std dev) = constant
Designers need to decide how important protecting constant-on miners is compared to stable solve times. If frequent 30 minute solves on a 2.5 minute coin are acceptable, a great level of miner protection is available.

The only way I have found to do perfect protection is to never let the difficulty drop but follow a slow steady rise, have a valid timestamp, and pay miners inversely proportional (Tim Olson's idea) to their solve time relative to the average time. This has very important stable, real value implications. For example, miners are paid PER BLOCK for the amount of electricity needed, getting closer to the ideal of value=joules, not merely based on the average electricity expense expected.

@zawy12
Copy link

zawy12 commented Sep 15, 2016

I'm concerned a mining pool or someone else with 2x network hash rate is going to get 30% or more of the blocks without raising the difficulty by using a time warp technique as someone did on z9 as I described in #998 . There are several things that may alleviate this.

  1. Timestamps from trusted peers? (ETH uses external clock somehow)
  2. BTC uses max 2 hours time between blocks, so ZEC should probably be using 30 minutes just to have equal protection with 2.5 min intervals.
  3. No limit on the rise and fall would reduce the profit some.
  4. 9 block averaging window would make it more difficult, but cause more variation in solve times.

The only cure seems to be an "oracle" clock of some sort from peers or whatever ETH is doing.

  1. Simply not letting a timestamp go negative might help: if someone adds 30 minutes (combining with # 2 above) then subsequent blocks trying to go negative would just have 1 minute added to it. They would see difficulty lower for only 8 blocks instead of 17. The 30 minute limit means they have to have more hash power to get it done quickly in order for it to make sense.

The initial "attack" (not a time warp) indicated a hash rate equal to 500 CPUs. If they or others can double that, a time warp attack should be expected every day until mainnet hash rate is 20x beta.

@str4d str4d deleted the 147-tweak-difficulty-adjustment-algorithm branch November 11, 2016 00:57
zkbot added a commit that referenced this pull request Oct 5, 2018
…cartel

Allow minimum-difficulty blocks on testnet

This is a consensus rule change on testnet that will result in a chain split (leaving the stuck chain, as desired).

Reverts #2766 and part of #1338.
Closes #3552.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rules A-pow Area: Proof-of-Work and mining I-SECURITY Problems and improvements related to security. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants