Skip to content
This repository has been archived by the owner on Oct 20, 2023. It is now read-only.

Add support for Conceal #1514

Merged
merged 7 commits into from
Jan 31, 2023
Merged

Add support for Conceal #1514

merged 7 commits into from
Jan 31, 2023

Conversation

blackmennewstyle
Copy link

Add support for Conceal
Fix typo with Cryptonight being mispelled Crytonight

Fix typo with `Cryptonight` being mispelled `Crytonight`
@oliverw
Copy link
Owner

oliverw commented Nov 30, 2022

Thanks for the PR. Checking.

Copy link
Contributor

@minershaven minershaven left a comment

Choose a reason for hiding this comment

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

Nice work. Looks good to me. You tested it and found/paid out blocks?

@blackmennewstyle
Copy link
Author

Nice work. Looks good to me. You tested it and found/paid out blocks?

Thank you :)

Yes, i tested it out on testnet for a while and found blocks and even sent payouts.

I have it running on mainnet since few weeks and also found blocks and sent payouts: https://conceal.cedric-crispin.com/

blackmennewstyle and others added 3 commits December 12, 2022 12:44
Add support for Ubiq
Fix couple of issues with EthereumPayoutHandler.cs
Add pool examples for Conceal, Ethereum Classic and Ubiq
Fix few issues with Conceal when user forget to specify the `NetworkType` in the config.json
Rename `NetworkTypeSpecified` to `NetworkTypeOverride` in order to be more comprehensible
@jon4hz
Copy link
Contributor

jon4hz commented Jan 27, 2023

@blackmennewstyle do you think it would be possible to use the libsodium from libethhash? Otherwise there is a lot unnecessary duplicated code.

And maybe create a separate PR for the etc and ubiq stuff to make a review a bit easier?

@blackmennewstyle
Copy link
Author

@jon4hz Yeah i can definitely try to do that as soon as i get some free times :)

@blackmennewstyle
Copy link
Author

Stupid question but how do i create another pull request, it does not seem to allow to create one more as long as this one is pending :/


bool ethash_get_default_dirname(char* strbuf, size_t buffsize)
{
static const char dir_suffix[] = ".ethash/";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make the suffix ".etchash/" ? Would fit better imo.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, sounds like a good idea. I can do the same for ubqhash, it will match with the default name respectively for:

  • geth, when using --classic or --mordor: .etchash/
  • gubiq: .ubqhash/

@@ -352,9 +356,24 @@ public override void Configure(PoolConfig pc, ClusterConfig cc)

// create it if necessary
Directory.CreateDirectory(dagDir);

Copy link
Contributor

Choose a reason for hiding this comment

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

 var dagDir = !string.IsNullOrEmpty(extraPoolConfig?.DagDir) ?
                Environment.ExpandEnvironmentVariables(extraPoolConfig.DagDir) :
                Dag.GetDefaultDagDirectory();

should probably be handled on a per "algo" basis, since each algo has it's own Dag.GetDefaultDagDirectory() function.

Copy link
Author

Choose a reason for hiding this comment

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

Yes absolutely. I can't believe i actually missed it. i will do it right away and test it.

@jon4hz
Copy link
Contributor

jon4hz commented Jan 30, 2023

Stupid question but how do i create another pull request, it does not seem to allow to create one more as long as this one is pending :/

You'd have to create another branch in your source repo and use that new branch for the PR.

@jon4hz
Copy link
Contributor

jon4hz commented Jan 30, 2023

One other thing: Currently the API will return always "ethhash" as algo because of

A while ago I started working on ETC support myself and I built an interface around ethash algo(s) which would solve that issue and also simplify other parts of the code. I never finished that code but parts of it were working. I'll see if I can find it and open a PR.

@oliverw oliverw merged commit 110b986 into oliverw:dev Jan 31, 2023
@oliverw
Copy link
Owner

oliverw commented Jan 31, 2023

Sorry for the late response. Been battling with Covid and then another flu since pretty much christmas.

Pretty impressive job. Implementing a new family is no easy feat 👍

@oliverw
Copy link
Owner

oliverw commented Jan 31, 2023

One other thing: Currently the API will return always "ethhash" as algo because of

A while ago I started working on ETC support myself and I built an interface around ethash algo(s) which would solve that issue and also simplify other parts of the code. I never finished that code but parts of it were working. I'll see if I can find it and open a PR.

That would be awesome. 👍

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

Successfully merging this pull request may close these issues.

None yet

4 participants