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

Difficulty algorithm #113

Open
zawy12 opened this issue Apr 26, 2018 · 20 comments
Open

Difficulty algorithm #113

zawy12 opened this issue Apr 26, 2018 · 20 comments

Comments

@zawy12
Copy link

zawy12 commented Apr 26, 2018

Karbo and I got Forknote to update the difficulty algorithm to LWMA, but we did not make it clear that we were not ready for an LWMA to be chosen (out of the several that are out there). The LWMA forknote has only 1 problem that I know of which is this that will be a big problem if a coin has, or starts out with low difficulty.

if (next_difficulty < 100000) { next_difficulty = 100000; }

This is in the Currency.cpp file.

There are also potential variable declaration problems that may not show up on testnet but could show up in the mainnet. One is that double everywhere instead of double_t is more sure to be the same on all systems. But the code as forknote has it is working fine on karbowanec.

I would like to coins using the code here which you should be able to copy and paste into the forknote curency.cpp file.

I am not asking forknote to re-commit and merge. There are sure to be more changes coming, such as changing it to integer math, correcting any other problems found, and making it a "Dynamic LWMA"

@pmitchev
Copy link

if (next_difficulty < 100000) { next_difficulty = 100000; }

should definitely be changed. Is 1 ok as min?
What are the changes for "Dynamic LWMA"?

@2acoin
Copy link

2acoin commented Apr 26, 2018

Monitoring closely... We have implemented the forknote LWMA code as is and running tests. If there are recommended changes then we would be interested in implementing them before our coin goes live.

@zawy12
Copy link
Author

zawy12 commented Apr 26, 2018

@2acoin If you difficulty is not going start stay above 10,000,000 then delete

if (next_difficulty < 100000) { next_difficulty = 100000; }

It's in Currency.cpp I believe.

Also, look for future_time_limit in your config params file and change it to 500 instead of 60*60*2. That a VERY important change @pmitchev I am surprised karbo and I did not make sure you had made that change. Intense lost 5000 coins in about an hour from that. Here it is:

const uint64_t CRYPTONOTE_BLOCK_FUTURE_TIME_LIMIT = 500;

Lag must be zero in both zawy v1 and LWMA

const size_t DIFFICULTY_LAG_V2 = 0;

const size_t DIFFICULTY_WINDOW_V2 = 60 + 1;

This is still set to 720 instead of 61! No one should use the code as it is until the above changes are made.

I have an equation that is needed to replace that 60 above. But I don't know if you can just throw it in there N should be a function of the target solvetime:

int(45*(600/T)^(0.2*(600/T)^0.3) + 0.5)

I would like for you to copy and paste our code into your currency.cpp file

I did this post rushed, so please send me links to commits before merging.

@pmitchev
Copy link

pmitchev commented Apr 26, 2018

The code is indeed still unsafe for coins different than Karbo.

The configuration file for Karbo is here (DIFFICULTY_LAG and DIFFICULTY_WINDOW are correct):
https://github.com/forknote/configs/blob/master/karbowanec.conf

I used this formula (found in Karbo's codebase), because I needed generalised formula for finding N (N is configurable - ZAWY_LWMA_DIFFICULTY_N in karbowanec.conf).
CRYPTONOTE_BLOCK_FUTURE_TIME_LIMIT will be added as configurable parameter too, because 500 is not suitable for all chains.

The commit with the Zawy LWMA difficulty changes:
0a686c8

@vitorgamer58
Copy link

Dashcoin, Bytecoin and Blood Donation Coin is update to Block Version V4, then the pools should have the updated cryptonote updated to be compatible.

@2acoin
Copy link

2acoin commented May 2, 2018

@zawy12 I have made some modifications to the ForkNote LWMA code as you had suggested in an earlier comment above.

Can you take a look at the changes I made in this branch of our 2ACoin code?

Our 2acoin.conf looks like the following;

BYTECOIN_NETWORK=3763960a-eac3-9bd5-0ba1-4631b30838bc
DEFAULT_DUST_THRESHOLD=1000000
GENESIS_COINBASE_TX_HEX=010a01ff000180b084bfbfdc28027e11e5559b5faaaed3ecc0fc4cbbeaf90a094e459f38d831fd3f3d02ca409a2421011caaed1e32b4415a451fcc8dcb599a0a2deb33c0c7ca9ad66555881c44d66100
MAX_BLOCK_SIZE_INITIAL=100000
MAX_TRANSACTION_SIZE_LIMIT=100000
MINIMUM_FEE=1000000
SYNC_FROM_ZERO=1
ZAWY_DIFFICULTY_LAST_BLOCK=5

seed-node=207.148.3.17:17890                          //seed01.2acoin.org
seed-node=207.148.6.195:17890                         //seed02.2acoin.org

I'm starting some testing with this code this evening.

@zawy12
Copy link
Author

zawy12 commented May 2, 2018

const size_t BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW = 60;
Change to 11

Set the following. For T=90 seconds, N=90 is about I don't the equation I previously used, and using it only in the LWMA algo may mean you do not have the right timestamp and difficulty vectors. It's +1 because

const size_t DIFFICULTY_WINDOW_V2 = 90 +1;

and in the algo use:
const size_t N = DIFFICULTY_WINDOW_V2;

The new reference version is what Masari and I worked on today:
masari-project/masari@f8b4c4d

There's still some casting I would like to simplify.

@zawy12
Copy link
Author

zawy12 commented May 3, 2018

Correction, @GABETRON pointed out this morning there is an exploit in the masari version if the newest commits by masari are not followed. The following change much be made.
Delete
solveTime = std::min<int64_t>((T * 10), std::max<int64_t>(solveTime, -FTL));

@2acoin
Copy link

2acoin commented May 4, 2018

@zawy12 Can you take a look at this branch for 2ACoin?

I have made the suggested updates... I also have LWMA setup to be the default difficulty setting for our coin by including ZAWY_DIFFICULTY_LAST_BLOCK=5 in our config file.

Our testing is progressing and the results look promising... You can see our tests via our pool - pool.2acoin.org

@zawy12
Copy link
Author

zawy12 commented May 4, 2018

If you can wait a day or two for devs to reveiw the code and test it, I have something a lot better:

https://github.com/zawy12/difficulty-algorithms/wiki/Dynamic-LWMA

@2acoin
Copy link

2acoin commented May 4, 2018

@zawy12 2ACoin can wait... We have a few weeks before we launch.

Looking forward to testing with the Dynamic-LWMA...

@vitorgamer58
Copy link

I still have some doubts and I intend to make the fork soon to reduce the N window to 70, I have this original code: https://github.com/BloodDonationCoinDev/bbrc-config/blob/master/BloodDonationCoin-Original.conf
and this is the code I intend to use after the fork, could you take a look?
https://github.com/BloodDonationCoinDev/bbrc-config/blob/master/BloodDonationCoin.conf

@zawy12
Copy link
Author

zawy12 commented Jun 13, 2018

These changes need to be made to the current code. In config file, change

const uint64_t CRYPTONOTE_BLOCK_FUTURE_TIME_LIMIT            = 60 * 60 * 2;
const size_t   BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW             = 60;

to 360 and 11.

In Currency.cpp delete the following (does not allow difficulty to go below 100,000). Kabo added this specifically for their high difficulty.

    // minimum limit
    if (next_difficulty < 100000) {
      next_difficulty = 100000;
    }

Change this line
solveTime = std::min<int64_t>((T * 7), std::max<int64_t>(solveTime, (-6 * T)));
to
solveTime = std::min<int64_t>((T * 6), std::max<int64_t>(solveTime, (-360T)));

Change all this

   const int64_t T = static_cast<int64_t>(m_difficultyTarget);
    const size_t N = m_zawyLWMADifficultyN ? m_zawyLWMADifficultyN : uint32_t(45 * std::pow(((double)600 / T), 0.3));

    if (timestamps.size() > N + 1) {
      timestamps.resize(N + 1);
      cumulativeDifficulties.resize(N + 1);
    }
    size_t n = timestamps.size();
    assert(n == cumulativeDifficulties.size());
    assert(n <= N);
    if (n <= 1)
      return 1;

To:

   const int64_t T = static_cast<int64_t>(m_difficultyTarget);
    const int64_t N = m_zawyLWMADifficultyN ? m_zawyLWMADifficultyN : uint32_t(45 * std::pow(((double)600 / T), 0.3));

      if (timestamps.size() > N + 1) {
         timestamps.resize(N + 1);
         cumulativeDifficulties.resize(N + 1);
      }
    else if (timestamps.size() < 4) {   return 100;    }
    else ( timestamps.size() < N+1 ) {  N = timestamps.size() - 1;    }

I don't know why m_difficultyTarget is used instead of DIFFICULTY_TARGET.

This:
else ( timestamps.size() < N+1 ) { N = timestamps.size() - 1; }
might need to be this:
else ( timestamps.size() < static_cast<uint64_t>(N)+1 ) { N = timestamps.size() - 1; }

@pmitchev
Copy link

solveTime = std::min<int64_t>((T * 6), std::max<int64_t>(solveTime, (-360T)));

What do you mean by this line?

@zawy12
Copy link
Author

zawy12 commented Jun 15, 2018

Typo...Change the 360T to 360.

@zawy12
Copy link
Author

zawy12 commented Jun 15, 2018

Also there is a huge benefit I'm calling LWMA-2: Add this

// implement LWMA-2 changes from LWMA
prev_D = cumulative_difficulties[N] - cumulative_difficulties[N-1];
if ( sum_3_ST < (8*T)/10) {  next_D = (prev_D*110)/100; }

right after this:
nextDifficulty = harmonic_mean_D * T / LWMA;

And put this:
if ( i > N-3 ) { sum_3_ST += ST; }
right after this:
sum_inverse_D += 1 / static_cast<double_t>(difficulty);

And declare prev_D and sum_3_ST as int64_t

@zawy12
Copy link
Author

zawy12 commented Jun 15, 2018

Here's the new LWMA-2.
zawy12/difficulty-algorithms#3

@zawy12
Copy link
Author

zawy12 commented Aug 5, 2018

What's N inside forknote's LWMA? What's the size of the timestamps and cumulative diofficulties vectors that are passed to the LWMA before this code is potentially activated?

    if (timestamps.size() > N + 1) {
      timestamps.resize(N + 1);
      cumulativeDifficulties.resize(N + 1);
    }

My concern is DIFFICULTY_WINDOW in the config file is 720. If that is the size of the vectors and N=60 like it should, then the resize above removes all the recent block data and sets difficulty as it needed to be 24 hours in in the past, making it worse than cryptonote default algorithm.

Also, I'm still getting emails from coins shocked that their fork or genesis is broke and I have to explain the following is the problem:

    if (next_difficulty < 100000) {
      next_difficulty = 100000;
    }

@inzider
Copy link

inzider commented Aug 6, 2018

Hi @zawy12, could you reference me to the latest stable version of forknote plz. Seems the one on official forknote repo with the LWMA is giving error - I can't find how to get rid of "proof of work too weak"

@zawy12
Copy link
Author

zawy12 commented Aug 6, 2018

I do not know if forknote has an LWMA that is working correctly. The problem is that I can't determine what size the timestamps and cumulative difficulties vectors are when they are sent to the LWMA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants