diff --git a/src/cpp/ripple/LedgerFormats.h b/src/cpp/ripple/LedgerFormats.h index 90ab9cf6b..3bc0afb50 100644 --- a/src/cpp/ripple/LedgerFormats.h +++ b/src/cpp/ripple/LedgerFormats.h @@ -37,10 +37,14 @@ enum LedgerNameSpace enum LedgerSpecificFlags { // ltACCOUNT_ROOT - lsfPasswordSpent = 0x00010000, // True if password set fee is spent. + lsfPasswordSpent = 0x00010000, // True, if password set fee is spent. // ltOFFER lsfPassive = 0x00010000, + + // ltRIPPLE_STATE + lsfLowReserve = 0x00010000, // True, if entry counts toward reserve. + lsfHighReserve = 0x00020000, }; class LedgerEntryFormat diff --git a/src/cpp/ripple/TransactionErr.cpp b/src/cpp/ripple/TransactionErr.cpp index a28ab5bc8..a36107e3b 100644 --- a/src/cpp/ripple/TransactionErr.cpp +++ b/src/cpp/ripple/TransactionErr.cpp @@ -50,11 +50,13 @@ bool transResultInfo(TER terCode, std::string& strToken, std::string& strHuman) { terDIR_FULL, "terDIR_FULL", "Can not add entry to full dir." }, { terFUNDS_SPENT, "terFUNDS_SPENT", "Can't set password, password set funds already spent." }, { terINSUF_FEE_B, "terINSUF_FEE_B", "Account balance can't pay fee." }, + { terINSUF_RESERVE, "terINSUF_RESERVE", "Insufficent reserve to add trust line." }, { terNO_ACCOUNT, "terNO_ACCOUNT", "The source account does not exist." }, { terNO_DST, "terNO_DST", "Destination does not exist. Send XRP to create it." }, { terNO_DST_INSUF_XRP, "terNO_DST_INSUF_XRP", "Destination does not exist. Too little XRP sent to create it." }, { terNO_LINE, "terNO_LINE", "No such line." }, - { terNO_LINE_NO_ZERO, "terNO_LINE_NO_ZERO", "Can't zero non-existant line, destination might make it." }, + { terNO_LINE_INSUF_RESERVE, "terNO_LINE_INSUF_RESERVE", "No such line. Too little reserve to create it." }, + { terNO_LINE_REDUNDANT, "terNO_LINE_REDUNDANT", "Can't set non-existant line to default." }, { terPRE_SEQ, "terPRE_SEQ", "Missing/inapplicable prior transaction." }, { terSET_MISSING_DST, "terSET_MISSING_DST", "Can't set password, destination missing." }, { terUNFUNDED, "terUNFUNDED", "Source account had insufficient balance for transaction." }, diff --git a/src/cpp/ripple/TransactionErr.h b/src/cpp/ripple/TransactionErr.h index 8292dd3aa..0a17405ec 100644 --- a/src/cpp/ripple/TransactionErr.h +++ b/src/cpp/ripple/TransactionErr.h @@ -80,11 +80,13 @@ enum TER // aka TransactionEngineResult terDIR_FULL, terFUNDS_SPENT, terINSUF_FEE_B, + terINSUF_RESERVE, terNO_ACCOUNT, terNO_DST, terNO_DST_INSUF_XRP, terNO_LINE, - terNO_LINE_NO_ZERO, + terNO_LINE_INSUF_RESERVE, + terNO_LINE_REDUNDANT, terPRE_SEQ, terSET_MISSING_DST, terUNFUNDED, diff --git a/src/cpp/ripple/TransactionFormats.h b/src/cpp/ripple/TransactionFormats.h index a67eb05c8..a9cab9ba8 100644 --- a/src/cpp/ripple/TransactionFormats.h +++ b/src/cpp/ripple/TransactionFormats.h @@ -60,7 +60,7 @@ const int TransactionMaxLen = 1048576; const uint32 tfPassive = 0x00010000; const uint32 tfOfferCreateMask = ~(tfPassive); -// Payment flags: +// Payment flags: (renumber on ledger wipe) const uint32 tfPaymentLegacy = 0x00010000; // Left here to avoid ledger change. const uint32 tfPartialPayment = 0x00020000; const uint32 tfLimitQuality = 0x00040000; diff --git a/src/cpp/ripple/TrustSetTransactor.cpp b/src/cpp/ripple/TrustSetTransactor.cpp index 54e1efc12..a78697292 100644 --- a/src/cpp/ripple/TrustSetTransactor.cpp +++ b/src/cpp/ripple/TrustSetTransactor.cpp @@ -1,3 +1,5 @@ +#include "Application.h" + #include "TrustSetTransactor.h" #include @@ -9,13 +11,19 @@ TER TrustSetTransactor::doApply() const STAmount saLimitAmount = mTxn.getFieldAmount(sfLimitAmount); const bool bQualityIn = mTxn.isFieldPresent(sfQualityIn); - const uint32 uQualityIn = bQualityIn ? mTxn.getFieldU32(sfQualityIn) : 0; const bool bQualityOut = mTxn.isFieldPresent(sfQualityOut); - const uint32 uQualityOut = bQualityIn ? mTxn.getFieldU32(sfQualityOut) : 0; const uint160 uCurrencyID = saLimitAmount.getCurrency(); uint160 uDstAccountID = saLimitAmount.getIssuer(); - const bool bFlipped = mTxnAccountID > uDstAccountID; // true, iff current is not lowest. - bool bDelIndex = false; + const bool bHigh = mTxnAccountID > uDstAccountID; // true, iff current is high account. + + uint32 uQualityIn = bQualityIn ? mTxn.getFieldU32(sfQualityIn) : 0; + uint32 uQualityOut = bQualityIn ? mTxn.getFieldU32(sfQualityOut) : 0; + + if (bQualityIn && QUALITY_ONE == uQualityIn) + uQualityIn = 0; + + if (bQualityOut && QUALITY_ONE == uQualityOut) + uQualityOut = 0; // Check if destination makes sense. @@ -46,83 +54,222 @@ TER TrustSetTransactor::doApply() return terNO_DST; } + const STAmount saSrcXRPBalance = mTxnAccount->getFieldAmount(sfBalance); + const uint32 uOwnerCount = mTxnAccount->getFieldU32(sfOwnerCount); + // The reserve required to create the line. + const uint64 uReserveCreate = theApp->scaleFeeBase(theConfig.FEE_ACCOUNT_RESERVE + (uOwnerCount+1)* theConfig.FEE_OWNER_RESERVE); + STAmount saLimitAllow = saLimitAmount; saLimitAllow.setIssuer(mTxnAccountID); SLE::pointer sleRippleState = mEngine->entryCache(ltRIPPLE_STATE, Ledger::getRippleStateIndex(mTxnAccountID, uDstAccountID, uCurrencyID)); if (sleRippleState) { - // A line exists in one or more directions. + STAmount saLowBalance; + STAmount saLowLimit; + STAmount saHighBalance; + STAmount saHighLimit; + uint32 uLowQualityIn; + uint32 uLowQualityOut; + uint32 uHighQualityIn; + uint32 uHighQualityOut; + const uint160& uLowAccountID = !bHigh ? mTxnAccountID : uDstAccountID; + const uint160& uHighAccountID = bHigh ? mTxnAccountID : uDstAccountID; + SLE::ref sleLowAccount = !bHigh ? mTxnAccount : sleDst; + SLE::ref sleHighAccount = bHigh ? mTxnAccount : sleDst; -#if 0 - // We might delete a ripple state node if everything is set to defaults. - // However, this is problematic as it may make predicting reserve amounts harder for users. - // The code here is incomplete. + // + // Balances + // - if (!saLimitAmount) + saLowBalance = sleRippleState->getFieldAmount(sfBalance); + saHighBalance = -saLowBalance; + + // + // Limits + // + + sleRippleState->setFieldAmount(!bHigh ? sfLowLimit : sfHighLimit, saLimitAllow); + + saLowLimit = !bHigh ? saLimitAllow : sleRippleState->getFieldAmount(sfLowLimit); + saHighLimit = bHigh ? saLimitAllow : sleRippleState->getFieldAmount(sfHighLimit); + + // + // Quality in + // + + if (!bQualityIn) { - // Zeroing line. - uint160 uLowID = sleRippleState->getFieldAmount(sfLowLimit).getIssuer(); - uint160 uHighID = sleRippleState->getFieldAmount(sfHighLimit).getIssuer(); - bool bLow = uLowID == uSrcAccountID; - bool bHigh = uLowID == uDstAccountID; - bool bBalanceZero = !sleRippleState->getFieldAmount(sfBalance); - STAmount saDstLimit = sleRippleState->getFieldAmount(bSendLow ? sfLowLimit : sfHighLimit); - bool bDstLimitZero = !saDstLimit; + // Not setting. Just get it. - assert(bLow || bHigh); - - if (bBalanceZero && bDstLimitZero) - { - // Zero balance and eliminating last limit. - - bDelIndex = true; - terResult = dirDelete(false, uSrcRef, Ledger::getOwnerDirIndex(mTxnAccountID), sleRippleState->getIndex(), false); - } + uLowQualityIn = sleRippleState->getFieldU32(sfLowQualityIn); + uHighQualityIn = sleRippleState->getFieldU32(sfHighQualityIn); } -#endif - - if (!bDelIndex) + else if (uQualityIn) { - sleRippleState->setFieldAmount(bFlipped ? sfHighLimit: sfLowLimit, saLimitAllow); + // Setting. - if (!bQualityIn) - { - nothing(); - } - else if (uQualityIn) - { - sleRippleState->setFieldU32(bFlipped ? sfLowQualityIn : sfHighQualityIn, uQualityIn); - } - else - { - sleRippleState->makeFieldAbsent(bFlipped ? sfLowQualityIn : sfHighQualityIn); - } + sleRippleState->setFieldU32(!bHigh ? sfLowQualityIn : sfHighQualityIn, uQualityIn); - if (!bQualityOut) - { - nothing(); - } - else if (uQualityOut) - { - sleRippleState->setFieldU32(bFlipped ? sfLowQualityOut : sfHighQualityOut, uQualityOut); - } - else - { - sleRippleState->makeFieldAbsent(bFlipped ? sfLowQualityOut : sfHighQualityOut); - } + uLowQualityIn = !bHigh ? uQualityIn : sleRippleState->getFieldU32(sfLowQualityIn); + uHighQualityIn = bHigh ? uQualityIn : sleRippleState->getFieldU32(sfHighQualityIn); + } + else + { + // Clearing. + sleRippleState->makeFieldAbsent(!bHigh ? sfLowQualityIn : sfHighQualityIn); + + uLowQualityIn = !bHigh ? 0 : sleRippleState->getFieldU32(sfLowQualityIn); + uHighQualityIn = bHigh ? 0 : sleRippleState->getFieldU32(sfHighQualityIn); + } + + if (QUALITY_ONE == uLowQualityIn) uLowQualityIn = 0; + if (QUALITY_ONE == uHighQualityIn) uHighQualityIn = 0; + + // + // Quality out + // + + if (!bQualityOut) + { + // Not setting. Just get it. + + uLowQualityOut = sleRippleState->getFieldU32(sfLowQualityOut); + uHighQualityOut = sleRippleState->getFieldU32(sfHighQualityOut); + } + else if (uQualityOut) + { + // Setting. + + sleRippleState->setFieldU32(!bHigh ? sfLowQualityOut : sfHighQualityOut, uQualityOut); + + uLowQualityOut = !bHigh ? uQualityOut : sleRippleState->getFieldU32(sfLowQualityOut); + uHighQualityOut = bHigh ? uQualityOut : sleRippleState->getFieldU32(sfHighQualityOut); + } + else + { + // Clearing. + + sleRippleState->makeFieldAbsent(!bHigh ? sfLowQualityOut : sfHighQualityOut); + + uLowQualityOut = !bHigh ? 0 : sleRippleState->getFieldU32(sfLowQualityOut); + uHighQualityOut = bHigh ? 0 : sleRippleState->getFieldU32(sfHighQualityOut); + } + + if (QUALITY_ONE == uLowQualityOut) uLowQualityOut = 0; + if (QUALITY_ONE == uHighQualityOut) uHighQualityOut = 0; + + const bool bLowReserveSet = uLowQualityIn || uLowQualityOut || !!saLowLimit || saLowBalance.isPositive(); + const bool bLowReserveClear = !bLowReserveSet; + + const bool bHighReserveSet = uHighQualityIn || uHighQualityOut || !!saHighLimit || saHighBalance.isPositive(); + const bool bHighReserveClear = !bHighReserveSet; + + const bool bDefault = bLowReserveClear && bHighReserveClear; + + const uint32 uFlagsIn = sleRippleState->getFieldU32(sfFlags); + uint32 uFlagsOut = uFlagsIn; + + const bool bLowReserved = isSetBit(uFlagsIn, lsfLowReserve); + const bool bHighReserved = isSetBit(uFlagsIn, lsfHighReserve); + + bool bReserveIncrease = false; + + if (bLowReserveSet && !bLowReserved) + { + // Set reserve for low account. + + terResult = mEngine->getNodes().ownerCountAdjust(uLowAccountID, 1, sleLowAccount); + uFlagsOut |= lsfLowReserve; + + if (!bHigh) + bReserveIncrease = true; + } + + if (tesSUCCESS == terResult && bLowReserveClear && bLowReserved) + { + // Clear reserve for low account. + + terResult = mEngine->getNodes().ownerCountAdjust(uLowAccountID, -1, sleLowAccount); + uFlagsOut &= ~lsfLowReserve; + } + + if (tesSUCCESS == terResult && bHighReserveSet && !bHighReserved) + { + // Set reserve for high account. + + terResult = mEngine->getNodes().ownerCountAdjust(uHighAccountID, 1, sleHighAccount); + uFlagsOut |= lsfHighReserve; + + if (bHigh) + bReserveIncrease = true; + } + + if (tesSUCCESS == terResult && bHighReserveClear && bHighReserved) + { + // Clear reserve for high account. + + terResult = mEngine->getNodes().ownerCountAdjust(uHighAccountID, -1, sleHighAccount); + uFlagsOut &= ~lsfHighReserve; + } + + if (uFlagsIn != uFlagsOut) + sleRippleState->setFieldU32(sfFlags, uFlagsOut); + + if (tesSUCCESS != terResult) + { + Log(lsINFO) << "doTrustSet: Error"; + + nothing(); + } + else if (bDefault) + { + // Can delete. + + uint64 uSrcRef; // <-- Ignored, dirs never delete. + + terResult = mEngine->getNodes().dirDelete(false, uSrcRef, Ledger::getOwnerDirIndex(uLowAccountID), sleRippleState->getIndex(), false); + + if (tesSUCCESS == terResult) + terResult = mEngine->getNodes().dirDelete(false, uSrcRef, Ledger::getOwnerDirIndex(uHighAccountID), sleRippleState->getIndex(), false); + + mEngine->entryDelete(sleRippleState); + + Log(lsINFO) << "doTrustSet: Deleting ripple line"; + } + else if (bReserveIncrease + && isSetBit(mParams, tapOPEN_LEDGER) // Ledger is not final, we can vote no. + && saSrcXRPBalance.getNValue() < uReserveCreate) // Reserve is not scaled by load. + { + Log(lsINFO) << "doTrustSet: Delay transaction: Insufficent reserve to add trust line."; + + // Another transaction could provide XRP to the account and then this transaction would succeed. + terResult = terINSUF_RESERVE; + } + else + { mEngine->entryModify(sleRippleState); - } - Log(lsINFO) << "doTrustSet: Modifying ripple line: bDelIndex=" << bDelIndex; + Log(lsINFO) << "doTrustSet: Modify ripple line"; + } } // Line does not exist. - else if (!saLimitAmount) + else if (!saLimitAmount // Setting default limit. + && (!bQualityIn || !uQualityIn) // Not setting quality in or setting default quality in. + && (!bQualityOut || !uQualityOut)) // Not setting quality out or setting default quality out. { - Log(lsINFO) << "doTrustSet: Redundant: Setting non-existent ripple line to 0."; + Log(lsINFO) << "doTrustSet: Redundant: Setting non-existent ripple line to defaults."; - return terNO_LINE_NO_ZERO; + return terNO_LINE_REDUNDANT; + } + else if (isSetBit(mParams, tapOPEN_LEDGER) // Ledger is not final, we can vote no. + && saSrcXRPBalance.getNValue() < uReserveCreate) // Reserve is not scaled by load. + { + Log(lsINFO) << "doTrustSet: Delay transaction: Line does not exist. Insufficent reserve to create line."; + + // Another transaction could create the account and then this transaction would succeed. + terResult = terNO_LINE_INSUF_RESERVE; } else { @@ -132,13 +279,16 @@ TER TrustSetTransactor::doApply() Log(lsINFO) << "doTrustSet: Creating ripple line: " << sleRippleState->getIndex().ToString(); sleRippleState->setFieldAmount(sfBalance, STAmount(uCurrencyID, ACCOUNT_ONE)); // Zero balance in currency. - sleRippleState->setFieldAmount(bFlipped ? sfHighLimit : sfLowLimit, saLimitAllow); - sleRippleState->setFieldAmount(bFlipped ? sfLowLimit : sfHighLimit, STAmount(uCurrencyID, uDstAccountID)); + sleRippleState->setFieldAmount(!bHigh ? sfLowLimit : sfHighLimit, saLimitAllow); + sleRippleState->setFieldAmount( bHigh ? sfLowLimit : sfHighLimit, STAmount(uCurrencyID, uDstAccountID)); if (uQualityIn) - sleRippleState->setFieldU32(bFlipped ? sfHighQualityIn : sfLowQualityIn, uQualityIn); + sleRippleState->setFieldU32(!bHigh ? sfLowQualityIn : sfHighQualityIn, uQualityIn); + if (uQualityOut) - sleRippleState->setFieldU32(bFlipped ? sfHighQualityOut : sfLowQualityOut, uQualityOut); + sleRippleState->setFieldU32(!bHigh ? sfLowQualityOut : sfHighQualityOut, uQualityOut); + + sleRippleState->setFieldU32(sfFlags, !bHigh ? lsfLowReserve : lsfHighReserve); uint64 uSrcRef; // <-- Ignored, dirs never delete. @@ -157,9 +307,6 @@ TER TrustSetTransactor::doApply() Ledger::getOwnerDirIndex(uDstAccountID), sleRippleState->getIndex(), boost::bind(&Ledger::ownerDirDescriber, _1, uDstAccountID)); - - if (tesSUCCESS == terResult) - terResult = mEngine->getNodes().ownerCountAdjust(uDstAccountID, 1, sleDst); } Log(lsINFO) << "doTrustSet<"; diff --git a/test/send-test.js b/test/send-test.js index 00ee6f339..85c4487cd 100644 --- a/test/send-test.js +++ b/test/send-test.js @@ -180,25 +180,6 @@ buster.testCase("Sending", { }) .request(); }, - function (callback) { - self.what = "Zero a credit limit."; - - testutils.credit_limit(self.remote, "alice", "0/USD/mtgox", callback); - }, - function (callback) { - self.what = "Make sure still exists."; - - self.remote.request_ripple_balance("alice", "mtgox", "USD", 'CURRENT') - .on('ripple_state', function (m) { - buster.assert(m.account_balance.equals("0/USD/alice")); - buster.assert(m.account_limit.equals("0/USD/alice")); - buster.assert(m.issuer_balance.equals("0/USD/mtgox")); - buster.assert(m.issuer_limit.equals("0/USD/mtgox")); - - callback(); - }) - .request(); - }, // Set negative limit. function (callback) { self.remote.transaction() @@ -212,6 +193,33 @@ buster.testCase("Sending", { }) .submit(); }, + function (callback) { + self.what = "Zero a credit limit."; + + testutils.credit_limit(self.remote, "alice", "0/USD/mtgox", callback); + }, + function (callback) { + self.what = "Make sure line is deleted."; + + self.remote.request_ripple_balance("alice", "mtgox", "USD", 'CURRENT') + .on('ripple_state', function (m) { + // Used to keep lines. + // buster.assert(m.account_balance.equals("0/USD/alice")); + // buster.assert(m.account_limit.equals("0/USD/alice")); + // buster.assert(m.issuer_balance.equals("0/USD/mtgox")); + // buster.assert(m.issuer_limit.equals("0/USD/mtgox")); + + buster.assert(false); + }) + .on('error', function (m) { + // console.log("error: %s", JSON.stringify(m)); + buster.assert.equals('remoteError', m.error); + buster.assert.equals('entryNotFound', m.remote.error); + + callback(); + }) + .request(); + }, // TODO Check in both owner books. function (callback) { self.what = "Set another limit."; diff --git a/testcommit.txt b/testcommit.txt deleted file mode 100644 index c05ac2bc0..000000000 --- a/testcommit.txt +++ /dev/null @@ -1 +0,0 @@ -test 12-17-12