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

More coins #353

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

TimeTravelersHackedMe
Copy link

No description provided.

@bobbieltd
Copy link
Contributor

Very abnormal ! LOL ! Why don’t I know all these coins ? Why aren’t there Bitcoin, Ethereum, .. ? Pushing them all is cool ... 😂😂😂 Unbelievable ! This PR is so shit !

@bobbieltd
Copy link
Contributor

Here is not a trash so you can throw your rubbish with unworking and unknown javascripts to it.

@TimeTravelersHackedMe
Copy link
Author

TimeTravelersHackedMe commented Mar 7, 2018 via email

@bobbieltd
Copy link
Contributor

  1. You changed leafApi, if other pool owners don’t know and update the codes. All pools will crash. It’s non sense name change.
  2. For coins javascript, some works some need other changes. You haven’t test them and make a rubbish PR.
  3. Some coins are scam or unknown or shit coins. It will make nodejs-pool officially supporting scam/shit coins.
  4. Let me guess. You don’t understand all these coins javascript. If you are not sure about anything, don’t make others lab rats to test your things. You can test them at your repo or other repos as a lab.

@TimeTravelersHackedMe
Copy link
Author

TimeTravelersHackedMe commented Mar 7, 2018 via email

@TimeTravelersHackedMe
Copy link
Author

I got them from here: https://github.com/ArqTras/nodejs-pool

@Venthos
Copy link

Venthos commented Mar 15, 2018

It looks like ArqTras pulled in the ITNS coin files from my fork, which will not be usable on this repo without also pulling in other dependent changes that I made to the rest of the pool code. The ITNS coin file is not a drop'n'go thing in this situation. Even on ArqTras' fork he didn't pull in my other related changes, which means ITNS is currently unusable on his fork, too.

I imagine many of the other coin files are not as simple as dropping them in place, either. These should be tested before being included. I can state for sure that this PR in its current state won't allow ITNS to be usable.

@Rhys79
Copy link

Rhys79 commented Mar 15, 2018

I can say from experience, forknote based coins require multiple modifications to base nodejs-pool code to function. There is no one size fits all version of nodejs-pool to support shitcoins. Each one is broken in it's own idiotic way, and you have to patch each pool implementation individually for what the crappy devs broke (or didn't fix) in their wallet/daemon code. Patches to fix one coin will likely break the next one, so don't expect to see shitcoins mainlined in Snipa's code.

@TimeTravelersHackedMe
Copy link
Author

TimeTravelersHackedMe commented Mar 15, 2018 via email

@ArqTras
Copy link

ArqTras commented Mar 15, 2018

@Venthos that true and I can confirm that more changes need to be done for runing that coin (ITNS) and yes it is borrowed from Your repo.
On my github I'm working to make deploy bash files to make it easier to install with more than basic sql info.
Some of coins need forknote-utils (Bathmat repository really nice)
A the moment I have tested all coins that run on https://supportcryptonight.com and some of them need more work to run. Also pool code is forked from moneroocean and still developed with cooperation and many thanks to Him for His great job. My github repo is different than Snippa22. So adding coins to public Snipa22 repo should get also dev coin addresses and Snippas poolDev wallet, for some reasons I use my own wallets there and if someone consider to add it that should be corrected. I'm not full coin coder except few of them.

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.

5 participants