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

Changes for the Monero v1 PoW #1126

Closed
wants to merge 1 commit into from
Closed

Conversation

vtnerd
Copy link
Contributor

@vtnerd vtnerd commented Mar 2, 2018

Monero v1 PoW changes. There was a warning about merging into master. I can update the patch for dev, but I'm not sure if that is what you want in this case. Please see monero PR to discuss the PoW changes.

Some implementation notes:

  • I didn't "catch" how the build system was injecting an ITERATION flag until after I tested this patch. It should be easy to add a MONERO or similar macro define to remove branches in the AEON OpenCL inner loop.
  • sgminer has implemented a macro'ed version for the OpenCL implementation. This is interesting because it detects the version in the CPU, then swaps out the kernel. Its not immediately clear this is the best approach because its going to vary based on what the pool provides. So it might be lots of book keeping during the fork. However, it won't branch in the inner loop. This is a little harder to implement but not terrible.
  • The cuda version uses a boolean template for monero to remove branches from the aeon version.
  • The cuda version could alternatively pass the input and state instead of adding another global variable, however that second process function now has to do two reads. So the alternative would remove a global write in the expansion phase but add another global read in the function containing the process loop.
  • The CPU version uses a booleam template for monero to remove branches from the aeon version.
  • The first tweak in the CPU version accesses the scratchpad directly after the simd write. I thought about putting that type (the 128-bit type) into a union, but it should mess with registers a bit since the tweak cannot be applied to multiple integers. So it would be messing around with simd registers vs L1 access. Perhaps you can twiddle something better.
  • The second tweak modifies the stack variable in the 1 and 2 at a time implementations. For 3, 4, 5 at a time, the second tweak modifies the scratchpad directly. Alternatively, you could setup a 128-bit value where the "first" 64-bits are zero. Since zero is the identity the CPU could apply the result without smashing up the simd registers. This would require an additional 128-bit value, so I wasn't sure because it will mess with register allocations.
  • It should be easy to add a template variable to remove branching from the monero version in the CPU and cuda versions, with the risk of more code generated. Possibly worth it.

Also see the PR linked above for "reference" hashes. Make sure you get my last commit if you want to verify.

@psychocrypt
Copy link
Collaborator

psychocrypt commented Mar 2, 2018 via email

@vtnerd
Copy link
Contributor Author

vtnerd commented Mar 3, 2018

I implemented a macro define for xmrig-amd that removes branches from the aeon build. Its not difficult to add to this implementation, but I will leave that to the team unless otherwise requested.

@fireice-uk
Copy link
Owner

@vtnerd Thanks for your work. Can you tell us when the PoW change is going get merged into monerod?

@GabrielKesler
Copy link

GabrielKesler commented Mar 4, 2018

As developers, in your opinion, how long will this PoW change keep the botnets out ? Is it a quick fix for them ?

@psychocrypt
Copy link
Collaborator

psychocrypt commented Mar 4, 2018 via email

@Molokai
Copy link

Molokai commented Mar 8, 2018

POW change to take effect on March 28, block 1539500:

monero-project/monero@84decbe

https://www.reddit.com/r/Monero/comments/82pulx/network_upgrade_scheduled_for_block_1544555_on_28/
(Note, discrepancy in the title of the reddit and github pullrequest - and the actual code)

@nbourbaki909
Copy link

Will there be a new switch in the config for monero, aeon, and a third for CN coins that haven't changed their POW?

Copy link

@stempelo stempelo left a comment

Choose a reason for hiding this comment

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

Output from input "This is a test"

@@ -228,38 +228,38 @@ bool minethd::self_test()
cn_hash_fun_multi hashf_multi;

hashf = func_selector(::jconf::inst()->HaveHardwareAes(), false, mineMonero);
hashf("This is a test", 14, out, ctx[0]);
hashf("This is a test", 14, out, ctx[0], 0);
bResult = memcmp(out, "\xa0\x84\xf0\x1d\x14\x37\xa0\x9c\x69\x85\x40\x1b\x60\xd4\x35\x54\xae\x10\x58\x02\xc5\xf5\xd8\xa9\xb3\x25\x36\x49\xc0\xbe\x66\x05", 32) == 0;

Choose a reason for hiding this comment

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

Is it correct \xa0\x84\xf0\x1d\x14\x37\xa0\x9c\x69\x85\x40\x1b\x60\xd4\x35\x54\xae\x10\x58\x02\xc5\xf5\xd8\xa9\xb3\x25\x36\x49\xc0\xbe\x66\x05
for the input "This is a test" ??

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, for the original cryptonote hash function.

@fireice-uk
Copy link
Owner

@nbourbaki909

Will there be a new switch in the config for monero, aeon, and a third for CN coins that haven't changed their POW?

Yes, we are not dropping support for any coins. As such you will need to use monero2 to actually use the new monero algo.

@plavirudar
Copy link

@fireice-uk Will the monero2 algo be backwards compatible with older pools? From my understanding, this is a change in the block template and it should be possible for the miner to dynamically adjust algos.

@jtgrassie jtgrassie mentioned this pull request Mar 18, 2018
@fireice-uk
Copy link
Owner

@plavirudar you will simply need to enter the coin name that you want to mine, with exception of Monero, which will be called monero2 in the config. Algorithms will be mapped internally.

@fireice-uk
Copy link
Owner

@vtnerd Some of your code went into 1e7911e I added you as a co-author on that commit. Thanks for the contribution.

@fireice-uk fireice-uk closed this Mar 25, 2018
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

Successfully merging this pull request may close these issues.

None yet

8 participants