From 84b40e08a2f80ea0114804e3dfc861b8e95f57e8 Mon Sep 17 00:00:00 2001 From: "Jonathan \"Duke\" Leto" Date: Sun, 12 Aug 2018 20:31:45 +0200 Subject: [PATCH 1/5] Start adding tests for rewardsunlock --- qa/rpc-tests/cryptoconditions.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qa/rpc-tests/cryptoconditions.py b/qa/rpc-tests/cryptoconditions.py index 849d7daea..9e7ddf8e5 100755 --- a/qa/rpc-tests/cryptoconditions.py +++ b/qa/rpc-tests/cryptoconditions.py @@ -207,6 +207,10 @@ class CryptoConditionsTest (BitcoinTestFramework): result = rpc.rewardsaddfunding("STUFF", txid, "100") assert_equal(result['result'], 'success') + fundingtxid = result['hex'] + assert fundingtxid, "got funding txid" + + result = rpc.rewardsunlock("STUFF", fundingtxid) def run_test (self): print("Mining blocks...") From 66027c0219598109a8fa86cddafe9a9bc7f49d80 Mon Sep 17 00:00:00 2001 From: "Jonathan \"Duke\" Leto" Date: Mon, 13 Aug 2018 09:55:17 +0200 Subject: [PATCH 2/5] Add rewards tests --- qa/rpc-tests/cryptoconditions.py | 23 +++++++++++++++++++++-- src/wallet/rpcwallet.cpp | 25 ++++++++++++++++++------- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/qa/rpc-tests/cryptoconditions.py b/qa/rpc-tests/cryptoconditions.py index 9e7ddf8e5..845d387b4 100755 --- a/qa/rpc-tests/cryptoconditions.py +++ b/qa/rpc-tests/cryptoconditions.py @@ -184,7 +184,7 @@ class CryptoConditionsTest (BitcoinTestFramework): result = rpc.rewardsinfo("none") assert_equal(result['result'], 'error') - result = rpc.rewardscreatefunding("STUFF", "1000", "5", "1", "10", "10") + result = rpc.rewardscreatefunding("STUFF", "1000", "5", "0", "10", "10") assert result['hex'], 'got raw xtn' txid = rpc.sendrawtransaction(result['hex']) assert txid, 'got txid' @@ -195,7 +195,7 @@ class CryptoConditionsTest (BitcoinTestFramework): assert_equal(result['result'], 'success') assert_equal(result['name'], 'STUFF') assert_equal(result['APR'], "5.00000000") - assert_equal(result['minseconds'], 86400) + assert_equal(result['minseconds'], 0) assert_equal(result['maxseconds'], 864000) assert_equal(result['funding'], "1000.00000000") assert_equal(result['mindeposit'], "10.00000000") @@ -210,7 +210,26 @@ class CryptoConditionsTest (BitcoinTestFramework): fundingtxid = result['hex'] assert fundingtxid, "got funding txid" + # the previous xtn has not been broadcasted yet result = rpc.rewardsunlock("STUFF", fundingtxid) + assert_equal(result['result'], 'error') + + # wrong plan name + result = rpc.rewardsunlock("SHTUFF", fundingtxid) + assert_equal(result['result'], 'error') + + txid = rpc.sendrawtransaction(fundingtxid) + assert txid, 'got txid from sendrawtransaction' + + # confirm the xtn above + rpc.generate(1) + + result = rpc.rewardsunlock("STUFF", fundingtxid) + # currently failing because reward is not > txfee? + # assert_equal(result['result'], 'success') + + result = rpc.rewardslock("STUFF", fundingtxid, "-5") + assert_equal(result['result'], 'error') def run_test (self): print("Mining blocks...") diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 6329d4a52..1654be8ba 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -5003,7 +5003,7 @@ UniValue rewardscreatefunding(const UniValue& params, bool fHelp) UniValue rewardslock(const UniValue& params, bool fHelp) { - UniValue result(UniValue::VOBJ); char *name; uint256 fundingtxid; uint64_t amount; std::string hex; + UniValue result(UniValue::VOBJ); char *name; uint256 fundingtxid; int64_t amount; std::string hex; if ( fHelp || params.size() != 3 ) throw runtime_error("rewardslock name fundingtxid amount\n"); if ( ensure_CCrequirements() < 0 ) @@ -5014,11 +5014,19 @@ UniValue rewardslock(const UniValue& params, bool fHelp) fundingtxid = Parseuint256((char *)params[1].get_str().c_str()); amount = atof(params[2].get_str().c_str()) * COIN; hex = RewardsLock(0,name,fundingtxid,amount); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create rewards lock transaction")); + if ( amount > 0 ) { + if ( hex.size() > 0 ) + { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else { + result.push_back(Pair("result", "error")); + result.push_back(Pair("error", "couldnt create rewards lock transaction")); + } + } else { + result.push_back(Pair("result", "error")); + result.push_back(Pair("error", "amount must be positive")); + } return(result); } @@ -5070,7 +5078,10 @@ UniValue rewardsunlock(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create rewards unlock transaction")); + } else { + result.push_back(Pair("result", "error")); + result.push_back(Pair("error", "couldnt create rewards unlock transaction")); + } return(result); } From ef44dd00e197cbc8068d9a8a03fa324a185740d3 Mon Sep 17 00:00:00 2001 From: "Jonathan \"Duke\" Leto" Date: Wed, 15 Aug 2018 10:21:59 +0200 Subject: [PATCH 3/5] more rewards tests --- qa/rpc-tests/cryptoconditions.py | 45 ++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/qa/rpc-tests/cryptoconditions.py b/qa/rpc-tests/cryptoconditions.py index 845d387b4..ad7ee7467 100755 --- a/qa/rpc-tests/cryptoconditions.py +++ b/qa/rpc-tests/cryptoconditions.py @@ -184,7 +184,7 @@ class CryptoConditionsTest (BitcoinTestFramework): result = rpc.rewardsinfo("none") assert_equal(result['result'], 'error') - result = rpc.rewardscreatefunding("STUFF", "1000", "5", "0", "10", "10") + result = rpc.rewardscreatefunding("STUFF", "7777", "25", "0", "10", "10") assert result['hex'], 'got raw xtn' txid = rpc.sendrawtransaction(result['hex']) assert txid, 'got txid' @@ -194,10 +194,10 @@ class CryptoConditionsTest (BitcoinTestFramework): result = rpc.rewardsinfo(txid) assert_equal(result['result'], 'success') assert_equal(result['name'], 'STUFF') - assert_equal(result['APR'], "5.00000000") + assert_equal(result['APR'], "25.00000000") assert_equal(result['minseconds'], 0) assert_equal(result['maxseconds'], 864000) - assert_equal(result['funding'], "1000.00000000") + assert_equal(result['funding'], "7777.00000000") assert_equal(result['mindeposit'], "10.00000000") assert_equal(result['fundingtxid'], txid) @@ -205,11 +205,14 @@ class CryptoConditionsTest (BitcoinTestFramework): result = rpc.rewardsaddfunding("STUFF", txid, "0") assert_equal(result['result'], 'error') - result = rpc.rewardsaddfunding("STUFF", txid, "100") + result = rpc.rewardsaddfunding("STUFF", txid, "555") assert_equal(result['result'], 'success') fundingtxid = result['hex'] assert fundingtxid, "got funding txid" + result = rpc.rewardslock("STUFF", fundingtxid, "7") + assert_equal(result['result'], 'error') + # the previous xtn has not been broadcasted yet result = rpc.rewardsunlock("STUFF", fundingtxid) assert_equal(result['result'], 'error') @@ -224,13 +227,39 @@ class CryptoConditionsTest (BitcoinTestFramework): # confirm the xtn above rpc.generate(1) - result = rpc.rewardsunlock("STUFF", fundingtxid) - # currently failing because reward is not > txfee? - # assert_equal(result['result'], 'success') - + # amount must be positive result = rpc.rewardslock("STUFF", fundingtxid, "-5") assert_equal(result['result'], 'error') + # amount must be positive + result = rpc.rewardslock("STUFF", fundingtxid, "0") + assert_equal(result['result'], 'error') + + # trying to lock less than the min amount is an error + result = rpc.rewardslock("STUFF", fundingtxid, "7") + assert_equal(result['result'], 'error') + + # not working + #result = rpc.rewardslock("STUFF", fundingtxid, "10") + #assert_equal(result['result'], 'success') + #locktxid = result['hex'] + #assert locktxid, "got lock txid" + + # locktxid has not been broadcast yet + #result = rpc.rewardsunlock("STUFF", locktxid) + #assert_equal(result['result'], 'error') + + # broadcast xtn + #txid = rpc.sendrawtransaction(locktxid) + #assert txid, 'got txid from sendrawtransaction' + + # confirm the xtn above + #rpc.generate(1) + + #result = rpc.rewardsunlock("STUFF", locktxid) + #assert_equal(result['result'], 'error') + + def run_test (self): print("Mining blocks...") rpc = self.nodes[0] From 8e0ff2b732ad421148f71160d41e44e8f201d812 Mon Sep 17 00:00:00 2001 From: "Jonathan \"Duke\" Leto" Date: Wed, 15 Aug 2018 23:24:59 +0200 Subject: [PATCH 4/5] Test refactoring, more tests, and lots of error checking improvements and uint64_t fixes --- qa/rpc-tests/cryptoconditions.py | 26 +++-- src/cc/rewards.cpp | 44 ++++++--- src/wallet/rpcwallet.cpp | 165 ++++++++++++++++++++----------- 3 files changed, 158 insertions(+), 77 deletions(-) diff --git a/qa/rpc-tests/cryptoconditions.py b/qa/rpc-tests/cryptoconditions.py index ad7ee7467..8cfa4fd2a 100755 --- a/qa/rpc-tests/cryptoconditions.py +++ b/qa/rpc-tests/cryptoconditions.py @@ -46,11 +46,19 @@ class CryptoConditionsTest (BitcoinTestFramework): ]] ) self.is_network_split = split + self.rpc = self.nodes[0] self.sync_all() print("Done setting up network") + def send_and_mine(self, xtn): + txid = self.rpc.sendrawtransaction(xtn) + assert txid, 'got txid' + # we need the tx above to be confirmed in the next block + self.rpc.generate(1) + return txid + def run_faucet_tests(self): - rpc = self.nodes[0] + rpc = self.rpc # basic sanity tests result = rpc.getwalletinfo() @@ -90,9 +98,6 @@ class CryptoConditionsTest (BitcoinTestFramework): # we need the tx above to be confirmed in the next block rpc.generate(1) - # clear the rawmempool - result = rpc.getrawmempool() - result = rpc.getwalletinfo() balance2 = result['balance'] # make sure our balance is less now @@ -145,13 +150,18 @@ class CryptoConditionsTest (BitcoinTestFramework): for x in ['AssetsCCaddress', 'myCCaddress', 'Assetsmarker', 'myaddress', 'CCaddress']: assert_equal(result[x][0], 'R') - result = rpc.tokencreate("DUKE", "1987.420", "duke") - assert_equal(result['result'], 'success') - # there are no tokens created yet result = rpc.tokenlist() assert_equal(result, []) + result = rpc.tokencreate("DUKE", "1987.420", "duke") + assert_equal(result['result'], 'success') + self.send_and_mine(result['hex']) + + result = rpc.tokenlist() + tokenid = result[0] + assert(tokenid, "got tokenid") + # there are no token orders yet result = rpc.tokenorders() assert_equal(result, []) @@ -166,6 +176,7 @@ class CryptoConditionsTest (BitcoinTestFramework): result = rpc.tokeninfo(self.pubkey) assert_equal(result['result'], 'error') + def run_rewards_tests(self): rpc = self.nodes[0] result = rpc.rewardsaddress() @@ -272,7 +283,6 @@ class CryptoConditionsTest (BitcoinTestFramework): print("Importing privkey") rpc.importprivkey(self.privkey) - self.run_faucet_tests() self.run_rewards_tests() self.run_dice_tests() diff --git a/src/cc/rewards.cpp b/src/cc/rewards.cpp index 9314f7c94..9b58f4bf3 100644 --- a/src/cc/rewards.cpp +++ b/src/cc/rewards.cpp @@ -66,6 +66,8 @@ */ +extern std::string CCerror; + int64_t RewardsCalc(int64_t amount,uint256 txid,uint64_t APR,uint64_t minseconds,uint64_t maxseconds,uint64_t mindeposit) { int32_t numblocks; uint64_t duration,reward = 0; @@ -532,6 +534,7 @@ std::string RewardsUnlock(uint64_t txfee,char *planstr,uint256 fundingtxid,uint2 if ( RewardsPlanExists(cp,sbits,rewardspk,APR,minseconds,maxseconds,mindeposit) == 0 ) { fprintf(stderr,"Rewards plan %s doesnt exist\n",planstr); + CCerror = "Rewards plan does not exist"; return(""); } fprintf(stderr,"APR %.8f minseconds.%llu maxseconds.%llu mindeposit %.8f\n",(double)APR/COIN,(long long)minseconds,(long long)maxseconds,(double)mindeposit/COIN); @@ -543,6 +546,7 @@ std::string RewardsUnlock(uint64_t txfee,char *planstr,uint256 fundingtxid,uint2 if ( (amount= CCutxovalue(coinaddr,locktxid,0)) == 0 ) { fprintf(stderr,"%s locktxid/v0 is spent\n",coinaddr); + CCerror = "locktxid/v0 is spent"; return(""); } if ( GetTransaction(locktxid,tx,hashBlock,false) != 0 && tx.vout.size() > 0 && tx.vout[1].scriptPubKey.IsPayToCryptoCondition() == 0 ) @@ -553,21 +557,37 @@ std::string RewardsUnlock(uint64_t txfee,char *planstr,uint256 fundingtxid,uint2 else { fprintf(stderr,"%s no normal vout.1 in locktxid\n",coinaddr); + CCerror = "no normal vout.1 in locktxid"; return(""); } } - if ( amount > 0 && (reward= RewardsCalc(amount,mtx.vin[0].prevout.hash,APR,minseconds,maxseconds,mindeposit)) > txfee && scriptPubKey.size() > 0 ) - { - if ( (inputs= AddRewardsInputs(ignore,1,cp,mtx,rewardspk,reward+txfee,30,sbits,fundingtxid)) > 0 ) - { - if ( inputs >= (reward + 2*txfee) ) - CCchange = (inputs - (reward + txfee)); - fprintf(stderr,"inputs %.8f CCchange %.8f amount %.8f reward %.8f\n",(double)inputs/COIN,(double)CCchange/COIN,(double)amount/COIN,(double)reward/COIN); - mtx.vout.push_back(MakeCC1vout(cp->evalcode,CCchange,rewardspk)); - mtx.vout.push_back(CTxOut(amount+reward,scriptPubKey)); - return(FinalizeCCTx(-1LL,cp,mtx,mypk,txfee,EncodeRewardsOpRet('U',sbits,fundingtxid))); - } - fprintf(stderr,"cant find enough rewards inputs\n"); + if ( amount > 0 ) { + reward = RewardsCalc(amount,mtx.vin[0].prevout.hash,APR,minseconds,maxseconds,mindeposit); + if (scriptPubKey.size() > 0) { + if (reward > txfee) { + if ( (inputs= AddRewardsInputs(ignore,1,cp,mtx,rewardspk,reward+txfee,30,sbits,fundingtxid)) > 0 ) + { + if ( inputs >= (reward + 2*txfee) ) + CCchange = (inputs - (reward + txfee)); + fprintf(stderr,"inputs %.8f CCchange %.8f amount %.8f reward %.8f\n",(double)inputs/COIN,(double)CCchange/COIN,(double)amount/COIN,(double)reward/COIN); + mtx.vout.push_back(MakeCC1vout(cp->evalcode,CCchange,rewardspk)); + mtx.vout.push_back(CTxOut(amount+reward,scriptPubKey)); + return(FinalizeCCTx(-1LL,cp,mtx,mypk,txfee,EncodeRewardsOpRet('U',sbits,fundingtxid))); + } + CCerror = "cant find enough rewards inputs"; + fprintf(stderr,"%s\n", CCerror.c_str()); + } else { + // strprintf? + CCerror = strprintf("reward %.8f is <= the transaction fee", reward); + fprintf(stderr,"%s\n", CCerror.c_str()); + } + } else { + CCerror = "invalid scriptPubKey"; + fprintf(stderr,"%s\n", CCerror.c_str()); + } + } else { + CCerror = "amount must be positive"; + fprintf(stderr,"%s\n", CCerror.c_str()); } fprintf(stderr,"amount %.8f -> reward %.8f\n",(double)amount/COIN,(double)reward/COIN); return(""); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 1654be8ba..cbd08dd6c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -38,6 +38,9 @@ #include +#define ERR_RESULT(x) result.push_back(Pair("result", "error")); \ + result.push_back(Pair("error", x)); + using namespace std; using namespace libzcash; @@ -49,6 +52,8 @@ uint32_t komodo_segid32(char *coinaddr); int64_t nWalletUnlockTime; static CCriticalSection cs_nWalletUnlockTime; +//TODO: find a better place for this +std::string CCerror; // Private method: UniValue z_getoperationstatus_IMPL(const UniValue&, bool); @@ -5074,7 +5079,10 @@ UniValue rewardsunlock(const UniValue& params, bool fHelp) txid = Parseuint256((char *)params[2].get_str().c_str()); else memset(&txid,0,sizeof(txid)); hex = RewardsUnlock(0,name,fundingtxid,txid); - if ( hex.size() > 0 ) + if (CCerror != "") { + result.push_back(Pair("result", "error")); + result.push_back(Pair("error", CCerror)); + } else if ( hex.size() > 0 ) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); @@ -5194,8 +5202,7 @@ UniValue faucetget(const UniValue& params, bool fHelp) const CKeyStore& keystore = *pwalletMain; LOCK2(cs_main, pwalletMain->cs_wallet); hex = FaucetGet(0); - if ( hex.size() > 0 ) - { + if ( hex.size() > 0 ) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); } else { @@ -5221,17 +5228,22 @@ UniValue dicefund(const UniValue& params, bool fHelp) maxodds = atol(params[4].get_str().c_str()); timeoutblocks = atol(params[5].get_str().c_str()); hex = DiceCreateFunding(0,name,funds,minbet,maxbet,maxodds,timeoutblocks); - if ( hex.size() > 0 ) - { + if (CCerror != "") { + result.push_back(Pair("result", "error")); + result.push_back(Pair("error", CCerror)); + } else if ( hex.size() > 0 ) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create dice funding transaction")); + } else { + result.push_back(Pair("error", "couldnt create dice funding transaction")); + result.push_back(Pair("result", "error")); + } return(result); } UniValue diceaddfunds(const UniValue& params, bool fHelp) { - UniValue result(UniValue::VOBJ); char *name; uint256 fundingtxid; uint64_t amount; std::string hex; + UniValue result(UniValue::VOBJ); char *name; uint256 fundingtxid; int64_t amount; std::string hex; if ( fHelp || params.size() != 3 ) throw runtime_error("diceaddfunds name fundingtxid amount\n"); if ( ensure_CCrequirements() < 0 ) @@ -5241,18 +5253,28 @@ UniValue diceaddfunds(const UniValue& params, bool fHelp) name = (char *)params[0].get_str().c_str(); fundingtxid = Parseuint256((char *)params[1].get_str().c_str()); amount = atof(params[2].get_str().c_str()) * COIN; - hex = DiceAddfunding(0,name,fundingtxid,amount); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create dice addfunding transaction")); + if ( amount > 0 ) { + hex = DiceAddfunding(0,name,fundingtxid,amount); + if (CCerror != "") { + result.push_back(Pair("result", "error")); + result.push_back(Pair("error", CCerror)); + } else if ( hex.size() > 0 ) { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else { + result.push_back(Pair("result", "error")); + result.push_back(Pair("error", "couldnt create dice addfunding transaction")); + } + } else { + result.push_back(Pair("result", "error")); + result.push_back(Pair("error", "amount must be positive")); + } return(result); } UniValue dicebet(const UniValue& params, bool fHelp) { - UniValue result(UniValue::VOBJ); std::string hex; uint256 fundingtxid; uint64_t amount,odds; char *name; + UniValue result(UniValue::VOBJ); std::string hex; uint256 fundingtxid; int64_t amount,odds; char *name; if ( fHelp || params.size() != 4 ) throw runtime_error("dicebet name fundingtxid amount odds\n"); if ( ensure_CCrequirements() < 0 ) @@ -5263,18 +5285,24 @@ UniValue dicebet(const UniValue& params, bool fHelp) fundingtxid = Parseuint256((char *)params[1].get_str().c_str()); amount = atof(params[2].get_str().c_str()) * COIN; odds = atol(params[3].get_str().c_str()); - hex = DiceBet(0,name,fundingtxid,amount,odds); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create faucet get transaction")); + if (amount > 0 && odds > 0) { + hex = DiceBet(0,name,fundingtxid,amount,odds); + if ( hex.size() > 0 ) + { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else { + ERR_RESULT("couldnt create faucet get transaction"); + } + } else { + ERR_RESULT("amount and odds must be positive"); + } return(result); } UniValue dicefinish(const UniValue& params, bool fHelp) { - UniValue result(UniValue::VOBJ); char *name; uint256 fundingtxid,bettxid; uint64_t amount; std::string hex; int32_t r; + UniValue result(UniValue::VOBJ); char *name; uint256 fundingtxid,bettxid; std::string hex; int32_t r; if ( fHelp || params.size() != 3 ) throw runtime_error("dicefinish name fundingtxid bettxid\n"); if ( ensure_CCrequirements() < 0 ) @@ -5295,7 +5323,7 @@ UniValue dicefinish(const UniValue& params, bool fHelp) UniValue dicestatus(const UniValue& params, bool fHelp) { - UniValue result(UniValue::VOBJ); char *name; uint256 fundingtxid,bettxid; uint64_t amount; std::string status; double winnings; + UniValue result(UniValue::VOBJ); char *name; uint256 fundingtxid,bettxid; std::string status; double winnings; if ( fHelp || (params.size() != 2 && params.size() != 3) ) throw runtime_error("dicestatus name fundingtxid bettxid\n"); if ( ensure_CCrequirements() < 0 ) @@ -5435,7 +5463,7 @@ UniValue tokencreate(const UniValue& params, bool fHelp) UniValue tokentransfer(const UniValue& params, bool fHelp) { - UniValue result(UniValue::VOBJ); std::string hex; uint64_t amount; uint256 tokenid; + UniValue result(UniValue::VOBJ); std::string hex; int64_t amount; uint256 tokenid; if ( fHelp || params.size() != 3 ) throw runtime_error("tokentransfer tokenid destpubkey amount\n"); if ( ensure_CCrequirements() < 0 ) @@ -5446,17 +5474,21 @@ UniValue tokentransfer(const UniValue& params, bool fHelp) std::vector pubkey(ParseHex(params[1].get_str().c_str())); amount = atol(params[2].get_str().c_str()); hex = AssetTransfer(0,tokenid,pubkey,amount); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt transfer assets")); + if (amount > 0) { + if ( hex.size() > 0 ) + { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else result.push_back(Pair("error", "couldnt transfer assets")); + } else { + ERR_RESULT("amount must be positive"); + } return(result); } UniValue tokenbid(const UniValue& params, bool fHelp) { - UniValue result(UniValue::VOBJ); uint64_t bidamount,numtokens; std::string hex; double price; uint256 tokenid; + UniValue result(UniValue::VOBJ); int64_t bidamount,numtokens; std::string hex; double price; uint256 tokenid; if ( fHelp || params.size() != 3 ) throw runtime_error("tokenbid numtokens tokenid price\n"); if ( ensure_CCrequirements() < 0 ) @@ -5468,11 +5500,15 @@ UniValue tokenbid(const UniValue& params, bool fHelp) price = atof(params[2].get_str().c_str()); bidamount = (price * numtokens) * COIN + 0.0000000049999; hex = CreateBuyOffer(0,bidamount,tokenid,numtokens); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create bid")); + if (price > 0 && numtokens > 0) { + if ( hex.size() > 0 ) + { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else result.push_back(Pair("error", "couldnt create bid")); + } else { + ERR_RESULT("price and numtokens must be positive"); + } return(result); } @@ -5519,7 +5555,7 @@ UniValue tokenfillbid(const UniValue& params, bool fHelp) UniValue tokenask(const UniValue& params, bool fHelp) { - UniValue result(UniValue::VOBJ); uint64_t askamount,numtokens; std::string hex; double price; uint256 tokenid; + UniValue result(UniValue::VOBJ); int64_t askamount,numtokens; std::string hex; double price; uint256 tokenid; if ( fHelp || params.size() != 3 ) throw runtime_error("tokenask numtokens tokenid price\n"); if ( ensure_CCrequirements() < 0 ) @@ -5531,18 +5567,22 @@ UniValue tokenask(const UniValue& params, bool fHelp) price = atof(params[2].get_str().c_str()); askamount = (price * numtokens) * COIN + 0.0000000049999; hex = CreateSell(0,numtokens,tokenid,askamount); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create ask")); + if (price > 0 && numtokens > 0) { + if ( hex.size() > 0 ) + { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else result.push_back(Pair("error", "couldnt create ask")); + } else { + ERR_RESULT("price and numtokens must be positive"); + } return(result); } UniValue tokenswapask(const UniValue& params, bool fHelp) { static uint256 zeroid; - UniValue result(UniValue::VOBJ); uint64_t askamount,numtokens; std::string hex; double price; uint256 tokenid,otherid; + UniValue result(UniValue::VOBJ); int64_t askamount,numtokens; std::string hex; double price; uint256 tokenid,otherid; if ( fHelp || params.size() != 4 ) throw runtime_error("tokenswapask numtokens tokenid otherid price\n"); if ( ensure_CCrequirements() < 0 ) @@ -5555,11 +5595,15 @@ UniValue tokenswapask(const UniValue& params, bool fHelp) price = atof(params[3].get_str().c_str()); askamount = (price * numtokens); hex = CreateSwap(0,numtokens,tokenid,otherid,askamount); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create swap")); + if (price > 0 && numtokens > 0) { + if ( hex.size() > 0 ) + { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else result.push_back(Pair("error", "couldnt create swap")); + } else { + ERR_RESULT("price and numtokens must be positive"); + } return(result); } @@ -5597,18 +5641,22 @@ UniValue tokenfillask(const UniValue& params, bool fHelp) asktxid = Parseuint256((char *)params[1].get_str().c_str()); fillunits = atol(params[2].get_str().c_str()); hex = FillSell(0,tokenid,zeroid,asktxid,fillunits); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt fill bid")); + if (fillunits > 0) { + if ( hex.size() > 0) + { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else ERR_RESULT("couldnt fill bid"); + } else { + ERR_RESULT("fillunits must be positive"); + } return(result); } UniValue tokenfillswap(const UniValue& params, bool fHelp) { static uint256 zeroid; - UniValue result(UniValue::VOBJ); uint64_t fillunits; std::string hex; uint256 tokenid,otherid,asktxid; + UniValue result(UniValue::VOBJ); int64_t fillunits; std::string hex; uint256 tokenid,otherid,asktxid; if ( fHelp || params.size() != 4 ) throw runtime_error("tokenfillswap tokenid otherid asktxid fillunits\n"); if ( ensure_CCrequirements() < 0 ) @@ -5620,11 +5668,14 @@ UniValue tokenfillswap(const UniValue& params, bool fHelp) asktxid = Parseuint256((char *)params[2].get_str().c_str()); fillunits = atol(params[3].get_str().c_str()); hex = FillSell(0,tokenid,otherid,asktxid,fillunits); - if ( hex.size() > 0 ) - { - result.push_back(Pair("result", "success")); - result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt fill bid")); + if (fillunits > 0) { + if ( hex.size() > 0 ) { + result.push_back(Pair("result", "success")); + result.push_back(Pair("hex", hex)); + } else ERR_RESULT("couldnt fill bid"); + } else { + ERR_RESULT("fillunits must be positive"); + } return(result); } From 8a3e1884d6a97dccffd3e90ffa8d288152485005 Mon Sep 17 00:00:00 2001 From: "Jonathan \"Duke\" Leto" Date: Thu, 16 Aug 2018 17:17:54 +0200 Subject: [PATCH 5/5] More token tests and more error checking and better errors from internals --- qa/rpc-tests/cryptoconditions.py | 50 ++++++++++++++++-- src/cc/CCassetstx.cpp | 12 +++-- src/wallet/rpcwallet.cpp | 90 +++++++++++--------------------- 3 files changed, 86 insertions(+), 66 deletions(-) diff --git a/qa/rpc-tests/cryptoconditions.py b/qa/rpc-tests/cryptoconditions.py index 8cfa4fd2a..68b381272 100755 --- a/qa/rpc-tests/cryptoconditions.py +++ b/qa/rpc-tests/cryptoconditions.py @@ -12,6 +12,12 @@ from test_framework.util import assert_equal, assert_greater_than, \ import time from decimal import Decimal +def assert_success(result): + assert_equal(result['result'], 'success') + +def assert_error(result): + assert_equal(result['result'], 'error') + class CryptoConditionsTest (BitcoinTestFramework): def setup_chain(self): @@ -148,8 +154,7 @@ class CryptoConditionsTest (BitcoinTestFramework): result = rpc.tokenaddress(self.pubkey) assert_equal(result['result'], 'success') for x in ['AssetsCCaddress', 'myCCaddress', 'Assetsmarker', 'myaddress', 'CCaddress']: - assert_equal(result[x][0], 'R') - + assert_equal(result[x][0], 'R') # there are no tokens created yet result = rpc.tokenlist() assert_equal(result, []) @@ -174,8 +179,47 @@ class CryptoConditionsTest (BitcoinTestFramework): # this is not a valid assetid result = rpc.tokeninfo(self.pubkey) - assert_equal(result['result'], 'error') + assert_error(result) + # invalid numtokens + result = rpc.tokenask("-1", tokenid, "1") + assert_error(result) + + # invalid numtokens + result = rpc.tokenask("0", tokenid, "1") + assert_error(result) + + # invalid price + result = rpc.tokenask("1", tokenid, "-1") + assert_error(result) + + # invalid price + result = rpc.tokenask("1", tokenid, "0") + assert_error(result) + + # invalid tokenid + result = rpc.tokenask("100", "deadbeef", "1") + assert_error(result) + + # valid + result = rpc.tokenask("100", tokenid, "7.77") + assert_success(result) + tokenaskhex = result['hex'] + assert tokenaskhex, "got tokenask hexk" + tokenaskid = self.send_and_mine(result['hex']) + + + # invalid fillunits + result = rpc.tokenfillask(tokenid, tokenaskid, "0") + assert_error(result) + + # invalid fillunits + result = rpc.tokenfillask(tokenid, tokenaskid, "-777") + assert_error(result) + + # should this pass or fail? + result = rpc.tokenfillask(tokenid, tokenaskid, "10") + #assert_success(result) def run_rewards_tests(self): rpc = self.nodes[0] diff --git a/src/cc/CCassetstx.cpp b/src/cc/CCassetstx.cpp index bfd51f4b7..40d76370c 100644 --- a/src/cc/CCassetstx.cpp +++ b/src/cc/CCassetstx.cpp @@ -14,6 +14,7 @@ ******************************************************************************/ #include "CCassets.h" +extern std::string CCerror; int64_t AddAssetInputs(struct CCcontract_info *cp,CMutableTransaction &mtx,CPubKey pk,uint256 assetid,int64_t total,int32_t maxinputs) { @@ -427,12 +428,14 @@ std::string FillSell(int64_t txfee,uint256 assetid,uint256 assetid2,uint256 askt CTransaction vintx,filltx; uint256 hashBlock; CMutableTransaction mtx; CPubKey mypk; std::vector origpubkey; double dprice; uint64_t mask; int32_t askvout=0; int64_t received_assetoshis,total_nValue,orig_assetoshis,paid_nValue,remaining_nValue,inputs,CCchange=0; struct CCcontract_info *cp,C; if ( fillunits < 0 ) { - fprintf(stderr,"negative fillunits %lld\n",(long long)fillunits); + CCerror = strprintf("negative fillunits %lld\n",(long long)fillunits); + fprintf(stderr,"%s\n",CCerror.c_str()); return(""); } if ( assetid2 != zeroid ) { - fprintf(stderr,"asset swaps disabled\n"); + CCerror = "asset swaps disabled"; + fprintf(stderr,"%s\n",CCerror.c_str()); return(""); } @@ -474,7 +477,10 @@ std::string FillSell(int64_t txfee,uint256 assetid,uint256 assetid2,uint256 askt if ( CCchange != 0 ) mtx.vout.push_back(MakeCC1vout(EVAL_ASSETS,CCchange,mypk)); return(FinalizeCCTx(mask,cp,mtx,mypk,txfee,EncodeAssetOpRet(assetid2!=zeroid?'E':'S',assetid,assetid2,remaining_nValue,origpubkey))); - } else fprintf(stderr,"filltx not enough utxos\n"); + } else { + CCerror = strprintf("filltx not enough utxos"); + fprintf(stderr,"%s\n", CCerror.c_str()); + } } } return(""); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index cbd08dd6c..0775842a2 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -38,8 +38,7 @@ #include -#define ERR_RESULT(x) result.push_back(Pair("result", "error")); \ - result.push_back(Pair("error", x)); +#define ERR_RESULT(x) result.push_back(Pair("result", "error")) , result.push_back(Pair("error", x)); using namespace std; @@ -52,7 +51,6 @@ uint32_t komodo_segid32(char *coinaddr); int64_t nWalletUnlockTime; static CCriticalSection cs_nWalletUnlockTime; -//TODO: find a better place for this std::string CCerror; // Private method: @@ -5002,7 +5000,7 @@ UniValue rewardscreatefunding(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create rewards funding transaction")); + } else ERR_RESULT("couldnt create rewards funding transaction"); return(result); } @@ -5024,14 +5022,8 @@ UniValue rewardslock(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", "couldnt create rewards lock transaction")); - } - } else { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", "amount must be positive")); - } + } else ERR_RESULT( "couldnt create rewards lock transaction"); + } else ERR_RESULT("amount must be positive"); return(result); } @@ -5080,16 +5072,11 @@ UniValue rewardsunlock(const UniValue& params, bool fHelp) else memset(&txid,0,sizeof(txid)); hex = RewardsUnlock(0,name,fundingtxid,txid); if (CCerror != "") { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", CCerror)); - } else if ( hex.size() > 0 ) - { + ERR_RESULT(CCerror); + } else if ( hex.size() > 0 ) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", "couldnt create rewards unlock transaction")); - } + } else ERR_RESULT("couldnt create rewards unlock transaction"); return(result); } @@ -5181,14 +5168,8 @@ UniValue faucetfund(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", "couldnt create faucet funding transaction")); - } - } else { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", "funding amount must be positive")); - } + } else ERR_RESULT("couldnt create faucet funding transaction"); + } else ERR_RESULT( "funding amount must be positive"); return(result); } @@ -5205,10 +5186,7 @@ UniValue faucetget(const UniValue& params, bool fHelp) if ( hex.size() > 0 ) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", "couldnt create faucet get transaction")); - } + } else ERR_RESULT("couldnt create faucet get transaction"); return(result); } @@ -5229,14 +5207,12 @@ UniValue dicefund(const UniValue& params, bool fHelp) timeoutblocks = atol(params[5].get_str().c_str()); hex = DiceCreateFunding(0,name,funds,minbet,maxbet,maxodds,timeoutblocks); if (CCerror != "") { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", CCerror)); + ERR_RESULT(CCerror); } else if ( hex.size() > 0 ) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); } else { - result.push_back(Pair("error", "couldnt create dice funding transaction")); - result.push_back(Pair("result", "error")); + ERR_RESULT( "couldnt create dice funding transaction"); } return(result); } @@ -5256,19 +5232,12 @@ UniValue diceaddfunds(const UniValue& params, bool fHelp) if ( amount > 0 ) { hex = DiceAddfunding(0,name,fundingtxid,amount); if (CCerror != "") { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", CCerror)); + ERR_RESULT(CCerror); } else if ( hex.size() > 0 ) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", "couldnt create dice addfunding transaction")); - } - } else { - result.push_back(Pair("result", "error")); - result.push_back(Pair("error", "amount must be positive")); - } + } else ERR_RESULT("couldnt create dice addfunding transaction"); + } else ERR_RESULT("amount must be positive"); return(result); } @@ -5291,9 +5260,7 @@ UniValue dicebet(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else { - ERR_RESULT("couldnt create faucet get transaction"); - } + } else ERR_RESULT("couldnt create faucet get transaction"); } else { ERR_RESULT("amount and odds must be positive"); } @@ -5317,7 +5284,7 @@ UniValue dicefinish(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create dicefinish transaction")); + } else ERR_RESULT( "couldnt create dicefinish transaction"); return(result); } @@ -5457,7 +5424,7 @@ UniValue tokencreate(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create transaction")); + } else ERR_RESULT("couldnt create transaction"); return(result); } @@ -5479,7 +5446,7 @@ UniValue tokentransfer(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt transfer assets")); + } else ERR_RESULT("couldnt transfer assets"); } else { ERR_RESULT("amount must be positive"); } @@ -5528,7 +5495,7 @@ UniValue tokencancelbid(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt cancel bid")); + } else ERR_RESULT("couldnt cancel bid"); return(result); } @@ -5549,7 +5516,7 @@ UniValue tokenfillbid(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt fill bid")); + } else ERR_RESULT("couldnt fill bid"); return(result); } @@ -5572,7 +5539,7 @@ UniValue tokenask(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create ask")); + } else ERR_RESULT("couldnt create ask"); } else { ERR_RESULT("price and numtokens must be positive"); } @@ -5600,7 +5567,7 @@ UniValue tokenswapask(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt create swap")); + } else ERR_RESULT("couldnt create swap"); } else { ERR_RESULT("price and numtokens must be positive"); } @@ -5623,7 +5590,7 @@ UniValue tokencancelask(const UniValue& params, bool fHelp) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else result.push_back(Pair("error", "couldnt cancel bid")); + } else ERR_RESULT("couldnt cancel bid"); return(result); } @@ -5642,11 +5609,14 @@ UniValue tokenfillask(const UniValue& params, bool fHelp) fillunits = atol(params[2].get_str().c_str()); hex = FillSell(0,tokenid,zeroid,asktxid,fillunits); if (fillunits > 0) { - if ( hex.size() > 0) - { + if (CCerror != "") { + ERR_RESULT(CCerror); + } else if ( hex.size() > 0) { result.push_back(Pair("result", "success")); result.push_back(Pair("hex", hex)); - } else ERR_RESULT("couldnt fill bid"); + } else { + ERR_RESULT("couldnt fill bid"); + } } else { ERR_RESULT("fillunits must be positive"); }