From 1cc4b2a132275b72e979dee0d350112fbb9a1f26 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Mon, 17 Dec 2012 21:29:40 -0800 Subject: [PATCH 1/7] Add new type specific flags for ripple state entry reserves. --- src/cpp/ripple/LedgerFormats.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 From b04aa198e6df08c28b959194a66ea56e5045239b Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Mon, 17 Dec 2012 21:30:19 -0800 Subject: [PATCH 2/7] Revise trust set transactor for reserves. --- src/cpp/ripple/TransactionErr.cpp | 2 +- src/cpp/ripple/TransactionErr.h | 2 +- src/cpp/ripple/TrustSetTransactor.cpp | 261 +++++++++++++++++++------- 3 files changed, 196 insertions(+), 69 deletions(-) diff --git a/src/cpp/ripple/TransactionErr.cpp b/src/cpp/ripple/TransactionErr.cpp index a28ab5bc8..40aa6edff 100644 --- a/src/cpp/ripple/TransactionErr.cpp +++ b/src/cpp/ripple/TransactionErr.cpp @@ -54,7 +54,7 @@ bool transResultInfo(TER terCode, std::string& strToken, std::string& strHuman) { 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_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..bed4e4f6e 100644 --- a/src/cpp/ripple/TransactionErr.h +++ b/src/cpp/ripple/TransactionErr.h @@ -84,7 +84,7 @@ enum TER // aka TransactionEngineResult terNO_DST, terNO_DST_INSUF_XRP, terNO_LINE, - terNO_LINE_NO_ZERO, + terNO_LINE_REDUNDANT, terPRE_SEQ, terSET_MISSING_DST, terUNFUNDED, diff --git a/src/cpp/ripple/TrustSetTransactor.cpp b/src/cpp/ripple/TrustSetTransactor.cpp index 54e1efc12..7560bc000 100644 --- a/src/cpp/ripple/TrustSetTransactor.cpp +++ b/src/cpp/ripple/TrustSetTransactor.cpp @@ -9,13 +9,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; + + 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. @@ -52,77 +58,198 @@ TER TrustSetTransactor::doApply() 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 = !bFlipped ? mTxnAccountID : uDstAccountID; + const uint160& uHighAccountID = bFlipped ? mTxnAccountID : uDstAccountID; + SLE::ref sleLowAccount = !bFlipped ? mTxnAccount : sleDst; + SLE::ref sleHighAccount = bFlipped ? 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; + + if (bFlipped) { - // 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; - - assert(bLow || bHigh); - - if (bBalanceZero && bDstLimitZero) - { - // Zero balance and eliminating last limit. - - bDelIndex = true; - terResult = dirDelete(false, uSrcRef, Ledger::getOwnerDirIndex(mTxnAccountID), sleRippleState->getIndex(), false); - } + saLowBalance.negate(); } -#endif - - if (!bDelIndex) + else { - sleRippleState->setFieldAmount(bFlipped ? sfHighLimit: sfLowLimit, saLimitAllow); + saHighBalance.negate(); + } - if (!bQualityIn) - { - nothing(); - } - else if (uQualityIn) - { - sleRippleState->setFieldU32(bFlipped ? sfLowQualityIn : sfHighQualityIn, uQualityIn); - } - else - { - sleRippleState->makeFieldAbsent(bFlipped ? sfLowQualityIn : sfHighQualityIn); - } + // + // Limits + // - if (!bQualityOut) - { - nothing(); - } - else if (uQualityOut) - { - sleRippleState->setFieldU32(bFlipped ? sfLowQualityOut : sfHighQualityOut, uQualityOut); - } - else - { - sleRippleState->makeFieldAbsent(bFlipped ? sfLowQualityOut : sfHighQualityOut); - } + if (bFlipped) + { + sleRippleState->setFieldAmount(sfHighLimit, saLimitAllow); + saLowLimit = sleRippleState->getFieldAmount(sfLowLimit); + saHighLimit = saLimitAllow; + } + else + { + sleRippleState->setFieldAmount(sfLowLimit, saLimitAllow); + + saLowLimit = saLimitAllow; + saHighLimit = sleRippleState->getFieldAmount(sfHighLimit); + } + + // + // Quality in + // + + if (!bQualityIn) + { + // Not setting. Just get it. + + uLowQualityIn = sleRippleState->getFieldU32(sfLowQualityIn); + uHighQualityIn = sleRippleState->getFieldU32(sfHighQualityIn); + } + else if (uQualityIn) + { + // Setting. + + sleRippleState->setFieldU32(!bFlipped ? sfLowQualityIn : sfHighQualityIn, uQualityIn); + + uLowQualityIn = !bFlipped ? uQualityIn : sleRippleState->getFieldU32(sfLowQualityIn); + uHighQualityIn = bFlipped ? uQualityIn : sleRippleState->getFieldU32(sfHighQualityIn); + } + else + { + // Clearing. + + sleRippleState->makeFieldAbsent(!bFlipped ? sfLowQualityIn : sfHighQualityIn); + + uLowQualityIn = !bFlipped ? 0 : sleRippleState->getFieldU32(sfLowQualityIn); + uHighQualityIn = bFlipped ? 0 : sleRippleState->getFieldU32(sfHighQualityIn); + } + + // + // Quality out + // + + if (!bQualityOut) + { + // Not setting. Just get it. + + uLowQualityOut = sleRippleState->getFieldU32(sfLowQualityOut); + uHighQualityOut = sleRippleState->getFieldU32(sfHighQualityOut); + } + else if (uQualityOut) + { + // Setting. + + sleRippleState->setFieldU32(!bFlipped ? sfLowQualityOut : sfHighQualityOut, uQualityOut); + + uLowQualityOut = !bFlipped ? uQualityOut : sleRippleState->getFieldU32(sfLowQualityOut); + uHighQualityOut = bFlipped ? uQualityOut : sleRippleState->getFieldU32(sfHighQualityOut); + } + else + { + // Clearing. + + sleRippleState->makeFieldAbsent(!bFlipped ? sfLowQualityOut : sfHighQualityOut); + + uLowQualityOut = !bFlipped ? 0 : sleRippleState->getFieldU32(sfLowQualityOut); + uHighQualityOut = bFlipped ? 0 : sleRippleState->getFieldU32(sfHighQualityOut); + } + + if (QUALITY_ONE == uLowQualityIn) uLowQualityIn = 0; + if (QUALITY_ONE == uHighQualityIn) uHighQualityIn = 0; + if (QUALITY_ONE == uLowQualityOut) uLowQualityOut = 0; + if (QUALITY_ONE == uHighQualityOut) uHighQualityOut = 0; + + const bool bLowReserveSet = uLowQualityIn || uLowQualityOut || !!saLowLimit || saLowBalance.isPositive(); + const bool bLowReserveClear = !uLowQualityIn && !uLowQualityOut && !saLowLimit && !saLowBalance.isPositive(); + + const bool bHighReserveSet = uHighQualityIn || uHighQualityOut || !!saHighLimit || saHighBalance.isPositive(); + const bool bHighReserveClear = !uHighQualityIn && !uHighQualityOut && !saHighLimit && !saHighBalance.isPositive(); + 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); + + if (bLowReserveSet && !bLowReserved) + { + // Set reserve for low account. + + terResult = mEngine->getNodes().ownerCountAdjust(uLowAccountID, 1, sleLowAccount); + uFlagsOut |= lsfLowReserve; + } + + if (bLowReserveClear && bLowReserved) + { + // Clear reserve for low account. + + terResult = mEngine->getNodes().ownerCountAdjust(uLowAccountID, -1, sleLowAccount); + uFlagsOut &= ~lsfLowReserve; + } + + if (bHighReserveSet && !bHighReserved) + { + // Set reserve for high account. + + terResult = mEngine->getNodes().ownerCountAdjust(uHighAccountID, 1, sleHighAccount); + uFlagsOut |= lsfHighReserve; + } + + if (bHighReserveClear && bHighReserved) + { + // Clear reserve for high account. + + terResult = mEngine->getNodes().ownerCountAdjust(uHighAccountID, -1, sleHighAccount); + uFlagsOut &= ~lsfHighReserve; + } + + if (uFlagsIn != uFlagsOut) + sleRippleState->setFieldU32(sfFlags, uFlagsOut); + + 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 + { 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 // Setting default quality in. + && bQualityOut && !uQualityOut // 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 { @@ -132,13 +259,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(!bFlipped ? sfLowLimit : sfHighLimit, saLimitAllow); + sleRippleState->setFieldAmount( bFlipped ? sfLowLimit : sfHighLimit, STAmount(uCurrencyID, uDstAccountID)); if (uQualityIn) - sleRippleState->setFieldU32(bFlipped ? sfHighQualityIn : sfLowQualityIn, uQualityIn); + sleRippleState->setFieldU32(!bFlipped ? sfLowQualityIn : sfHighQualityIn, uQualityIn); + if (uQualityOut) - sleRippleState->setFieldU32(bFlipped ? sfHighQualityOut : sfLowQualityOut, uQualityOut); + sleRippleState->setFieldU32(!bFlipped ? sfLowQualityOut : sfHighQualityOut, uQualityOut); + + sleRippleState->setFieldU32(sfFlags, !bFlipped ? lsfLowReserve : lsfHighReserve); uint64 uSrcRef; // <-- Ignored, dirs never delete. @@ -157,9 +287,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<"; From 1ae62cac139157fe10ae39aff5c00e84570f1f56 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Tue, 18 Dec 2012 12:29:20 -0800 Subject: [PATCH 3/7] Clean up and fix TrustSet. --- src/cpp/ripple/TrustSetTransactor.cpp | 86 +++++++++++---------------- 1 file changed, 34 insertions(+), 52 deletions(-) diff --git a/src/cpp/ripple/TrustSetTransactor.cpp b/src/cpp/ripple/TrustSetTransactor.cpp index 7560bc000..1f80dbde4 100644 --- a/src/cpp/ripple/TrustSetTransactor.cpp +++ b/src/cpp/ripple/TrustSetTransactor.cpp @@ -12,7 +12,7 @@ TER TrustSetTransactor::doApply() const bool bQualityOut = mTxn.isFieldPresent(sfQualityOut); const uint160 uCurrencyID = saLimitAmount.getCurrency(); uint160 uDstAccountID = saLimitAmount.getIssuer(); - const bool bFlipped = mTxnAccountID > uDstAccountID; // true, iff current is not lowest. + 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; @@ -66,45 +66,26 @@ TER TrustSetTransactor::doApply() uint32 uLowQualityOut; uint32 uHighQualityIn; uint32 uHighQualityOut; - const uint160& uLowAccountID = !bFlipped ? mTxnAccountID : uDstAccountID; - const uint160& uHighAccountID = bFlipped ? mTxnAccountID : uDstAccountID; - SLE::ref sleLowAccount = !bFlipped ? mTxnAccount : sleDst; - SLE::ref sleHighAccount = bFlipped ? mTxnAccount : sleDst; + const uint160& uLowAccountID = !bHigh ? mTxnAccountID : uDstAccountID; + const uint160& uHighAccountID = bHigh ? mTxnAccountID : uDstAccountID; + SLE::ref sleLowAccount = !bHigh ? mTxnAccount : sleDst; + SLE::ref sleHighAccount = bHigh ? mTxnAccount : sleDst; // // Balances // saLowBalance = sleRippleState->getFieldAmount(sfBalance); - saHighBalance = saLowBalance; - - if (bFlipped) - { - saLowBalance.negate(); - } - else - { - saHighBalance.negate(); - } + saHighBalance = -saLowBalance; // // Limits // - if (bFlipped) - { - sleRippleState->setFieldAmount(sfHighLimit, saLimitAllow); + sleRippleState->setFieldAmount(!bHigh ? sfLowLimit : sfHighLimit, saLimitAllow); - saLowLimit = sleRippleState->getFieldAmount(sfLowLimit); - saHighLimit = saLimitAllow; - } - else - { - sleRippleState->setFieldAmount(sfLowLimit, saLimitAllow); - - saLowLimit = saLimitAllow; - saHighLimit = sleRippleState->getFieldAmount(sfHighLimit); - } + saLowLimit = !bHigh ? saLimitAllow : sleRippleState->getFieldAmount(sfLowLimit); + saHighLimit = bHigh ? saLimitAllow : sleRippleState->getFieldAmount(sfHighLimit); // // Quality in @@ -121,21 +102,24 @@ TER TrustSetTransactor::doApply() { // Setting. - sleRippleState->setFieldU32(!bFlipped ? sfLowQualityIn : sfHighQualityIn, uQualityIn); + sleRippleState->setFieldU32(!bHigh ? sfLowQualityIn : sfHighQualityIn, uQualityIn); - uLowQualityIn = !bFlipped ? uQualityIn : sleRippleState->getFieldU32(sfLowQualityIn); - uHighQualityIn = bFlipped ? uQualityIn : sleRippleState->getFieldU32(sfHighQualityIn); + uLowQualityIn = !bHigh ? uQualityIn : sleRippleState->getFieldU32(sfLowQualityIn); + uHighQualityIn = bHigh ? uQualityIn : sleRippleState->getFieldU32(sfHighQualityIn); } else { // Clearing. - sleRippleState->makeFieldAbsent(!bFlipped ? sfLowQualityIn : sfHighQualityIn); + sleRippleState->makeFieldAbsent(!bHigh ? sfLowQualityIn : sfHighQualityIn); - uLowQualityIn = !bFlipped ? 0 : sleRippleState->getFieldU32(sfLowQualityIn); - uHighQualityIn = bFlipped ? 0 : sleRippleState->getFieldU32(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 // @@ -151,31 +135,30 @@ TER TrustSetTransactor::doApply() { // Setting. - sleRippleState->setFieldU32(!bFlipped ? sfLowQualityOut : sfHighQualityOut, uQualityOut); + sleRippleState->setFieldU32(!bHigh ? sfLowQualityOut : sfHighQualityOut, uQualityOut); - uLowQualityOut = !bFlipped ? uQualityOut : sleRippleState->getFieldU32(sfLowQualityOut); - uHighQualityOut = bFlipped ? uQualityOut : sleRippleState->getFieldU32(sfHighQualityOut); + uLowQualityOut = !bHigh ? uQualityOut : sleRippleState->getFieldU32(sfLowQualityOut); + uHighQualityOut = bHigh ? uQualityOut : sleRippleState->getFieldU32(sfHighQualityOut); } else { // Clearing. - sleRippleState->makeFieldAbsent(!bFlipped ? sfLowQualityOut : sfHighQualityOut); + sleRippleState->makeFieldAbsent(!bHigh ? sfLowQualityOut : sfHighQualityOut); - uLowQualityOut = !bFlipped ? 0 : sleRippleState->getFieldU32(sfLowQualityOut); - uHighQualityOut = bFlipped ? 0 : sleRippleState->getFieldU32(sfHighQualityOut); + uLowQualityOut = !bHigh ? 0 : sleRippleState->getFieldU32(sfLowQualityOut); + uHighQualityOut = bHigh ? 0 : sleRippleState->getFieldU32(sfHighQualityOut); } - if (QUALITY_ONE == uLowQualityIn) uLowQualityIn = 0; - if (QUALITY_ONE == uHighQualityIn) uHighQualityIn = 0; if (QUALITY_ONE == uLowQualityOut) uLowQualityOut = 0; if (QUALITY_ONE == uHighQualityOut) uHighQualityOut = 0; const bool bLowReserveSet = uLowQualityIn || uLowQualityOut || !!saLowLimit || saLowBalance.isPositive(); - const bool bLowReserveClear = !uLowQualityIn && !uLowQualityOut && !saLowLimit && !saLowBalance.isPositive(); + const bool bLowReserveClear = !bLowReserveSet; const bool bHighReserveSet = uHighQualityIn || uHighQualityOut || !!saHighLimit || saHighBalance.isPositive(); - const bool bHighReserveClear = !uHighQualityIn && !uHighQualityOut && !saHighLimit && !saHighBalance.isPositive(); + const bool bHighReserveClear = !bHighReserveSet; + const bool bDefault = bLowReserveClear && bHighReserveClear; const uint32 uFlagsIn = sleRippleState->getFieldU32(sfFlags); @@ -243,9 +226,8 @@ TER TrustSetTransactor::doApply() } // Line does not exist. else if (!saLimitAmount // Setting default limit. - && bQualityIn && !uQualityIn // Setting default quality in. - && bQualityOut && !uQualityOut // Setting default quality out. - ) + && (!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 defaults."; @@ -259,16 +241,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 ? sfLowLimit : sfHighLimit, 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 ? sfLowQualityIn : sfHighQualityIn, uQualityIn); + sleRippleState->setFieldU32(!bHigh ? sfLowQualityIn : sfHighQualityIn, uQualityIn); if (uQualityOut) - sleRippleState->setFieldU32(!bFlipped ? sfLowQualityOut : sfHighQualityOut, uQualityOut); + sleRippleState->setFieldU32(!bHigh ? sfLowQualityOut : sfHighQualityOut, uQualityOut); - sleRippleState->setFieldU32(sfFlags, !bFlipped ? lsfLowReserve : lsfHighReserve); + sleRippleState->setFieldU32(sfFlags, !bHigh ? lsfLowReserve : lsfHighReserve); uint64 uSrcRef; // <-- Ignored, dirs never delete. From 3bab52fb7514b223743b7770963ea04e4b9f810c Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Tue, 18 Dec 2012 13:20:39 -0800 Subject: [PATCH 4/7] Note opportunity for ledger wipe. --- src/cpp/ripple/TransactionFormats.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; From dc2b1d55aed48c027b361e9a7391188503bc7c17 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Tue, 18 Dec 2012 14:04:54 -0800 Subject: [PATCH 5/7] Have TrustSet require reserve to set. --- src/cpp/ripple/TransactionErr.cpp | 2 + src/cpp/ripple/TransactionErr.h | 2 + src/cpp/ripple/TrustSetTransactor.cpp | 58 ++++++++++++++++++++++----- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/cpp/ripple/TransactionErr.cpp b/src/cpp/ripple/TransactionErr.cpp index 40aa6edff..a36107e3b 100644 --- a/src/cpp/ripple/TransactionErr.cpp +++ b/src/cpp/ripple/TransactionErr.cpp @@ -50,10 +50,12 @@ 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_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." }, diff --git a/src/cpp/ripple/TransactionErr.h b/src/cpp/ripple/TransactionErr.h index bed4e4f6e..0a17405ec 100644 --- a/src/cpp/ripple/TransactionErr.h +++ b/src/cpp/ripple/TransactionErr.h @@ -80,10 +80,12 @@ 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_INSUF_RESERVE, terNO_LINE_REDUNDANT, terPRE_SEQ, terSET_MISSING_DST, diff --git a/src/cpp/ripple/TrustSetTransactor.cpp b/src/cpp/ripple/TrustSetTransactor.cpp index 1f80dbde4..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 @@ -52,6 +54,11 @@ 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); @@ -167,15 +174,20 @@ TER TrustSetTransactor::doApply() 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; + terResult = mEngine->getNodes().ownerCountAdjust(uLowAccountID, 1, sleLowAccount); + uFlagsOut |= lsfLowReserve; + + if (!bHigh) + bReserveIncrease = true; } - if (bLowReserveClear && bLowReserved) + if (tesSUCCESS == terResult && bLowReserveClear && bLowReserved) { // Clear reserve for low account. @@ -183,15 +195,18 @@ TER TrustSetTransactor::doApply() uFlagsOut &= ~lsfLowReserve; } - if (bHighReserveSet && !bHighReserved) + if (tesSUCCESS == terResult && bHighReserveSet && !bHighReserved) { // Set reserve for high account. terResult = mEngine->getNodes().ownerCountAdjust(uHighAccountID, 1, sleHighAccount); uFlagsOut |= lsfHighReserve; + + if (bHigh) + bReserveIncrease = true; } - if (bHighReserveClear && bHighReserved) + if (tesSUCCESS == terResult && bHighReserveClear && bHighReserved) { // Clear reserve for high account. @@ -202,11 +217,17 @@ TER TrustSetTransactor::doApply() if (uFlagsIn != uFlagsOut) sleRippleState->setFieldU32(sfFlags, uFlagsOut); - if (bDefault) + if (tesSUCCESS != terResult) + { + Log(lsINFO) << "doTrustSet: Error"; + + nothing(); + } + else if (bDefault) { // Can delete. - uint64 uSrcRef; // <-- Ignored, dirs never delete. + uint64 uSrcRef; // <-- Ignored, dirs never delete. terResult = mEngine->getNodes().dirDelete(false, uSrcRef, Ledger::getOwnerDirIndex(uLowAccountID), sleRippleState->getIndex(), false); @@ -217,6 +238,15 @@ TER TrustSetTransactor::doApply() 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); @@ -225,14 +255,22 @@ TER TrustSetTransactor::doApply() } } // Line does not exist. - 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. + 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 defaults."; 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 { // Create a new ripple line. From 6c3c104f8d7f1c3d1809a9c839885be85d0fc8ed Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Tue, 18 Dec 2012 14:27:18 -0800 Subject: [PATCH 6/7] UT: Update send test for deleting ripple state entries. --- test/send-test.js | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) 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."; From 05b77f7468129eea418294fa8088a5d396d5d81b Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Tue, 18 Dec 2012 14:28:34 -0800 Subject: [PATCH 7/7] Remove test file. --- testcommit.txt | 1 - 1 file changed, 1 deletion(-) delete mode 100644 testcommit.txt 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