From 77d92e176726fda830471a723753ec699b69af73 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Tue, 25 Dec 2012 16:14:10 -0800 Subject: [PATCH] Fix ripple state creating and deleting. --- src/cpp/ripple/LedgerEntrySet.cpp | 91 +++++++++++++++++++++++---- src/cpp/ripple/LedgerEntrySet.h | 11 ++++ src/cpp/ripple/LedgerFormats.cpp | 2 + src/cpp/ripple/SerializeProto.h | 2 + src/cpp/ripple/TransactionErr.cpp | 1 + src/cpp/ripple/TransactionErr.h | 1 + src/cpp/ripple/Transactor.cpp | 13 ++-- src/cpp/ripple/TrustSetTransactor.cpp | 67 ++++++++------------ test/send-test.js | 20 ++++-- 9 files changed, 142 insertions(+), 66 deletions(-) diff --git a/src/cpp/ripple/LedgerEntrySet.cpp b/src/cpp/ripple/LedgerEntrySet.cpp index 0060479b9b..b8ba586f83 100644 --- a/src/cpp/ripple/LedgerEntrySet.cpp +++ b/src/cpp/ripple/LedgerEntrySet.cpp @@ -1,6 +1,7 @@ #include "LedgerEntrySet.h" +#include #include #include @@ -620,7 +621,7 @@ TER LedgerEntrySet::dirDelete( const bool bKeepRoot, // --> True, if we never completely clean up, after we overflow the root node. const uint64& uNodeDir, // --> Node containing entry. const uint256& uRootIndex, // --> The index of the base of the directory. Nodes are based off of this. - const uint256& uLedgerIndex, // --> Value to add to directory. + const uint256& uLedgerIndex, // --> Value to remove from directory. const bool bStable) // --> True, not to change relative order of entries. { uint64 uNodeCur = uNodeDir; @@ -628,7 +629,11 @@ TER LedgerEntrySet::dirDelete( if (!sleNode) { - cLog(lsWARNING) << "dirDelete: no such node"; + cLog(lsWARNING) + << boost::str(boost::format("dirDelete: no such node: uRootIndex=%s uNodeDir=%s uLedgerIndex=%s") + % uRootIndex.ToString() + % strHex(uNodeDir) + % uLedgerIndex.ToString()); assert(false); return tefBAD_LEDGER; @@ -1130,6 +1135,64 @@ STAmount LedgerEntrySet::rippleTransferFee(const uint160& uSenderID, const uint1 return saTransitFee; } +TER LedgerEntrySet::trustCreate( + const bool bSrcHigh, // Who to charge with reserve. + const uint160& uSrcAccountID, + SLE::ref sleSrcAccount, + const uint160& uDstAccountID, + const uint256& uIndex, + const STAmount& saSrcBalance, // Issuer should be ACCOUNT_ONE + const STAmount& saSrcLimit, + const uint32 uSrcQualityIn, + const uint32 uSrcQualityOut) +{ + const uint160& uLowAccountID = !bSrcHigh ? uSrcAccountID : uDstAccountID; + const uint160& uHighAccountID = bSrcHigh ? uSrcAccountID : uDstAccountID; + + SLE::pointer sleRippleState = entryCreate(ltRIPPLE_STATE, uIndex); + + uint64 uLowNode; + uint64 uHighNode; + + TER terResult = dirAdd( + uLowNode, + Ledger::getOwnerDirIndex(uLowAccountID), + sleRippleState->getIndex(), + boost::bind(&Ledger::ownerDirDescriber, _1, uLowAccountID)); + + if (tesSUCCESS == terResult) + { + terResult = dirAdd( + uHighNode, + Ledger::getOwnerDirIndex(uHighAccountID), + sleRippleState->getIndex(), + boost::bind(&Ledger::ownerDirDescriber, _1, uHighAccountID)); + } + + if (tesSUCCESS == terResult) + { + sleRippleState->setFieldU64(sfLowNode, uLowNode); + sleRippleState->setFieldU64(sfHighNode, uHighNode); + + sleRippleState->setFieldAmount(!bSrcHigh ? sfLowLimit : sfHighLimit, saSrcLimit); + sleRippleState->setFieldAmount( bSrcHigh ? sfLowLimit : sfHighLimit, STAmount(saSrcBalance.getCurrency(), uDstAccountID)); + + if (uSrcQualityIn) + sleRippleState->setFieldU32(bSrcHigh ? sfHighQualityIn : sfLowQualityIn, uSrcQualityIn); + + if (uSrcQualityOut) + sleRippleState->setFieldU32(bSrcHigh ? sfHighQualityOut : sfLowQualityOut, uSrcQualityIn); + + sleRippleState->setFieldU32(sfFlags, !bSrcHigh ? lsfLowReserve : lsfHighReserve); + + ownerCountAdjust(uSrcAccountID, 1, sleSrcAccount); + + sleRippleState->setFieldAmount(sfBalance, bSrcHigh ? -saSrcBalance: saSrcBalance); + } + + return terResult; +} + // Direct send w/o fees: redeeming IOUs and/or sending own IOUs. void LedgerEntrySet::rippleCredit(const uint160& uSenderID, const uint160& uReceiverID, const STAmount& saAmount, bool bCheckIssuer) { @@ -1138,7 +1201,7 @@ void LedgerEntrySet::rippleCredit(const uint160& uSenderID, const uint160& uRece assert(!bCheckIssuer || uSenderID == uIssuerID || uReceiverID == uIssuerID); - bool bFlipped = uSenderID > uReceiverID; + bool bSenderHigh = uSenderID > uReceiverID; uint256 uIndex = Ledger::getRippleStateIndex(uSenderID, uReceiverID, saAmount.getCurrency()); SLE::pointer sleRippleState = entryCache(ltRIPPLE_STATE, uIndex); @@ -1147,6 +1210,7 @@ void LedgerEntrySet::rippleCredit(const uint160& uSenderID, const uint160& uRece if (!sleRippleState) { + STAmount saSrcLimit = STAmount(uCurrencyID, uSenderID); STAmount saBalance = saAmount; saBalance.setIssuer(ACCOUNT_ONE); @@ -1157,20 +1221,21 @@ void LedgerEntrySet::rippleCredit(const uint160& uSenderID, const uint160& uRece % RippleAddress::createHumanAccountID(uReceiverID) % saAmount.getFullText()); - sleRippleState = entryCreate(ltRIPPLE_STATE, uIndex); - - if (!bFlipped) - saBalance.negate(); - - sleRippleState->setFieldAmount(sfBalance, saBalance); - sleRippleState->setFieldAmount(bFlipped ? sfHighLimit : sfLowLimit, STAmount(uCurrencyID, uSenderID)); - sleRippleState->setFieldAmount(bFlipped ? sfLowLimit : sfHighLimit, STAmount(uCurrencyID, uReceiverID)); + // XXX Pass back result. + TER terResult = trustCreate( + bSenderHigh, + uSenderID, + entryCache(ltACCOUNT_ROOT, Ledger::getAccountRootIndex(uSenderID)), + uReceiverID, + uIndex, + saBalance, + saSrcLimit); } else { STAmount saBalance = sleRippleState->getFieldAmount(sfBalance); - if (!bFlipped) + if (!bSenderHigh) saBalance.negate(); // Put balance in low terms. cLog(lsDEBUG) << boost::str(boost::format("rippleCredit> %s (%s) -> %s : %s") @@ -1181,7 +1246,7 @@ void LedgerEntrySet::rippleCredit(const uint160& uSenderID, const uint160& uRece saBalance += saAmount; - if (!bFlipped) + if (!bSenderHigh) saBalance.negate(); sleRippleState->setFieldAmount(sfBalance, saBalance); diff --git a/src/cpp/ripple/LedgerEntrySet.h b/src/cpp/ripple/LedgerEntrySet.h index 5faf184764..59a1a2944a 100644 --- a/src/cpp/ripple/LedgerEntrySet.h +++ b/src/cpp/ripple/LedgerEntrySet.h @@ -128,6 +128,17 @@ public: STAmount accountFunds(const uint160& uAccountID, const STAmount& saDefault, bool bAvail=false); void accountSend(const uint160& uSenderID, const uint160& uReceiverID, const STAmount& saAmount); + TER trustCreate( + const bool bSrcHigh, + const uint160& uSrcAccountID, + SLE::ref sleSrcAccount, + const uint160& uDstAccountID, + const uint256& uIndex, + const STAmount& saSrcBalance, + const STAmount& saSrcLimit, + const uint32 uSrcQualityIn = 0, + const uint32 uSrcQualityOut = 0); + Json::Value getJson(int) const; void calcRawMeta(Serializer&, TER result, uint32 index); diff --git a/src/cpp/ripple/LedgerFormats.cpp b/src/cpp/ripple/LedgerFormats.cpp index 2b9f01e80c..cca4ba8c0e 100644 --- a/src/cpp/ripple/LedgerFormats.cpp +++ b/src/cpp/ripple/LedgerFormats.cpp @@ -87,8 +87,10 @@ static bool LEFInit() << SOElement(sfHighLimit, SOE_REQUIRED) << SOElement(sfPreviousTxnID, SOE_REQUIRED) << SOElement(sfPreviousTxnLgrSeq, SOE_REQUIRED) + << SOElement(sfLowNode, SOE_OPTIONAL) << SOElement(sfLowQualityIn, SOE_OPTIONAL) << SOElement(sfLowQualityOut, SOE_OPTIONAL) + << SOElement(sfHighNode, SOE_OPTIONAL) << SOElement(sfHighQualityIn, SOE_OPTIONAL) << SOElement(sfHighQualityOut, SOE_OPTIONAL) ; diff --git a/src/cpp/ripple/SerializeProto.h b/src/cpp/ripple/SerializeProto.h index 6b3d661150..cc8a932b7b 100644 --- a/src/cpp/ripple/SerializeProto.h +++ b/src/cpp/ripple/SerializeProto.h @@ -71,6 +71,8 @@ FIELD(OwnerNode, UINT64, 4) FIELD(BaseFee, UINT64, 5) FIELD(ExchangeRate, UINT64, 6) + FIELD(LowNode, UINT64, 7) + FIELD(HighNode, UINT64, 8) // 128-bit diff --git a/src/cpp/ripple/TransactionErr.cpp b/src/cpp/ripple/TransactionErr.cpp index e196f3a614..8e4c5e8acf 100644 --- a/src/cpp/ripple/TransactionErr.cpp +++ b/src/cpp/ripple/TransactionErr.cpp @@ -41,6 +41,7 @@ bool transResultInfo(TER terCode, std::string& strToken, std::string& strHuman) { temBAD_AUTH_MASTER, "temBAD_AUTH_MASTER", "Auth for unclaimed account needs correct master key." }, { temBAD_EXPIRATION, "temBAD_EXPIRATION", "Malformed." }, { temBAD_ISSUER, "temBAD_ISSUER", "Malformed." }, + { temBAD_LIMIT, "temBAD_LIMIT", "Limits must be non-negative." }, { temBAD_OFFER, "temBAD_OFFER", "Malformed." }, { temBAD_PATH, "temBAD_PATH", "Malformed." }, { temBAD_PATH_LOOP, "temBAD_PATH_LOOP", "Malformed." }, diff --git a/src/cpp/ripple/TransactionErr.h b/src/cpp/ripple/TransactionErr.h index b09dea4a6f..e7a667b073 100644 --- a/src/cpp/ripple/TransactionErr.h +++ b/src/cpp/ripple/TransactionErr.h @@ -29,6 +29,7 @@ enum TER // aka TransactionEngineResult temBAD_AUTH_MASTER, temBAD_EXPIRATION, temBAD_ISSUER, + temBAD_LIMIT, temBAD_OFFER, temBAD_PATH, temBAD_PATH_LOOP, diff --git a/src/cpp/ripple/Transactor.cpp b/src/cpp/ripple/Transactor.cpp index 7b0091a3ac..cc76d26e56 100644 --- a/src/cpp/ripple/Transactor.cpp +++ b/src/cpp/ripple/Transactor.cpp @@ -77,7 +77,6 @@ TER Transactor::payFee() return tesSUCCESS; } - TER Transactor::checkSig() { // Consistency: Check signature @@ -199,14 +198,14 @@ TER Transactor::apply() mHasAuthKey = mTxnAccount->isFieldPresent(sfRegularKey); } - terResult=payFee(); - if(terResult != tesSUCCESS) return(terResult); + terResult = payFee(); + if (terResult != tesSUCCESS) return(terResult); - terResult=checkSig(); - if(terResult != tesSUCCESS) return(terResult); + terResult = checkSig(); + if (terResult != tesSUCCESS) return(terResult); - terResult=checkSeq(); - if(terResult != tesSUCCESS) return(terResult); + terResult = checkSeq(); + if (terResult != tesSUCCESS) return(terResult); mEngine->entryModify(mTxnAccount); diff --git a/src/cpp/ripple/TrustSetTransactor.cpp b/src/cpp/ripple/TrustSetTransactor.cpp index a3239f23dd..abb6cfbc05 100644 --- a/src/cpp/ripple/TrustSetTransactor.cpp +++ b/src/cpp/ripple/TrustSetTransactor.cpp @@ -2,8 +2,6 @@ #include "TrustSetTransactor.h" -#include - SETUP_LOG(); TER TrustSetTransactor::doApply() @@ -33,7 +31,7 @@ TER TrustSetTransactor::doApply() { cLog(lsINFO) << "doTrustSet: Malformed transaction: Negatived credit limit."; - return temBAD_AMOUNT; + return temBAD_LIMIT; } else if (!uDstAccountID || uDstAccountID == ACCOUNT_ONE) { @@ -223,16 +221,20 @@ TER TrustSetTransactor::doApply() { // Can delete. - uint64 uSrcRef; // <-- Ignored, dirs never delete. + uint64 uLowNode = sleRippleState->getFieldU64(sfLowNode); + uint64 uHighNode = sleRippleState->getFieldU64(sfHighNode); - terResult = mEngine->getNodes().dirDelete(false, uSrcRef, Ledger::getOwnerDirIndex(uLowAccountID), sleRippleState->getIndex(), false); + cLog(lsTRACE) << "doTrustSet: Deleting ripple line: low"; + terResult = mEngine->getNodes().dirDelete(false, uLowNode, Ledger::getOwnerDirIndex(uLowAccountID), sleRippleState->getIndex(), false); if (tesSUCCESS == terResult) - terResult = mEngine->getNodes().dirDelete(false, uSrcRef, Ledger::getOwnerDirIndex(uHighAccountID), sleRippleState->getIndex(), false); + { + cLog(lsTRACE) << "doTrustSet: Deleting ripple line: high"; + terResult = mEngine->getNodes().dirDelete(false, uHighNode, Ledger::getOwnerDirIndex(uHighAccountID), sleRippleState->getIndex(), false); + } + cLog(lsINFO) << "doTrustSet: Deleting ripple line: state"; mEngine->entryDelete(sleRippleState); - - cLog(lsINFO) << "doTrustSet: Deleting ripple line"; } else if (bReserveIncrease && saSrcXRPBalance.getNValue() < uReserveCreate) // Reserve is not scaled by load. @@ -267,41 +269,22 @@ TER TrustSetTransactor::doApply() } else { + STAmount saBalance = STAmount(uCurrencyID, ACCOUNT_ONE); // Zero balance in currency. + + cLog(lsINFO) << "doTrustSet: Creating ripple line: " + << Ledger::getRippleStateIndex(mTxnAccountID, uDstAccountID, uCurrencyID).ToString(); + // Create a new ripple line. - sleRippleState = mEngine->entryCreate(ltRIPPLE_STATE, Ledger::getRippleStateIndex(mTxnAccountID, uDstAccountID, uCurrencyID)); - - cLog(lsINFO) << "doTrustSet: Creating ripple line: " << sleRippleState->getIndex().ToString(); - - sleRippleState->setFieldAmount(sfBalance, STAmount(uCurrencyID, ACCOUNT_ONE)); // Zero balance in currency. - sleRippleState->setFieldAmount(!bHigh ? sfLowLimit : sfHighLimit, saLimitAllow); - sleRippleState->setFieldAmount( bHigh ? sfLowLimit : sfHighLimit, STAmount(uCurrencyID, uDstAccountID)); - - if (uQualityIn) - sleRippleState->setFieldU32(!bHigh ? sfLowQualityIn : sfHighQualityIn, uQualityIn); - - if (uQualityOut) - sleRippleState->setFieldU32(!bHigh ? sfLowQualityOut : sfHighQualityOut, uQualityOut); - - sleRippleState->setFieldU32(sfFlags, !bHigh ? lsfLowReserve : lsfHighReserve); - - uint64 uSrcRef; // <-- Ignored, dirs never delete. - - terResult = mEngine->getNodes().dirAdd( - uSrcRef, - Ledger::getOwnerDirIndex(mTxnAccountID), - sleRippleState->getIndex(), - boost::bind(&Ledger::ownerDirDescriber, _1, mTxnAccountID)); - - if (tesSUCCESS == terResult) - { - mEngine->getNodes().ownerCountAdjust(mTxnAccountID, 1, mTxnAccount); - - terResult = mEngine->getNodes().dirAdd( - uSrcRef, - Ledger::getOwnerDirIndex(uDstAccountID), - sleRippleState->getIndex(), - boost::bind(&Ledger::ownerDirDescriber, _1, uDstAccountID)); - } + terResult = mEngine->getNodes().trustCreate( + bHigh, // Who to charge with reserve. + mTxnAccountID, + mTxnAccount, + uDstAccountID, + Ledger::getRippleStateIndex(mTxnAccountID, uDstAccountID, uCurrencyID), + saBalance, + saLimitAllow, + uQualityIn, + uQualityOut); } cLog(lsINFO) << "doTrustSet<"; diff --git a/test/send-test.js b/test/send-test.js index 547a59a41c..2322da8a92 100644 --- a/test/send-test.js +++ b/test/send-test.js @@ -40,8 +40,9 @@ buster.testCase("Fee Changes", { */ buster.testCase("Sending", { - 'setUp' : testutils.build_setup(), // - 'tearDown' : testutils.build_teardown(), + 'setUp' : testutils.build_setup(), + // 'setUp' : testutils.build_setup({verbose: true , no_server: true}), + 'tearDown' : testutils.build_teardown(), "send XRP to non-existent account with insufficent fee" : function (done) { @@ -185,14 +186,25 @@ buster.testCase("Sending", { self.remote.transaction() .ripple_line_set("alice", "-1/USD/mtgox") .on('proposed', function (m) { - buster.assert.equals('temBAD_AMOUNT', m.result); + buster.assert.equals('temBAD_LIMIT', m.result); // After a malformed transaction, need to recover correct sequence. self.remote.set_account_seq("alice", self.remote.account_seq("alice")-1); - callback('temBAD_AMOUNT' !== m.result); + callback('temBAD_LIMIT' !== m.result); }) .submit(); }, +// function (callback) { +// self.what = "Display ledger"; +// +// self.remote.request_ledger('current', true) +// .on('success', function (m) { +// console.log("Ledger: %s", JSON.stringify(m, undefined, 2)); +// +// callback(); +// }) +// .request(); +// }, function (callback) { self.what = "Zero a credit limit.";