Skip to content

Commit

Permalink
Collateral issues / Exact denoms / Disabled denoms
Browse files Browse the repository at this point in the history 
- When attempting to connect to a masternode for submission into the pool a recursive call to DoAutoDenominate was used. This could possibly take more than 1 minute to complete if it found a string of bad masternodes, in which case the correct masternode was overwritten and replaced with an invalid one. Upon submission, the DS TX was given to the incorrect node causing collateral to be charged.
- To fix this I've removed the recursion and added a critical section to DoAutoDenominate.
- Exact input denominations are matched in PrepareDarksendDenominate to remove the possibility of having change in the pool
- Removed disabled denominations, not needed anymore
  • Loading branch information
Evan Duffield committed Jan 20, 2015
1 parent 3322b02 commit bbd8695
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 87 deletions.
71 changes: 27 additions & 44 deletions src/darksend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using namespace std;
using namespace boost;

CCriticalSection cs_darksend;

/** The main object for accessing darksend */
CDarkSendPool darkSendPool;
Expand Down Expand Up @@ -461,7 +462,6 @@ void CDarkSendPool::SetNull(bool clearEverything){
sessionUsers = 0;
sessionDenom = 0;
sessionFoundMasternode = false;
sessionTries = 0;
vecSessionCollateral.clear();
txCollateral = CTransaction();

Expand Down Expand Up @@ -864,7 +864,6 @@ void CDarkSendPool::CheckTimeout(){
sessionUsers = 0;
sessionDenom = 0;
sessionFoundMasternode = false;
sessionTries = 0;
vecSessionCollateral.clear();

UpdateState(POOL_STATUS_ACCEPTING_ENTRIES);
Expand Down Expand Up @@ -1410,6 +1409,8 @@ void CDarkSendPool::ClearLastMessage()
//
bool CDarkSendPool::DoAutomaticDenominating(bool fDryRun, bool ready)
{
LOCK(cs_darksend);

if(fMasterNode) return false;
if(state == POOL_STATUS_ERROR || state == POOL_STATUS_SUCCESS) return false;

Expand Down Expand Up @@ -1443,6 +1444,7 @@ bool CDarkSendPool::DoAutomaticDenominating(bool fDryRun, bool ready)

// ** find the coins we'll use
std::vector<CTxIn> vCoins;
std::vector<COutput> vCoins2;
int64_t nValueMin = 0.01*COIN;
int64_t nValueIn = 0;
int64_t lowestDenom = COIN*0.1;
Expand All @@ -1456,8 +1458,7 @@ bool CDarkSendPool::DoAutomaticDenominating(bool fDryRun, bool ready)
int64_t nonanonymized = pwalletMain->GetBalance() - pwalletMain->GetAnonymizedBalance() - COIN;
if(balanceNeedsAnonymized > nonanonymized) balanceNeedsAnonymized = nonanonymized;

if(balanceNeedsAnonymized < lowestDenom ||
(vecDisabledDenominations.size() > 0 && balanceNeedsAnonymized < COIN*10)){
if(balanceNeedsAnonymized < lowestDenom){
LogPrintf("DoAutomaticDenominating : No funds detected in need of denominating \n");
strAutoDenomResult = "No funds detected in need of denominating";
return false;
Expand Down Expand Up @@ -1489,15 +1490,6 @@ bool CDarkSendPool::DoAutomaticDenominating(bool fDryRun, bool ready)

if(fDryRun) return true;

if(vecDisabledDenominations.size() == 0){
//if we have 20x 0.1DRk and 1DRK inputs, we can start just anonymizing 10DRK inputs.
if(pwalletMain->CountInputsWithAmount((1 * COIN)+1) >= 20 &&
pwalletMain->CountInputsWithAmount((.1 * COIN)+1) >= 20){
vecDisabledDenominations.push_back((1 * COIN)+1);
vecDisabledDenominations.push_back((.1 * COIN)+1);
}
}

// initial phase, find a masternode
if(!sessionFoundMasternode){
int nUseQueue = rand()%100;
Expand Down Expand Up @@ -1542,11 +1534,9 @@ bool CDarkSendPool::DoAutomaticDenominating(bool fDryRun, bool ready)
}

// Try to match their denominations if possible
if (!pwalletMain->SelectCoinsByDenominations(dsq.nDenom, nValueMin, balanceNeedsAnonymized, vCoins, nValueIn, 0, nDarksendRounds)){
// if (!pwalletMain->SelectCoinsByDenominations(dsq.nDenom, nValueMin, balanceNeedsAnonymized, vCoins, nValueIn, -2, 0)){
LogPrintf("DoAutomaticDenominating - Couldn't match denominations %d\n", dsq.nDenom);
continue;
// }
if (!pwalletMain->SelectCoinsByDenominations(dsq.nDenom, nValueMin, balanceNeedsAnonymized, vCoins, vCoins2, nValueIn, 0, nDarksendRounds)){
LogPrintf("DoAutomaticDenominating - Couldn't match denominations %d\n", dsq.nDenom);
continue;
}

// connect to masternode and submit the queue request
Expand Down Expand Up @@ -1584,30 +1574,33 @@ bool CDarkSendPool::DoAutomaticDenominating(bool fDryRun, bool ready)
}
}

// otherwise, try one randomly
if(sessionTries++ < 10){
//pick a random masternode to use
int max_value = vecMasternodes.size();
if(max_value <= 0) return false;
int i = (rand() % max_value);
//shuffle masternodes around before we try to connect
std::random_shuffle ( vecMasternodes.begin(), vecMasternodes.end() );
int i = 0;

// otherwise, try one randomly
while(i < 10)
{
//don't reuse masternodes
BOOST_FOREACH(CTxIn usedVin, vecMasternodesUsed) {
if(vecMasternodes[i].vin == usedVin){
return DoAutomaticDenominating();
i++;
continue;
}
}
if(vecMasternodes[i].protocolVersion < MIN_PEER_PROTO_VERSION) {
return DoAutomaticDenominating();
i++;
continue;
}

if(vecMasternodes[i].nLastDsq != 0 &&
vecMasternodes[i].nLastDsq + CountMasternodesAboveProtocol(darkSendPool.MIN_PEER_PROTO_VERSION)/5 > darkSendPool.nDsqCount){
return DoAutomaticDenominating();
i++;
continue;
}

lastTimeChanged = GetTimeMillis();
LogPrintf("DoAutomaticDenominating -- attempt %d connection to masternode %s\n", sessionTries, vecMasternodes[i].addr.ToString().c_str());
LogPrintf("DoAutomaticDenominating -- attempt %d connection to masternode %s\n", i, vecMasternodes[i].addr.ToString().c_str());
if(ConnectNode((CAddress)vecMasternodes[i].addr, NULL, true)){
submittedToMasternode = vecMasternodes[i].addr;

Expand All @@ -1619,7 +1612,7 @@ bool CDarkSendPool::DoAutomaticDenominating(bool fDryRun, bool ready)
std::string strReason;
if(txCollateral == CTransaction()){
if(!pwalletMain->CreateCollateralTransaction(txCollateral, strReason)){
LogPrintf("DoAutomaticDenominating -- dsa error:%s\n", strReason.c_str());
LogPrintf("DoAutomaticDenominating -- create collateral error:%s\n", strReason.c_str());
return false;
}
}
Expand All @@ -1636,13 +1629,13 @@ bool CDarkSendPool::DoAutomaticDenominating(bool fDryRun, bool ready)
return true;
}
} else {
LogPrintf("DoAutomaticDenominating --- error connecting \n");
return DoAutomaticDenominating();
i++;
continue;
}
} else {
strAutoDenomResult = "No compatible masternode found";
return false;
}

strAutoDenomResult = "No compatible masternode found";
return false;
}

strAutoDenomResult = "";
Expand Down Expand Up @@ -1820,15 +1813,8 @@ bool CDarkSendPool::CreateDenominated(int64_t nTotalValue)

// ****** Add denoms ************ /
BOOST_REVERSE_FOREACH(int64_t v, darkSendDenominations){

int nOutputs = 0;

if(std::find(vecDisabledDenominations.begin(),
vecDisabledDenominations.end(), v) !=
vecDisabledDenominations.end()){
continue;
}

// add each output up to 10 times until it can't be added again
while(nValueLeft - v >= 0 && nOutputs <= 10) {
CScript scriptChange;
Expand Down Expand Up @@ -2055,9 +2041,6 @@ int CDarkSendPool::GetDenominationsByAmount(int64_t nAmount, int nDenomTarget){

int nOutputs = 0;

if(std::find(vecDisabledDenominations.begin(), vecDisabledDenominations.end(), v) != vecDisabledDenominations.end())
continue;

// add each output up to 10 times until it can't be added again
while(nValueLeft - v >= 0 && nOutputs <= 10) {
CTxOut o(v, e);
Expand Down
4 changes: 0 additions & 4 deletions src/darksend.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@ class CDarkSendPool
int sessionDenom; //Users must submit an denom matching this
int sessionUsers; //N Users have said they'll join
bool sessionFoundMasternode; //If we've found a compatible masternode
int sessionTries;
int64_t sessionTotalValue; //used for autoDenom
std::vector<CTransaction> vecSessionCollateral;

Expand All @@ -270,8 +269,6 @@ class CDarkSendPool
//debugging data
std::string strAutoDenomResult;

std::vector<int64_t> vecDisabledDenominations;

//incremented whenever a DSQ comes through
int64_t nDsqCount;

Expand All @@ -288,7 +285,6 @@ class CDarkSendPool
txCollateral = CTransaction();
minBlockSpacing = 1;
nDsqCount = 0;
vecDisabledDenominations.clear();

SetNull();
}
Expand Down
56 changes: 18 additions & 38 deletions src/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1505,18 +1505,17 @@ struct CompareByPriority
}
};

bool CWallet::SelectCoinsByDenominations(int nDenom, int64_t nValueMin, int64_t nValueMax, std::vector<CTxIn>& setCoinsRet, int64_t& nValueRet, int nDarksendRoundsMin, int nDarksendRoundsMax)
bool CWallet::SelectCoinsByDenominations(int nDenom, int64_t nValueMin, int64_t nValueMax, std::vector<CTxIn>& setCoinsRet, vector<COutput>& setCoinsRet2, int64_t& nValueRet, int nDarksendRoundsMin, int nDarksendRoundsMax)
{
CCoinControl *coinControl=NULL;

setCoinsRet.clear();
nValueRet = 0;

setCoinsRet2.clear();
vector<COutput> vCoins;
AvailableCoins(vCoins, false, coinControl, ALL_COINS);

set<pair<const CWalletTx*,unsigned int> > setCoinsRet2;

//order the array so fees are first, then denominated money, then the rest.
std::random_shuffle(vCoins.rbegin(), vCoins.rend());

Expand Down Expand Up @@ -1579,7 +1578,7 @@ bool CWallet::SelectCoinsByDenominations(int nDenom, int64_t nValueMin, int64_t
vin.prevPubKey = out.tx->vout[out.i].scriptPubKey; // the inputs PubKey
nValueRet += out.tx->vout[out.i].nValue;
setCoinsRet.push_back(vin);
setCoinsRet2.insert(make_pair(out.tx, out.i));
setCoinsRet2.push_back(out);
}
}

Expand Down Expand Up @@ -2112,6 +2111,7 @@ string CWallet::PrepareDarksendDenominate(int minRounds, int maxRounds)

// ** find the coins we'll use
std::vector<CTxIn> vCoins;
std::vector<COutput> vCoins2;
int64_t nValueIn = 0;
CReserveKey reservekey(this);

Expand All @@ -2121,7 +2121,7 @@ string CWallet::PrepareDarksendDenominate(int minRounds, int maxRounds)
if minRounds >= 0 it means only denominated inputs are going in and coming out
*/
if(minRounds >= 0){
if (!SelectCoinsByDenominations(darkSendPool.sessionDenom, 0.1*COIN, DARKSEND_POOL_MAX, vCoins, nValueIn, minRounds, maxRounds))
if (!SelectCoinsByDenominations(darkSendPool.sessionDenom, 0.1*COIN, DARKSEND_POOL_MAX, vCoins, vCoins2, nValueIn, minRounds, maxRounds))
return _("Insufficient funds");
}

Expand All @@ -2138,49 +2138,29 @@ string CWallet::PrepareDarksendDenominate(int minRounds, int maxRounds)
std::vector<CTxOut> vOut;

// Make outputs by looping through denominations, from small to large
BOOST_REVERSE_FOREACH(int64_t v, darkSendDenominations){
bool fAccepted = false;
if((darkSendPool.sessionDenom & (1 << 0)) && v == ((100*COIN)+1)) {fAccepted = true;}
else if((darkSendPool.sessionDenom & (1 << 1)) && v == ((10*COIN)+1)) {fAccepted = true;}
else if((darkSendPool.sessionDenom & (1 << 2)) && v == ((1*COIN)+1)) {fAccepted = true;}
else if((darkSendPool.sessionDenom & (1 << 3)) && v == ((.1*COIN)+1)) {fAccepted = true;}
if(!fAccepted) continue;

int nOutputs = 0;

// add each output up to 10 times until it can't be added again
while(nValueLeft - v >= 0 && nOutputs <= 10) {
CScript scriptChange;
CPubKey vchPubKey;
//use a unique change address
assert(reservekey.GetReservedKey(vchPubKey)); // should never fail, as we just unlocked
scriptChange.SetDestination(vchPubKey.GetID());
reservekey.KeepKey();

CTxOut o(v, scriptChange);
vOut.push_back(o);

//increment outputs and subtract denomination amount
nOutputs++;
nValueLeft -= v;
}

if(nValueLeft == 0) break;
}

// if we have anything left over, send it back as change

if(nValueLeft > 0){
BOOST_FOREACH(const COutput& out, vCoins2){
CScript scriptChange;
CPubKey vchPubKey;
//use a unique change address
assert(reservekey.GetReservedKey(vchPubKey)); // should never fail, as we just unlocked
scriptChange.SetDestination(vchPubKey.GetID());
reservekey.KeepKey();

CTxOut o(nValueLeft, scriptChange);
CTxOut o(out.tx->vout[out.i].nValue, scriptChange);
vOut.push_back(o);

//increment outputs and subtract denomination amount
nValueLeft -= out.tx->vout[out.i].nValue;

if(nValueLeft == 0) break;
}

// if we have anything left over, send it back as change

if(nValueLeft != 0)
return "Error: change left-over in pool. Must use denominations only";

darkSendPool.SendDarksendDenominate(vCoins, vOut, nValueIn);

return "";
Expand Down
2 changes: 1 addition & 1 deletion src/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class CWallet : public CCryptoKeyStore, public CWalletInterface
public:
bool SelectCoins(int64_t nTargetValue, std::set<std::pair<const CWalletTx*,unsigned int> >& setCoinsRet, int64_t& nValueRet, const CCoinControl *coinControl = NULL, AvailableCoinsType coin_type=ALL_COINS) const;
bool SelectCoinsDark(int64_t nValueMin, int64_t nValueMax, std::vector<CTxIn>& setCoinsRet, int64_t& nValueRet, int nDarksendRoundsMin, int nDarksendRoundsMax) const;
bool SelectCoinsByDenominations(int nDenom, int64_t nValueMin, int64_t nValueMax, std::vector<CTxIn>& setCoinsRet, int64_t& nValueRet, int nDarksendRoundsMin, int nDarksendRoundsMax);
bool SelectCoinsByDenominations(int nDenom, int64_t nValueMin, int64_t nValueMax, std::vector<CTxIn>& setCoinsRet, vector<COutput>& vCoins, int64_t& nValueRet, int nDarksendRoundsMin, int nDarksendRoundsMax);
bool SelectCoinsDarkDenominated(int64_t nTargetValue, std::vector<CTxIn>& setCoinsRet, int64_t& nValueRet) const;
bool SelectCoinsMasternode(CTxIn& vin, int64_t& nValueRet, CScript& pubScript) const;
bool HasDarksendFeeInputs() const;
Expand Down

0 comments on commit bbd8695

Please sign in to comment.