simplecoin (OP)
|
|
June 26, 2011, 04:07:56 PM |
|
Hi Mike, I've been hacking on your code a bit for the last day again and testing everything on a private testnet in a box setup with 2 nodes and there are some things puzzling me about how the cronjob.php code works. Mainly in the logic. Ill try and explain each on separately. Checking if we found a block. Here we query bitcoind with a listtransactions query with no extra arguments so we get a default count of the last 10 transactions. We check if the category is "generate" and if so we assume we may have found a block and start doing some logic to make sure of that. This is where i already dont get it. On my testnet setup, when we find a new block its category is immature and remains that way until it receives 120 confirmations at which point it changes from "immature" to "generate". see below: { "account" : "", "category" : "generate", "amount" : 50.00000000, "confirmations" : 120, "txid" : "47a833ba9e311a839988136a283d9e6a8d05f7c609cc5ec26de397f352472751", "time" : 1300079031 }, { "account" : "", "category" : "immature", "amount" : 50.00000000, "confirmations" : 119, "txid" : "c31c89383ec77234cc2c2cbeca87edbc2b6348edba9d0e6c343fc34f1ed730ff", "time" : 1300079115 },
This means that by the time we have a transaction with the "generate" category it will have long ago fallen out of the data returned by the listtransactions query since that only shows the last 10 transactions. I realize that works a bit different on an active live network but it still seems odd to me that at least the way im reading the code that you rely on the assumption that we wont find more than 10 blocks out of 120 (or something like this - maybe i made a math error). If i leave it set like this on a private testnet the side effect is that no blocks are ever seen as found and the round never ends because by the time the block has changed from immature to generate it has fallen out of the list of the last 10 transactions because our pool is finding every single block. Does this make sense? Updateing confirmsThe next part also confuses me for a similar reason. NExt we go through all the transactions and update their confirms. Here again we check the category if its set to "receive" but from what i can see an immature or generated block will never have this category. Its category is always either generate or immature (on testnet). see below: here is a closer look at a txid which has matured # bitcoind -datadir=testnet/1 gettransaction 47a833ba9e311a839988136a283d9e6a8d05f7c609cc5ec26de397f352472751
{ "amount" : 50.00000000, "confirmations" : 120, "txid" : "47a833ba9e311a839988136a283d9e6a8d05f7c609cc5ec26de397f352472751", "time" : 1300079031, "details" : [ { "account" : "", "category" : "generate", "amount" : 50.00000000 } ] }
and a closer look at one right after it with only 119 confirms and will be maturing next round: # bitcoind -datadir=testnet/1 gettransaction c31c89383ec77234cc2c2cbeca87edbc2b6348edba9d0e6c343fc34f1ed730ff
{ "amount" : 0.00000000, "confirmations" : 119, "txid" : "c31c89383ec77234cc2c2cbeca87edbc2b6348edba9d0e6c343fc34f1ed730ff", "time" : 1300079115, "details" : [ { "account" : "", "category" : "immature", "amount" : 50.00000000 } ] }
so the effect here is that nothing will ever get its confirms updated unless your check is changed to immature or generate. Can you explain to me the "receive" category and why you are using this as a check? Calculating payouts and ending the roundThis part seems to be pretty straightforward assuming that the previous 2 steps went right with the exception that it looks like in this part of the code is where we close out the round (am i wrong here). It seems that the round doesnt get closed out properly if this part of the code doesnt run (for example because there are not yet enough confirms on any of the blocks.) Is it true that this is where the round ends or at least some part of the logic for ending a round is in here? Or is that completely all up top of the code where we move all old shares to shares_history. Other random questionsWouldnt it be better to track 2 balance amounts - an unconfirmed and a confirmed amount and only allow payout of course on the confirmed balance? It seems the way the code is written now that a user will have no idea they might have earned until all found blocks have at least 120 confirms. So if you are just starting up a new pool, your new users will show a 0 balance until at least 120 or so blocks have been found on the network which would confuse them and maybe scare them off right from the start as it would look like they arent earning anything. Am i correct about it working like this? I hope im explaining this well enough for you to get my point. It;s late and i know i might not be making myself very clear. Thanks for any clarification you can give!! Maybe im missing something very simple here. Yes, that's on the agenda. The code was based around modifications to Xenlands original. After seeing it in action, I think there are better ways to implement it. Thanks for the input, I'll definitely put some weight to it.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
AnnihilaT
|
|
June 26, 2011, 09:48:26 PM |
|
Hi Mike, Maybe i didnt make it clear that im implementing these things also where i think they make sense so maybe we should talk about working together on the project? Also, i had alot of questions in there as well for you and wonder if you could maybe (if you have time) answer some of my questions and let me know if things are actually as i understand them to be or if im making some wrong assumptions. While i am testing the changes im making, i also want to be sure that im not making some false assumptions about how it all works. Particularly im interested to hear your thoughts about the whole cronjob logic because im planning on making some significant changes here and would be nice to hear another opinion from someone who has used the code live. I can only test on testnet. Can you verify that under normal operation with the code you run that indeed shares just keep counting up until the first block we found has 120 confirms and that maybe some other things work kind of weird as well until we hit the first block with 120 confirms? Perhaps with my changes ive intoduced some oddities and want to make sure that the problems im "fixing" arent self induced Anni
|
|
|
|
simplecoin (OP)
|
|
June 26, 2011, 10:26:31 PM Last edit: June 26, 2011, 10:55:06 PM by simplecoin |
|
Hi Mike, Maybe i didnt make it clear that im implementing these things also where i think they make sense so maybe we should talk about working together on the project? Also, i had alot of questions in there as well for you and wonder if you could maybe (if you have time) answer some of my questions and let me know if things are actually as i understand them to be or if im making some wrong assumptions. While i am testing the changes im making, i also want to be sure that im not making some false assumptions about how it all works. Particularly im interested to hear your thoughts about the whole cronjob logic because im planning on making some significant changes here and would be nice to hear another opinion from someone who has used the code live. I can only test on testnet. Can you verify that under normal operation with the code you run that indeed shares just keep counting up until the first block we found has 120 confirms and that maybe some other things work kind of weird as well until we hit the first block with 120 confirms? Perhaps with my changes ive intoduced some oddities and want to make sure that the problems im "fixing" arent self induced Anni As far as working together, currently I want to personally check any code that comes through before it posts. That said, I tend to implement just about any pull request that is beneficial and benign. I'm currently going through the cronjob logic myself. It works currently since blocks aren't generated very often, but it needed a heavy overhaul. I'm changing the way it checks confirms & generations. The initial block now looks at generate and immature in the past 200 txs. The second block will look through networkblocks and not transactions. it will check bitcoind against txid to count confirms if less than 120 in networkblocks. After that is all said and done, I intend to fork bitcoin & pushpool into one codebase to cut down on getwork traffic and expose new rpc commands to filter transactions based on category. To answer your other questions: calculating payouts: yes this ends the round. by setting counted='1' the shares are now invalid for the current round. confirmed & unconfirmed balances: I think that would just add further confusion. Estimated balances alone have caused confusion that is a real pain. I'm tempted to even remove that from my own pool as it seems to anger people when they can't cashout a non-existent balance.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
simplecoin (OP)
|
|
June 26, 2011, 11:17:34 PM |
|
Here is my proposed improved method... //Get list of transactions $transactions = $bitcoinController->query("listtransactions \"*\" 200");
//Go through all the transactions check if there is 50BTC inside $numAccounts = count($transactions);
for($i = 0; $i < $numAccounts; $i++) { //Check for 50BTC inside only if they are in the generate category if($transactions[$i]["amount"] >= 50 && ($transactions[$i]["category"] == "generate" || $transactions[$i]["category"] == "immature")) { //At this point we may or may not have found a block, //Check to see if this account addres is already added to `networkBlocks` $accountExistsQ = mysql_query("SELECT id FROM networkBlocks WHERE accountAddress = '".$transactions[$i]["txid"]."' ORDER BY blockNumber DESC LIMIT 0,1")or die(mysql_error()); $accountExists = mysql_num_rows($accountExistsQ);
//Insert txid into latest network block if (!$accountExists) { //Get last winning block $lastSuccessfullBlockQ = mysql_query("SELECT n.id FROM networkBlocks n, winning_shares w where n.blockNumber = w.blockNumber ORDER BY w.id DESC LIMIT 1"); $lastSuccessfullBlockR = mysql_fetch_object($lastSuccessfullBlockQ); $lastEmptyBlock = $lastSuccessfullBlockR->id;
$insertBlockSuccess = mysql_query("UPDATE networkBlocks SET accountAddress = '".$transactions[$i]["txid"]."' WHERE id = $lastEmptyBlock")or die(mysql_error()); //Update site balance for tx fee $poolReward = $transactions[$i]["amount"] - $bonusCoins; mysql_query("UPDATE settings SET value = value + $poolReward WHERE setting='sitebalance'"); } } }
//Go through networkblocks and update confirms as needed $winningAccountQ = mysql_query("SELECT id, accountAddress FROM networkBlocks WHERE accountAddress <> '' AND confirms < 120"); while ($winningAccountR = mysql_fetch_object($winningAccountQ)) { $txInfo = $bitcoinController->gettransaction($winningAccountR->accountAddress); if (count($txInfo["confirmations"]) > 0) { mysql_query("UPDATE networkBlocks SET confirms = ".$txInfo["confirmations"]." WHERE id = $winningAccountR->id"); } } Also, I'm thinking of ways to remove shares_history, I hate having so many table joins slowing things down.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
AnnihilaT
|
|
June 27, 2011, 12:39:40 AM |
|
As far as working together, currently I want to personally check any code that comes through before it posts. That said, I tend to implement just about any pull request that is beneficial and benign.
Makes sense. Ill have to see if i can get up to speed on GIT. I always use SVN myself. I'm currently going through the cronjob logic myself. It works currently since blocks aren't generated very often, but it needed a heavy overhaul.
I'm changing the way it checks confirms & generations. The initial block now looks at generate and immature in the past 200 txs. The second block will look through networkblocks and not transactions. it will check bitcoind against txid to count confirms if less than 120 in networkblocks.
Funny...ive been sitting here doing the exact same thing for the last couple hours and finally think i have it working quite nicely. I think 200 might be a bit excessive and have set mine for now to 60 i think. This seems to catch everything. Certainly alot better than 10 After that is all said and done, I intend to fork bitcoin & pushpool into one codebase to cut down on getwork traffic and expose new rpc commands to filter transactions based on category.
Sounds good. To answer your other questions: calculating payouts: yes this ends the round. by setting counted='1' the shares are now invalid for the current round. confirmed & unconfirmed balances: I think that would just add further confusion. Estimated balances alone have caused confusion that is a real pain. I'm tempted to even remove that from my own pool as it seems to anger people when they can't cashout a non-existent balance.
RE: ending the round... Yes this makes sense. It only seems to make more sense to my brain at this very moment to end the round immediately when a block is found and we start counting confirms rather than wait until we have 120 confirms on our first block to end a round. I just think its going to cause lots of confusion to users of someone who starts up a brand new pool. It should work completely normally and intuitive once you get to your first 120 confirms on that first found block but before that.... yeah... weird. Or do i misunderstand how it works? For the rest of what you say i agree with!
|
|
|
|
AnnihilaT
|
|
June 27, 2011, 12:57:35 AM |
|
Here is my proposed improved method... //Get list of transactions $transactions = $bitcoinController->query("listtransactions \"*\" 200");
//Go through all the transactions check if there is 50BTC inside $numAccounts = count($transactions);
for($i = 0; $i < $numAccounts; $i++) { //Check for 50BTC inside only if they are in the generate category if($transactions[$i]["amount"] >= 50 && ($transactions[$i]["category"] == "generate" || $transactions[$i]["category"] == "immature")) { //At this point we may or may not have found a block, //Check to see if this account addres is already added to `networkBlocks` $accountExistsQ = mysql_query("SELECT id FROM networkBlocks WHERE accountAddress = '".$transactions[$i]["txid"]."' ORDER BY blockNumber DESC LIMIT 0,1")or die(mysql_error()); $accountExists = mysql_num_rows($accountExistsQ);
//Insert txid into latest network block if (!$accountExists) { //Get last winning block $lastSuccessfullBlockQ = mysql_query("SELECT n.id FROM networkBlocks n, winning_shares w where n.blockNumber = w.blockNumber ORDER BY w.id DESC LIMIT 1"); $lastSuccessfullBlockR = mysql_fetch_object($lastSuccessfullBlockQ); $lastEmptyBlock = $lastSuccessfullBlockR->id;
$insertBlockSuccess = mysql_query("UPDATE networkBlocks SET accountAddress = '".$transactions[$i]["txid"]."' WHERE id = $lastEmptyBlock")or die(mysql_error()); //Update site balance for tx fee $poolReward = $transactions[$i]["amount"] - $bonusCoins; mysql_query("UPDATE settings SET value = value + $poolReward WHERE setting='sitebalance'"); } } }
//Go through networkblocks and update confirms as needed $winningAccountQ = mysql_query("SELECT id, accountAddress FROM networkBlocks WHERE accountAddress <> '' AND confirms < 120"); while ($winningAccountR = mysql_fetch_object($winningAccountQ)) { $txInfo = $bitcoinController->gettransaction($winningAccountR->accountAddress); if (count($txInfo["confirmations"]) > 0) { mysql_query("UPDATE networkBlocks SET confirms = ".$txInfo["confirmations"]." WHERE id = $winningAccountR->id"); } } Also, I'm thinking of ways to remove shares_history, I hate having so many table joins slowing things down. Funny! I was just doing more or less the same. My code looks almost identical. //Query bitcoind for list of transactions $transactions = $bitcoinController->query('listtransactions', '', '60');
//Go through all the transactions check if there is 50BTC inside $numAccounts = count($transactions);
for($i = 0; $i < $numAccounts; $i++){ //Check for 50BTC in each transaction (even when immature so we can start tracking confirms) if($transactions[$i]["amount"] >= 50 && ($transactions[$i]["category"] == "generate" || $transactions[$i]["category"] == "immature")) {
Also it look like the mtgox.php is broken. Not sure why so i hacked this in real quick. It depends on having curl installed at the command line: // Update MtGox last price via curl, 3 second timeout on connection $mtgox_ticker = exec("/usr/bin/curl -q -s --connect-timeout 3 'https://mtgox.com/code/data/ticker.php'"); if (!is_null($mtgox_ticker)) { $ticker_obj = json_decode($mtgox_ticker); if (intval($ticker_obj->ticker->last) > 0) { $settings->setsetting('mtgoxlast', $ticker_obj->ticker->last); } }
Furthur: logout.php is completely broken for me (or was). It was impossible to logout. Im using this instead. Does this look sane to you? //Include site functions include("includes/requiredFunctions.php"); include("includes/universalChecklogin.php");
$user = $userInfo->username; $checkPassQ = mysql_query("SELECT id FROM webUsers WHERE username = '".$user."' LIMIT 0,1"); $checkPass = mysql_fetch_object($checkPassQ); $userid = $checkPass->id;
setcookie($cookieName, false); unset($cookieValid); mysql_query("UPDATE `webUsers` SET `sessionTimeoutStamp` = '0' WHERE `id` = '" .$userid. "'"); header("Location: /index.php");
Some of the other things ive done so far are: - added worker deletion and improved layout of account details a bit - logout menu item - rewritten the stats leftsidebar thing - started working on some nicer admin features and stats - improved layout of the stats page - Fixed a security issue (maybe introduced 1 or 2 as well ) - gotten rid of the login.php refresh page and replace it with a 2 second sleep after each login attempt (plans to make this safer) - prolly lots more... anyway ive been busy
|
|
|
|
simplecoin (OP)
|
|
June 27, 2011, 02:40:48 PM |
|
New updates for v2 checked in.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
simplecoin (OP)
|
|
June 27, 2011, 03:04:53 PM |
|
Some of the other things ive done so far are: - added worker deletion and improved layout of account details a bit did you merge the workers existing uncounted shares to another worker? Also, did you make sure that there is at least 1 worker (to keep from losing all shares)? - Fixed a security issue (maybe introduced 1 or 2 as well ) Security is my biggest priority. If you could forward anything you see it is greatly appreciated. - gotten rid of the login.php refresh page and replace it with a 2 second sleep after each login attempt (plans to make this safer)
Good idea, I was thinking a captcha after 2 unsuccessful logins by any 1 ip.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
simplecoin (OP)
|
|
June 27, 2011, 03:27:33 PM |
|
More updates to v2.
Currently folding in parts of Ozco.in fork to the v2 code where it makes sense.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
AnnihilaT
|
|
June 27, 2011, 06:03:10 PM |
|
did you merge the workers existing uncounted shares to another worker? Also, did you make sure that there is at least 1 worker (to keep from losing all shares)?
Oh! Good point! Ill have to have another look at how i did that Security is my biggest priority. If you could forward anything you see it is greatly appreciated.
not sure if its a huge issue but this is one that pops to mind: //Check if the cookie is set, if so check if the cookie is valid if(isSet($_COOKIE[$cookieName])){ $cookieValid = false; $ip = $_SERVER['REMOTE_ADDR']; //Get Ip address for cookie validation $validateCookie = new checkLogin(); $cookieValid = $validateCookie->checkCookie(mysql_real_escape_string($_COOKIE[$cookieName]), $ip); $userId = $validateCookie->returnUserId($_COOKIE[$cookieName]);
//ensure userId is integer to prevent sql injection attack if (!is_numeric($userId)) { $userId = 0; exit; }
- gotten rid of the login.php refresh page and replace it with a 2 second sleep after each login attempt (plans to make this safer)
Good idea, I was thinking a captcha after 2 unsuccessful logins by any 1 ip. I hate captcha's. Gonna try to think of something nicer... for now im just sleeping 2 secs and then header("Location:") back to index.php.
|
|
|
|
simplecoin (OP)
|
|
June 27, 2011, 06:17:53 PM |
|
Many Ozcoin improvements merged into main tree. Excellent work by Wayno & tom lightspeed!
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
simplecoin (OP)
|
|
June 27, 2011, 06:28:57 PM |
|
Anni, that security fix is in the repo Also, worker update/delete is there from ozcoin code. (it doesn't roll in old workers yet though). The login attempt ban is also now in place.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
simplecoin (OP)
|
|
June 28, 2011, 02:10:12 PM |
|
Version 2 stable released, includes upgrade script from simplecoin 1.0 database.
All sql files are now in the sql/ directory.
Cheers!
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
AnnihilaT
|
|
June 28, 2011, 05:52:14 PM Last edit: June 29, 2011, 12:45:47 AM by AnnihilaT |
|
Mike, I think ive decided to fork the codebase you have maintained into a new project as i see you heading in a different direction with v2 than what originally attracted me in the first place. Ill be keeping a SVN repo up and open so that my changes will be also available. I will still probably borrow from your project as you can do from mine where and when applicable. At this point i have more than 1390 lines of differences to the original v1 code that i checked out and so this just seems to make more sense to me. Also if you are curious, my version is running at http://mining.mainframe.nl/Let me know if you have any questions or concerns. Anni
|
|
|
|
simplecoin (OP)
|
|
June 28, 2011, 07:11:05 PM |
|
Mike, I think ive decided to fork the codebase you have maintained into a new project as i see you heading in a different direction with v2 than what originally attracted me in the first place. Ill be keeping a SVN repo up and open so that my changes will be also available. I will still probably borrow from your project as you can do from mine where and when applicable. At this point i have more than 1390 lines of differences to the original v1 code that i checked out and so this just seems to make more sense to me. Also if you are curious, my version is running at http://btc.mfis.netLet me know if you have any questions or concerns. Anni Fork away Let me know where your repo is and I'll put a link up on the OP.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
simplecoin (OP)
|
|
June 28, 2011, 11:09:12 PM |
|
Experimental branch created in github. All new development will go here until it is merged into a new version.
That said, just added Server load stats to the experimental branch. Should be good and stable. No db changes required.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
AnnihilaT
|
|
June 29, 2011, 12:50:55 AM |
|
Mike, I think ive decided to fork the codebase you have maintained into a new project as i see you heading in a different direction with v2 than what originally attracted me in the first place. Ill be keeping a SVN repo up and open so that my changes will be also available. I will still probably borrow from your project as you can do from mine where and when applicable. At this point i have more than 1390 lines of differences to the original v1 code that i checked out and so this just seems to make more sense to me. Also if you are curious, my version is running at http://btc.mfis.netLet me know if you have any questions or concerns. Anni Fork away Let me know where your repo is and I'll put a link up on the OP. Should be up at: svn://svn.mfis.net/btc/
|
|
|
|
simplecoin (OP)
|
|
June 29, 2011, 03:05:28 AM |
|
oops, server load stat was was 100x too high, thanks jine for pointing that out.
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
simplecoin (OP)
|
|
June 29, 2011, 04:56:35 PM |
|
pushpool v5, bitcoin v0.3.23 (or if you are comfortable compiling add the multi-thread diff from the 20btc bounty project page)
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
simplecoin (OP)
|
|
June 30, 2011, 07:07:38 AM |
|
Thanks, got everything up and running. Only problem is when I try and access the admin page I get a 500 error...I have set the flag to 1 in the database ....any idea why I get this error only when trying to access the admin panel?
Thanks!
your best bet is to turn on display_errors in php.ini
|
Donations: 1VjGJHPtLodwCFBDWsHJMdEhqRcRKdBQk
|
|
|
|