zone117x / node-open-mining-portal

A scalable all-in-one easy to setup cryptocurrency mining pool and portal written entirely in Node.js.
GNU General Public License v2.0
1.02k stars 1.03k forks source link

worker name issues (breaking payments) #194

Open bonesoul opened 10 years ago

bonesoul commented 10 years ago

Following this issue; https://github.com/zone117x/node-open-mining-portal/issues/180

@zone117x I've switched to "defaultPoolConfigs" setup and even my validateWorkerUsername is set to true;

a worker like; WALLET_ADDRESS.SOME_STRING_HERE

breaks the payments.

I think somehow nomp thinks the part after dot as another wallet-address and tries to pay him (SOME_STRING_HERE)

zone117x commented 10 years ago

Thanks for posting - I'll look into it when I get the time. Perhaps the config is getting ignored or there is a typo or something.

bonesoul commented 10 years ago

and with this it's quite possible to pause payments for any nomp-running pools around, so guess it's a bit an urgent issue.

l0rdicon commented 10 years ago

I made a simple pull request for this. A regex checks to make sure no unwanted characters are part of the string when validateWorkerUsername is false.

If you set validateWorkUsername to false on your pool I suggest implementing my pull request or something similar asap.

https://github.com/zone117x/node-open-mining-portal/pull/210

bonesoul commented 10 years ago

@l0rdicon my pool is already set to validateWorkerUsername = true

azamatms commented 10 years ago

What is the error, does it break the addressAmounts array or does sendmany output something?

bonesoul commented 10 years ago

Breakes sendmany.

So if you basically want to break payments of any nomp-pool as your miner username use WALLET_ADDRESS.somestring

and payments will be stalled. The admin now has to fix redis db and remove that ".somestring" part from rounds or the pool will not be able to pay miners.

Outstanding issue waiting since the last week.

nicoschtein commented 10 years ago

Was validateWorkerUsername true from the beginning ? Or you changed it from false to true after it got into the Redis db ?

On May 19, 2014, at 7:30 PM, Hüseyin Uslu notifications@github.com wrote:

Breakes sendmany.

So if you basically want to break payments of any nomp-pool as your miner username use WALLET_ADDRESS.somestring

and payments will be stalled. The admin now has to fix redis db and remove that ".somestring" part from rounds or the pool will not be able to pay miners.

Outstanding issue waiting since the last week.

— Reply to this email directly or view it on GitHub.

bonesoul commented 10 years ago

Yes i always had it true. Though i moved to defaultPoolConfig's option introduced lately (https://github.com/zone117x/node-open-mining-portal/blob/master/config_example.json#L12)

which is probably the basic reason behind the bug.

nicoschtein commented 10 years ago

So basically you say that the validation is not working, since having that true makes the auth for each worker validate the username as a valid address against coin daemon. Does this happen with any coin ? I can take a look into that part of the validation code to see if something's off.

On May 19, 2014, at 7:39 PM, Hüseyin Uslu notifications@github.com wrote:

Yes i always had it true.

— Reply to this email directly or view it on GitHub.

bonesoul commented 10 years ago

lottocoin is having the issue - it accepts WALLET_ADDRESS.somestring as a valid username and then sendmany will fail trying to send as it takes "WALLET_ADDRESS.somestring" as a whole address.

nicoschtein commented 10 years ago

Ok then we have the reason, that coind is not validating addresses the way it should. You can submit an issue to that coin repo, maybe coin dev can fix it. Regarding nomp, I think it works fine. You may want to close this issue for now. If this happens with a coin that validates addresses then reopen!

On May 19, 2014, at 7:44 PM, Hüseyin Uslu notifications@github.com wrote:

lottocoin is having the issue - it accepts WALLET_ADDRESS.somestring as a valid username and then sendmany will fail trying to send as it takes "WALLET_ADDRESS.somestring" as a whole address.

— Reply to this email directly or view it on GitHub.

bonesoul commented 10 years ago

the thing is that this started happening after; https://github.com/zone117x/node-open-mining-portal/blob/master/config_example.json#L12 chnage

nicoschtein commented 10 years ago

Hmm Disable nomp website, set enabled false under website.

On May 19, 2014, at 8:08 PM, Hüseyin Uslu notifications@github.com wrote:

the thing is that this started happening after; https://github.com/zone117x/node-open-mining-portal/blob/master/config_example.json#L12 chnage

— Reply to this email directly or view it on GitHub.

bonesoul commented 10 years ago

nomp has to trim out ".string" part..

nicoschtein commented 10 years ago

Disregard my last comment I mixed up issues in my email...

bonesoul commented 10 years ago

@nicoschtein , @zone117x

I've added again validateWorkerUsername = true to directly to pool-config file, and no changes, he still gets authenticated.

here's the log line FYI

 00:54:10.069  2014-05-22 21:54:09 [Pool]   [lottocoin] (Thread 2) Authorized Lqv8gzqKXQuU99SOMEADDRESS.ocean:ocean [IP]

nomp accepts this username without stripping out the .ocean part in the username and tries to pay to;

Lqv8gzqKXQuU99SOMEADDRESS.ocean
crackfoo commented 10 years ago

I wasn't aware you could use WALLET_ADDRESS.something as the worker name... what's the theory behind that? To allow vardiff to calculate diff separately?

bonesoul commented 10 years ago

@crackfoo doesn't seem so, just a bug.