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

Algo change to 192_7 from dev work done in May 2018 #202

Merged
merged 16 commits into from Feb 21, 2019

Conversation

Projects
None yet
7 participants
@mattpass
Copy link
Contributor

mattpass commented Jan 26, 2019

This PR is from work done in May 2018 before the the rebase idea was started upon. The status of it according to @michaelotis is that:

  • It was tested only a little, not a whole lot, just a little. It was producing blocks just fine but it was only ever attempted on a new chain and not as a fork.
  • It's believed that, similar to other coins, there may be some re-targeting issues that may need to be resolved

It should be noted that a few commits on the BTCPrivate/BitcoinPrivate repo have happened since and not in the repo being proposed to be merged in, so worthwhile considering especially re any potential missed areas or conflicts. The commits are from this point to master's head: 4045199...master

Proposed steps to take are therefore:

  • Check over commits in this PR
  • Fix merge conflicts
  • Check over the commits not in that PR in the commit range link above for anything potentially missing or would cause conflicts
  • Test it does indeed run perfectly fine as a new chain
  • Test it does also run perfectly as a fork
  • Consider the re-targeting issues mentioned
  • Test again thoroughly in a small testnet as a final check

Extra steps to take and comments on the above welcomed.

jc and others added some commits May 6, 2018

Changed regtest to use the Optimised equihash solver so that regtest …
…is practically usable with 192,7 parameters.
Merge pull request #1 from donshin/192_7
Changed generate rpc call to use the Optimised equihash solver
Refined testnet parameters for testing switching over to new equihash…
… n,k using CPU solvers; automatically use default solver for 200,9 and tromp solver for 192,7 as a temporary workaround.
@dskjkljdas
Copy link
Contributor

dskjkljdas left a comment

AOK

@dskjkljdas dskjkljdas referenced this pull request Feb 3, 2019

Closed

192_7 algorithm switch #209

@bigdaveakers
Copy link
Collaborator

bigdaveakers left a comment

src/miner.cpp
Lines 704 and 892 - comments suggest implementation is a temporary workaround. Can the suggestion to parameterise n & k be implemented?

@bigdaveakers
Copy link
Collaborator

bigdaveakers left a comment

src/chainparams.cpp
Lines 230 - undocumented change to allow solo mining on testnet

@bigdaveakers

This comment has been minimized.

Copy link
Collaborator

bigdaveakers commented Feb 3, 2019

Are the changes in this 'similar' to those in #181? Can #181 be closed in favour of this change?

@bigdaveakers

This comment has been minimized.

Copy link
Collaborator

bigdaveakers commented Feb 3, 2019

I am not comfortable with block 600001, I would favour earlier implementation if the desire is to enhance 'ASIC' resistance. I would suspect 7 months is plenty of time to develop a hardware solution meaning that we continually chase our tail to eliminate the threat.

@dskjkljdas

This comment has been minimized.

Copy link
Contributor

dskjkljdas commented Feb 3, 2019

I am not comfortable with block 600001, I would favour earlier implementation if the desire is to enhance 'ASIC' resistance. I would suspect 7 months is plenty of time to develop a hardware solution meaning that we continually chase our tail to eliminate the threat.

Agreed, the difficulty bomb can be reprogrammed to 1 block after the alogo change fork blockheight

@dskjkljdas

This comment has been minimized.

Copy link
Contributor

dskjkljdas commented Feb 3, 2019

Are the changes in this 'similar' to those in #181? Can #181 be closed in favour of this change?

Yeah the base code is the same, 202 has all the shared contributions 181 seems to be the initial guide

@bigdaveakers

This comment has been minimized.

Copy link
Collaborator

bigdaveakers commented Feb 3, 2019

Agreed, the difficulty bomb can be reprogrammed to 1 block after the alogo change fork blockheight

1 may be a bit controversial, but it would be effective.

@interbiznw

This comment has been minimized.

Copy link
Member

interbiznw commented Feb 7, 2019

Testing now @ https://testnet2.btcprivate.org/ If anyone would like to join in with some hashrate so we can analyze the difficulty adjustment thoroughly. TYIA

@interbiznw interbiznw changed the base branch from master to 192_7 Feb 10, 2019

@interbiznw interbiznw self-requested a review Feb 10, 2019

@interbiznw interbiznw changed the base branch from 192_7 to master Feb 10, 2019

@interbiznw interbiznw changed the base branch from master to 192_7 Feb 10, 2019

@interbiznw interbiznw requested a review from ch4ot1c Feb 10, 2019

@interbiznw
Copy link
Member

interbiznw left a comment

LGTM and still in testing on testnet @ https://testnet2.btcprivate.org/ If anyone would like to join in with some hashrate so we can analyze the difficulty adjustment thoroughly. TYIA

@richardvtbtcp
Copy link

richardvtbtcp left a comment

tested and AOK

@interbiznw interbiznw merged commit 6ca434a into BTCPrivate:192_7 Feb 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment