From 96bb0bf5324df423e893a91006878e94607ef826 Mon Sep 17 00:00:00 2001 From: Jon Jove Date: Tue, 5 Feb 2019 09:22:44 -0600 Subject: [PATCH 1/7] Add a simple loadBestOffers benchmark --- src/ledger/LedgerTxnTests.cpp | 172 ++++++++++++++++++++++++++++++++++ 1 file changed, 172 insertions(+) diff --git a/src/ledger/LedgerTxnTests.cpp b/src/ledger/LedgerTxnTests.cpp index 2538d07432..1490ffa66d 100644 --- a/src/ledger/LedgerTxnTests.cpp +++ b/src/ledger/LedgerTxnTests.cpp @@ -2434,3 +2434,175 @@ TEST_CASE("Signers performance benchmark", "[!hide][signersbench]") } #endif } + +TEST_CASE("Load best offers benchmark", "[!hide][bestoffersbench]") +{ + auto getTimeScope = [](Application& app, std::string const& phase) { + return app.getMetrics() + .NewTimer({"bestoffers", "benchmark", phase}) + .TimeScope(); + }; + + auto getTimeSpent = [](Application& app, std::string const& phase) { + auto time = + app.getMetrics().NewTimer({"bestoffers", "benchmark", phase}).sum(); + return phase + ": " + std::to_string(time) + " ms"; + }; + + auto generateAssets = [](size_t numAssets, size_t numIssuers) { + CLOG(WARNING, "Ledger") << "Generating issuers"; + REQUIRE(numIssuers >= 1); + std::vector issuers; + for (size_t i = 1; i < numIssuers; ++i) + { + issuers.emplace_back(autocheck::generator()(5)); + } + + CLOG(WARNING, "Ledger") << "Generating assets"; + REQUIRE(numAssets >= 2); + std::vector assets; + assets.emplace_back(ASSET_TYPE_NATIVE); + for (size_t i = 1; i < numAssets; ++i) + { + size_t issuerIndex = + autocheck::generator()(issuers.size() - 1); + REQUIRE(issuerIndex < issuers.size()); + + Asset a(ASSET_TYPE_CREDIT_ALPHANUM4); + strToAssetCode(a.alphaNum4().assetCode, "A" + std::to_string(i)); + a.alphaNum4().issuer = issuers[issuerIndex]; + assets.emplace_back(a); + } + return assets; + }; + + auto selectAssetPair = [](std::vector const& assets) { + REQUIRE(assets.size() >= 2); + size_t maxIndex = assets.size() - 1; + + size_t firstIndex = autocheck::generator()(maxIndex); + size_t secondIndex; + do + { + secondIndex = autocheck::generator()(maxIndex); + } while (firstIndex == secondIndex); + + return std::make_pair(assets[firstIndex], assets[secondIndex]); + }; + + auto generateEntries = [&](size_t numOffers, + std::vector const& assets) { + CLOG(WARNING, "Ledger") << "Generating offers"; + std::vector offers; + offers.reserve(numOffers); + for (size_t i = 0; i < numOffers; ++i) + { + LedgerEntry le; + le.data.type(OFFER); + le.lastModifiedLedgerSeq = 1; + le.data.offer() = LedgerTestUtils::generateValidOfferEntry(); + + auto& oe = le.data.offer(); + auto assetPair = selectAssetPair(assets); + oe.selling = assetPair.first; + oe.buying = assetPair.second; + + offers.emplace_back(le); + } + return offers; + }; + + auto writeEntries = [&](Application& app, + std::vector const& offers) { + LedgerTxn ltx(app.getLedgerTxnRoot()); + { + CLOG(WARNING, "Ledger") << "Creating offers"; + auto timer = getTimeScope(app, "create"); + for (auto const& le : offers) + { + ltx.create(le); + } + } + + { + CLOG(WARNING, "Ledger") << "Writing offers"; + auto timer = getTimeScope(app, "write"); + ltx.commit(); + } + }; + + auto loadBestOffers = [&](Application& app, Asset const& buying, + Asset const& selling, + std::vector const& sortedOffers) { + auto timer = getTimeScope(app, "load"); + + size_t numOffers = 0; + LedgerTxn ltx(app.getLedgerTxnRoot()); + while (auto le = ltx.loadBestOffer(buying, selling)) + { + REQUIRE(le.current() == sortedOffers[numOffers]); + ++numOffers; + le.erase(); + } + }; + + auto runTest = [&](Config::TestDbMode mode, size_t numAssets, + size_t numIssuers, size_t numOffers) { + VirtualClock clock; + Config cfg(getTestConfig(0, mode)); + cfg.ENTRY_CACHE_SIZE = 100000; + cfg.BEST_OFFERS_CACHE_SIZE = 1000; + Application::pointer app = createTestApplication(clock, cfg); + + CLOG(WARNING, "Ledger") + << "Generating " << numOffers << " offers buying and selling " + << numAssets << " assets with " << numIssuers << " issuers"; + auto assets = generateAssets(numAssets, numIssuers); + auto offers = generateEntries(numOffers, assets); + + std::map, std::vector> + sortedOffers; + for (auto const& offer : offers) + { + auto const& oe = offer.data.offer(); + auto& vec = sortedOffers[std::make_pair(oe.buying, oe.selling)]; + vec.emplace_back(offer); + } + for (auto& kv : sortedOffers) + { + std::sort(kv.second.begin(), kv.second.end(), isBetterOffer); + } + + writeEntries(*app, offers); + + CLOG(WARNING, "Ledger") << "Loading best offers"; + for (auto const& buying : assets) + { + for (auto const& selling : assets) + { + if (!(buying == selling)) + { + loadBestOffers( + *app, buying, selling, + sortedOffers[std::make_pair(buying, selling)]); + } + } + } + + CLOG(WARNING, "Ledger") << "Done (" << getTimeSpent(*app, "create") + << ", " << getTimeSpent(*app, "write") << ", " + << getTimeSpent(*app, "load") << ")"; + }; + +#ifdef USE_POSTGRES + SECTION("postgres") + { + runTest(Config::TESTDB_POSTGRESQL, 10, 5, 25000); + } +#endif + + SECTION("sqlite") + { + runTest(Config::TESTDB_ON_DISK_SQLITE, 10, 5, 25000); + } +} From 9c6581310ecef30733244cd4118eb5c1033b72e0 Mon Sep 17 00:00:00 2001 From: Jon Jove Date: Tue, 5 Feb 2019 09:24:23 -0600 Subject: [PATCH 2/7] LedgerTxnRoot::getBestOffer populates entry cache --- src/ledger/LedgerTxn.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ledger/LedgerTxn.cpp b/src/ledger/LedgerTxn.cpp index 83df4c5e5a..bd56406d02 100644 --- a/src/ledger/LedgerTxn.cpp +++ b/src/ledger/LedgerTxn.cpp @@ -1657,6 +1657,11 @@ LedgerTxnRoot::Impl::getBestOffer(Asset const& buying, Asset const& selling, } res = findIncludedOffer(newOfferIter, offers.cend(), exclude); } + + if (res) + { + putInEntryCache(getEntryCacheKey(LedgerEntryKey(*res)), res); + } return res; } From e1b54aefc3acaa5d1c5e82e5a5a034d55b8ec82a Mon Sep 17 00:00:00 2001 From: Jon Jove Date: Tue, 5 Feb 2019 09:24:37 -0600 Subject: [PATCH 3/7] Improve offers schema for loadBestOffers --- src/database/Database.cpp | 3 + src/ledger/LedgerTxn.cpp | 6 + src/ledger/LedgerTxn.h | 2 + src/ledger/LedgerTxnImpl.h | 6 + src/ledger/LedgerTxnOfferSQL.cpp | 622 ++++++++++++++----------------- 5 files changed, 293 insertions(+), 346 deletions(-) diff --git a/src/database/Database.cpp b/src/database/Database.cpp index 8ad761ddfa..0cd349b35d 100644 --- a/src/database/Database.cpp +++ b/src/database/Database.cpp @@ -214,6 +214,9 @@ Database::applySchemaUpgrade(unsigned long vers) // Update schema for base-64 encoding mApp.getLedgerTxnRoot().encodeDataNamesBase64(); mApp.getLedgerTxnRoot().encodeHomeDomainsBase64(); + + // Update schema for simplified offers table + mApp.getLedgerTxnRoot().writeOffersIntoSimplifiedOffersTable(); break; default: diff --git a/src/ledger/LedgerTxn.cpp b/src/ledger/LedgerTxn.cpp index bd56406d02..fc9e4688e4 100644 --- a/src/ledger/LedgerTxn.cpp +++ b/src/ledger/LedgerTxn.cpp @@ -1896,4 +1896,10 @@ LedgerTxnRoot::encodeHomeDomainsBase64() { mImpl->encodeHomeDomainsBase64(); } + +void +LedgerTxnRoot::writeOffersIntoSimplifiedOffersTable() +{ + mImpl->writeOffersIntoSimplifiedOffersTable(); +} } diff --git a/src/ledger/LedgerTxn.h b/src/ledger/LedgerTxn.h index 8275caf2d7..962f3190ba 100644 --- a/src/ledger/LedgerTxn.h +++ b/src/ledger/LedgerTxn.h @@ -598,5 +598,7 @@ class LedgerTxnRoot : public AbstractLedgerTxnParent void writeSignersTableIntoAccountsTable(); void encodeDataNamesBase64(); void encodeHomeDomainsBase64(); + + void writeOffersIntoSimplifiedOffersTable(); }; } diff --git a/src/ledger/LedgerTxnImpl.h b/src/ledger/LedgerTxnImpl.h index 2d3af028f8..f89c2f68a5 100644 --- a/src/ledger/LedgerTxnImpl.h +++ b/src/ledger/LedgerTxnImpl.h @@ -554,6 +554,12 @@ class LedgerTxnRoot::Impl // - the prepared statement cache may be, but is not guaranteed to be, // modified void encodeHomeDomainsBase64(); + + // writeOffersIntoSimplifiedOffersTable has the basic exception safety + // guarantee. If it throws an exception, then + // - the prepared statement cache may be, but is not guaranteed to be, + // modified + void writeOffersIntoSimplifiedOffersTable(); }; #ifdef USE_POSTGRES diff --git a/src/ledger/LedgerTxnOfferSQL.cpp b/src/ledger/LedgerTxnOfferSQL.cpp index ed38fc01eb..e94072d569 100644 --- a/src/ledger/LedgerTxnOfferSQL.cpp +++ b/src/ledger/LedgerTxnOfferSQL.cpp @@ -7,84 +7,22 @@ #include "database/Database.h" #include "database/DatabaseTypeSpecificOperation.h" #include "ledger/LedgerTxnImpl.h" +#include "util/Decoder.h" +#include "util/Logging.h" #include "util/XDROperators.h" #include "util/types.h" +#include "xdrpp/marshal.h" namespace stellar { -void -getAssetStrings(Asset const& asset, std::string& assetCodeStr, - std::string& issuerStr, soci::indicator& assetCodeIndicator, - soci::indicator& issuerIndicator) -{ - if (asset.type() == ASSET_TYPE_CREDIT_ALPHANUM4) - { - assetCodeToStr(asset.alphaNum4().assetCode, assetCodeStr); - issuerStr = KeyUtils::toStrKey(asset.alphaNum4().issuer); - assetCodeIndicator = soci::i_ok; - issuerIndicator = soci::i_ok; - } - else if (asset.type() == ASSET_TYPE_CREDIT_ALPHANUM12) - { - assetCodeToStr(asset.alphaNum12().assetCode, assetCodeStr); - issuerStr = KeyUtils::toStrKey(asset.alphaNum12().issuer); - assetCodeIndicator = soci::i_ok; - issuerIndicator = soci::i_ok; - } - else - { - assert(asset.type() == ASSET_TYPE_NATIVE); - assetCodeStr = ""; - issuerStr = ""; - assetCodeIndicator = soci::i_null; - issuerIndicator = soci::i_null; - } -} - -void -processAsset(Asset& asset, AssetType assetType, std::string const& issuerStr, - soci::indicator const& issuerIndicator, - std::string const& assetCode, - soci::indicator const& assetCodeIndicator) -{ - asset.type(assetType); - if (assetType != ASSET_TYPE_NATIVE) - { - if ((assetCodeIndicator != soci::i_ok) || - (issuerIndicator != soci::i_ok)) - { - throw std::runtime_error("bad database state"); - } - - if (assetType == ASSET_TYPE_CREDIT_ALPHANUM12) - { - asset.alphaNum12().issuer = - KeyUtils::fromStrKey(issuerStr); - strToAssetCode(asset.alphaNum12().assetCode, assetCode); - } - else if (assetType == ASSET_TYPE_CREDIT_ALPHANUM4) - { - asset.alphaNum4().issuer = - KeyUtils::fromStrKey(issuerStr); - strToAssetCode(asset.alphaNum4().assetCode, assetCode); - } - else - { - throw std::runtime_error("bad database state"); - } - } -} - std::shared_ptr LedgerTxnRoot::Impl::loadOffer(LedgerKey const& key) const { uint64_t offerID = key.offer().offerID; std::string actIDStrKey = KeyUtils::toStrKey(key.offer().sellerID); - std::string sql = "SELECT sellerid, offerid, " - "sellingassettype, sellingassetcode, sellingissuer, " - "buyingassettype, buyingassetcode, buyingissuer, " + std::string sql = "SELECT sellerid, offerid, sellingasset, buyingasset, " "amount, pricen, priced, flags, lastmodified " "FROM offers " "WHERE sellerid= :id AND offerid= :offerid"; @@ -108,9 +46,7 @@ LedgerTxnRoot::Impl::loadOffer(LedgerKey const& key) const std::vector LedgerTxnRoot::Impl::loadAllOffers() const { - std::string sql = "SELECT sellerid, offerid, " - "sellingassettype, sellingassetcode, sellingissuer, " - "buyingassettype, buyingassetcode, buyingissuer, " + std::string sql = "SELECT sellerid, offerid, sellingasset, buyingasset, " "amount, pricen, priced, flags, lastmodified " "FROM offers"; auto prep = mDatabase.getPreparedStatement(sql); @@ -128,61 +64,14 @@ LedgerTxnRoot::Impl::loadBestOffers(std::list& offers, Asset const& buying, Asset const& selling, size_t numOffers, size_t offset) const { - std::string sql = "SELECT sellerid, offerid, " - "sellingassettype, sellingassetcode, sellingissuer, " - "buyingassettype, buyingassetcode, buyingissuer, " + std::string sql = "SELECT sellerid, offerid, sellingasset, buyingasset, " "amount, pricen, priced, flags, lastmodified " - "FROM offers "; + "FROM offers " + "WHERE sellingasset = :v1 AND buyingasset = :v2"; - std::string sellingAssetCode, sellingIssuerStrKey; - if (selling.type() == ASSET_TYPE_NATIVE) - { - sql += " WHERE sellingassettype = 0 AND sellingissuer IS NULL"; - } - else - { - if (selling.type() == ASSET_TYPE_CREDIT_ALPHANUM4) - { - assetCodeToStr(selling.alphaNum4().assetCode, sellingAssetCode); - sellingIssuerStrKey = - KeyUtils::toStrKey(selling.alphaNum4().issuer); - } - else if (selling.type() == ASSET_TYPE_CREDIT_ALPHANUM12) - { - assetCodeToStr(selling.alphaNum12().assetCode, sellingAssetCode); - sellingIssuerStrKey = - KeyUtils::toStrKey(selling.alphaNum12().issuer); - } - else - { - throw std::runtime_error("unknown asset type"); - } - sql += " WHERE sellingassetcode = :sac AND sellingissuer = :si"; - } - - std::string buyingAssetCode, buyingIssuerStrKey; - if (buying.type() == ASSET_TYPE_NATIVE) - { - sql += " AND buyingassettype = 0 AND buyingissuer IS NULL"; - } - else - { - if (buying.type() == ASSET_TYPE_CREDIT_ALPHANUM4) - { - assetCodeToStr(buying.alphaNum4().assetCode, buyingAssetCode); - buyingIssuerStrKey = KeyUtils::toStrKey(buying.alphaNum4().issuer); - } - else if (buying.type() == ASSET_TYPE_CREDIT_ALPHANUM12) - { - assetCodeToStr(buying.alphaNum12().assetCode, buyingAssetCode); - buyingIssuerStrKey = KeyUtils::toStrKey(buying.alphaNum12().issuer); - } - else - { - throw std::runtime_error("unknown asset type"); - } - sql += " AND buyingassetcode = :bac AND buyingissuer = :bi"; - } + std::string buyingAsset, sellingAsset; + buyingAsset = decoder::encode_b64(xdr::xdr_to_opaque(buying)); + sellingAsset = decoder::encode_b64(xdr::xdr_to_opaque(selling)); // price is an approximation of the actual n/d (truncated math, 15 digits) // ordering by offerid gives precendence to older offers for fairness @@ -190,18 +79,10 @@ LedgerTxnRoot::Impl::loadBestOffers(std::list& offers, auto prep = mDatabase.getPreparedStatement(sql); auto& st = prep.statement(); - if (selling.type() != ASSET_TYPE_NATIVE) - { - st.exchange(soci::use(sellingAssetCode, "sac")); - st.exchange(soci::use(sellingIssuerStrKey, "si")); - } - if (buying.type() != ASSET_TYPE_NATIVE) - { - st.exchange(soci::use(buyingAssetCode, "bac")); - st.exchange(soci::use(buyingIssuerStrKey, "bi")); - } - st.exchange(soci::use(numOffers, "n")); - st.exchange(soci::use(offset, "o")); + st.exchange(soci::use(sellingAsset)); + st.exchange(soci::use(buyingAsset)); + st.exchange(soci::use(numOffers)); + st.exchange(soci::use(offset)); { auto timer = mDatabase.getSelectTimer("offer"); @@ -238,45 +119,30 @@ isBetterOffer(LedgerEntry const& lhsEntry, LedgerEntry const& rhsEntry) // Note: This function is currently only used in AllowTrustOpFrame, which means // the asset parameter will never satisfy asset.type() == ASSET_TYPE_NATIVE. As -// a consequence, I have not implemented that possibility so this function -// throws in that case. +// a consequence, this function throws in that case. std::vector LedgerTxnRoot::Impl::loadOffersByAccountAndAsset(AccountID const& accountID, Asset const& asset) const { - std::string sql = "SELECT sellerid, offerid, " - "sellingassettype, sellingassetcode, sellingissuer, " - "buyingassettype, buyingassetcode, buyingissuer, " + std::string sql = "SELECT sellerid, offerid, sellingasset, buyingasset, " "amount, pricen, priced, flags, lastmodified " - "FROM offers "; - sql += " WHERE sellerid = :acc" - " AND ((sellingassetcode = :code AND sellingissuer = :iss)" - " OR (buyingassetcode = :code AND buyingissuer = :iss))"; + "FROM offers WHERE sellerid = :v1 AND " + "(sellingasset = :v2 OR buyingasset = :v3)"; + // Note: v2 == v3 but positional parameters are faster std::string accountStr = KeyUtils::toStrKey(accountID); - std::string assetCode; - std::string assetIssuer; - if (asset.type() == ASSET_TYPE_CREDIT_ALPHANUM4) - { - assetCodeToStr(asset.alphaNum4().assetCode, assetCode); - assetIssuer = KeyUtils::toStrKey(asset.alphaNum4().issuer); - } - else if (asset.type() == ASSET_TYPE_CREDIT_ALPHANUM12) - { - assetCodeToStr(asset.alphaNum12().assetCode, assetCode); - assetIssuer = KeyUtils::toStrKey(asset.alphaNum12().issuer); - } - else + if (asset.type() == ASSET_TYPE_NATIVE) { throw std::runtime_error("Invalid asset type"); } + std::string assetStr = decoder::encode_b64(xdr::xdr_to_opaque(asset)); auto prep = mDatabase.getPreparedStatement(sql); auto& st = prep.statement(); - st.exchange(soci::use(accountStr, "acc")); - st.exchange(soci::use(assetCode, "code")); - st.exchange(soci::use(assetIssuer, "iss")); + st.exchange(soci::use(accountStr)); + st.exchange(soci::use(assetStr)); + st.exchange(soci::use(assetStr)); std::vector offers; { @@ -286,17 +152,22 @@ LedgerTxnRoot::Impl::loadOffersByAccountAndAsset(AccountID const& accountID, return offers; } -std::vector -LedgerTxnRoot::Impl::loadOffers(StatementContext& prep) const +static Asset +processAsset(std::string const& asset) { - std::vector offers; + Asset res; + std::vector assetOpaque; + decoder::decode_b64(asset, assetOpaque); + xdr::xdr_from_opaque(assetOpaque, res); + return res; +} +std::list::const_iterator +LedgerTxnRoot::Impl::loadOffers(StatementContext& prep, + std::list& offers) const +{ std::string actIDStrKey; - unsigned int sellingAssetType, buyingAssetType; - std::string sellingAssetCode, buyingAssetCode, sellingIssuerStrKey, - buyingIssuerStrKey; - soci::indicator sellingAssetCodeIndicator, buyingAssetCodeIndicator, - sellingIssuerIndicator, buyingIssuerIndicator; + std::string sellingAsset, buyingAsset; LedgerEntry le; le.data.type(OFFER); @@ -305,12 +176,8 @@ LedgerTxnRoot::Impl::loadOffers(StatementContext& prep) const auto& st = prep.statement(); st.exchange(soci::into(actIDStrKey)); st.exchange(soci::into(oe.offerID)); - st.exchange(soci::into(sellingAssetType)); - st.exchange(soci::into(sellingAssetCode, sellingAssetCodeIndicator)); - st.exchange(soci::into(sellingIssuerStrKey, sellingIssuerIndicator)); - st.exchange(soci::into(buyingAssetType)); - st.exchange(soci::into(buyingAssetCode, buyingAssetCodeIndicator)); - st.exchange(soci::into(buyingIssuerStrKey, buyingIssuerIndicator)); + st.exchange(soci::into(sellingAsset)); + st.exchange(soci::into(buyingAsset)); st.exchange(soci::into(oe.amount)); st.exchange(soci::into(oe.price.n)); st.exchange(soci::into(oe.price.d)); @@ -318,33 +185,35 @@ LedgerTxnRoot::Impl::loadOffers(StatementContext& prep) const st.exchange(soci::into(le.lastModifiedLedgerSeq)); st.define_and_bind(); st.execute(true); + + auto iterNext = offers.cend(); while (st.got_data()) { oe.sellerID = KeyUtils::fromStrKey(actIDStrKey); - processAsset(oe.selling, (AssetType)sellingAssetType, - sellingIssuerStrKey, sellingIssuerIndicator, - sellingAssetCode, sellingAssetCodeIndicator); - processAsset(oe.buying, (AssetType)buyingAssetType, buyingIssuerStrKey, - buyingIssuerIndicator, buyingAssetCode, - buyingAssetCodeIndicator); + oe.selling = processAsset(sellingAsset); + oe.buying = processAsset(buyingAsset); - offers.emplace_back(le); + if (iterNext == offers.cend()) + { + iterNext = offers.emplace(iterNext, le); + } + else + { + offers.emplace_back(le); + } st.fetch(); } - return offers; + return iterNext; } -std::list::const_iterator -LedgerTxnRoot::Impl::loadOffers(StatementContext& prep, - std::list& offers) const +std::vector +LedgerTxnRoot::Impl::loadOffers(StatementContext& prep) const { + std::vector offers; + std::string actIDStrKey; - unsigned int sellingAssetType, buyingAssetType; - std::string sellingAssetCode, buyingAssetCode, sellingIssuerStrKey, - buyingIssuerStrKey; - soci::indicator sellingAssetCodeIndicator, buyingAssetCodeIndicator, - sellingIssuerIndicator, buyingIssuerIndicator; + std::string sellingAsset, buyingAsset; LedgerEntry le; le.data.type(OFFER); @@ -353,12 +222,8 @@ LedgerTxnRoot::Impl::loadOffers(StatementContext& prep, auto& st = prep.statement(); st.exchange(soci::into(actIDStrKey)); st.exchange(soci::into(oe.offerID)); - st.exchange(soci::into(sellingAssetType)); - st.exchange(soci::into(sellingAssetCode, sellingAssetCodeIndicator)); - st.exchange(soci::into(sellingIssuerStrKey, sellingIssuerIndicator)); - st.exchange(soci::into(buyingAssetType)); - st.exchange(soci::into(buyingAssetCode, buyingAssetCodeIndicator)); - st.exchange(soci::into(buyingIssuerStrKey, buyingIssuerIndicator)); + st.exchange(soci::into(sellingAsset)); + st.exchange(soci::into(buyingAsset)); st.exchange(soci::into(oe.amount)); st.exchange(soci::into(oe.price.n)); st.exchange(soci::into(oe.price.d)); @@ -367,29 +232,17 @@ LedgerTxnRoot::Impl::loadOffers(StatementContext& prep, st.define_and_bind(); st.execute(true); - auto iterNext = offers.cend(); while (st.got_data()) { oe.sellerID = KeyUtils::fromStrKey(actIDStrKey); - processAsset(oe.selling, (AssetType)sellingAssetType, - sellingIssuerStrKey, sellingIssuerIndicator, - sellingAssetCode, sellingAssetCodeIndicator); - processAsset(oe.buying, (AssetType)buyingAssetType, buyingIssuerStrKey, - buyingIssuerIndicator, buyingAssetCode, - buyingAssetCodeIndicator); + oe.selling = processAsset(sellingAsset); + oe.buying = processAsset(buyingAsset); - if (iterNext == offers.cend()) - { - iterNext = offers.emplace(iterNext, le); - } - else - { - offers.emplace_back(le); - } + offers.emplace_back(le); st.fetch(); } - return iterNext; + return offers; } class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation @@ -397,16 +250,8 @@ class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation Database& mDB; std::vector mSellerIDs; std::vector mOfferIDs; - std::vector mSellingAssetTypes; - std::vector mSellingAssetCodes; - std::vector mSellingIssuers; - std::vector mSellingAssetCodeInds; - std::vector mSellingIssuerInds; - std::vector mBuyingAssetTypes; - std::vector mBuyingAssetCodes; - std::vector mBuyingIssuers; - std::vector mBuyingAssetCodeInds; - std::vector mBuyingIssuerInds; + std::vector mSellingAssets; + std::vector mBuyingAssets; std::vector mAmounts; std::vector mPriceNs; std::vector mPriceDs; @@ -414,19 +259,61 @@ class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation std::vector mFlags; std::vector mLastModifieds; + void + accumulateEntry(LedgerEntry const& entry) + { + assert(entry.data.type() == OFFER); + auto const& offer = entry.data.offer(); + + mSellerIDs.emplace_back(KeyUtils::toStrKey(offer.sellerID)); + mOfferIDs.emplace_back(offer.offerID); + + mSellingAssets.emplace_back( + decoder::encode_b64(xdr::xdr_to_opaque(offer.selling))); + mBuyingAssets.emplace_back( + decoder::encode_b64(xdr::xdr_to_opaque(offer.buying))); + + mAmounts.emplace_back(offer.amount); + mPriceNs.emplace_back(offer.price.n); + mPriceDs.emplace_back(offer.price.d); + double price = double(offer.price.n) / double(offer.price.d); + mPrices.emplace_back(price); + + mFlags.emplace_back(unsignedToSigned(offer.flags)); + mLastModifieds.emplace_back( + unsignedToSigned(entry.lastModifiedLedgerSeq)); + } + public: + BulkUpsertOffersOperation(Database& DB, + std::vector const& entries) + : mDB(DB) + { + mSellerIDs.reserve(entries.size()); + mOfferIDs.reserve(entries.size()); + mSellingAssets.reserve(entries.size()); + mBuyingAssets.reserve(entries.size()); + mAmounts.reserve(entries.size()); + mPriceNs.reserve(entries.size()); + mPriceDs.reserve(entries.size()); + mPrices.reserve(entries.size()); + mFlags.reserve(entries.size()); + mLastModifieds.reserve(entries.size()); + + for (auto const& e : entries) + { + accumulateEntry(e); + } + } + BulkUpsertOffersOperation(Database& DB, std::vector const& entries) : mDB(DB) { mSellerIDs.reserve(entries.size()); mOfferIDs.reserve(entries.size()); - mSellingAssetTypes.reserve(entries.size()); - mSellingAssetCodes.reserve(entries.size()); - mSellingIssuers.reserve(entries.size()); - mBuyingAssetTypes.reserve(entries.size()); - mBuyingAssetCodes.reserve(entries.size()); - mBuyingIssuers.reserve(entries.size()); + mSellingAssets.reserve(entries.size()); + mBuyingAssets.reserve(entries.size()); mAmounts.reserve(entries.size()); mPriceNs.reserve(entries.size()); mPriceDs.reserve(entries.size()); @@ -437,45 +324,7 @@ class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation for (auto const& e : entries) { assert(e.entryExists()); - assert(e.entry().data.type() == OFFER); - auto const& offer = e.entry().data.offer(); - std::string sellerIDStr, sellingIssuerStr, sellingAssetCodeStr, - buyingIssuerStr, buyingAssetCodeStr; - soci::indicator sellingIssuerInd, sellingAssetCodeInd, - buyingIssuerInd, buyingAssetCodeInd; - getAssetStrings(offer.selling, sellingAssetCodeStr, - sellingIssuerStr, sellingAssetCodeInd, - sellingIssuerInd); - getAssetStrings(offer.buying, buyingAssetCodeStr, buyingIssuerStr, - buyingAssetCodeInd, buyingIssuerInd); - - sellerIDStr = KeyUtils::toStrKey(offer.sellerID); - mSellerIDs.emplace_back(sellerIDStr); - mOfferIDs.emplace_back(offer.offerID); - - mSellingAssetTypes.emplace_back( - unsignedToSigned(static_cast(offer.selling.type()))); - mSellingAssetCodes.emplace_back(sellingAssetCodeStr); - mSellingIssuers.emplace_back(sellingIssuerStr); - mSellingAssetCodeInds.emplace_back(sellingAssetCodeInd); - mSellingIssuerInds.emplace_back(sellingIssuerInd); - - mBuyingAssetTypes.emplace_back( - unsignedToSigned(static_cast(offer.buying.type()))); - mBuyingAssetCodes.emplace_back(buyingAssetCodeStr); - mBuyingIssuers.emplace_back(buyingIssuerStr); - mBuyingAssetCodeInds.emplace_back(buyingAssetCodeInd); - mBuyingIssuerInds.emplace_back(buyingIssuerInd); - - mAmounts.emplace_back(offer.amount); - mPriceNs.emplace_back(offer.price.n); - mPriceDs.emplace_back(offer.price.d); - double price = double(offer.price.n) / double(offer.price.d); - mPrices.emplace_back(price); - - mFlags.emplace_back(unsignedToSigned(offer.flags)); - mLastModifieds.emplace_back( - unsignedToSigned(e.entry().lastModifiedLedgerSeq)); + accumulateEntry(e.entry()); } } @@ -483,21 +332,14 @@ class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation doSociGenericOperation() { std::string sql = "INSERT INTO offers ( " - "sellerid, offerid, " - "sellingassettype, sellingassetcode, sellingissuer, " - "buyingassettype, buyingassetcode, buyingissuer, " + "sellerid, offerid, sellingasset, buyingasset, " "amount, pricen, priced, price, flags, lastmodified " ") VALUES ( " - ":sellerid, :offerid, :v1, :v2, :v3, :v4, :v5, :v6, " - ":v7, :v8, :v9, :v10, :v11, :v12 " + ":v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10 " ") ON CONFLICT (offerid) DO UPDATE SET " "sellerid = excluded.sellerid, " - "sellingassettype = excluded.sellingassettype, " - "sellingassetcode = excluded.sellingassetcode, " - "sellingissuer = excluded.sellingissuer, " - "buyingassettype = excluded.buyingassettype, " - "buyingassetcode = excluded.buyingassetcode, " - "buyingissuer = excluded.buyingissuer, " + "sellingasset = excluded.sellingasset, " + "buyingasset = excluded.buyingasset, " "amount = excluded.amount, " "pricen = excluded.pricen, " "priced = excluded.priced, " @@ -508,12 +350,8 @@ class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation soci::statement& st = prep.statement(); st.exchange(soci::use(mSellerIDs)); st.exchange(soci::use(mOfferIDs)); - st.exchange(soci::use(mSellingAssetTypes)); - st.exchange(soci::use(mSellingAssetCodes, mSellingAssetCodeInds)); - st.exchange(soci::use(mSellingIssuers, mSellingIssuerInds)); - st.exchange(soci::use(mBuyingAssetTypes)); - st.exchange(soci::use(mBuyingAssetCodes, mBuyingAssetCodeInds)); - st.exchange(soci::use(mBuyingIssuers, mBuyingIssuerInds)); + st.exchange(soci::use(mSellingAssets)); + st.exchange(soci::use(mBuyingAssets)); st.exchange(soci::use(mAmounts)); st.exchange(soci::use(mPriceNs)); st.exchange(soci::use(mPriceDs)); @@ -542,26 +380,16 @@ class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation doPostgresSpecificOperation(soci::postgresql_session_backend* pg) override { - std::string strSellerIDs, strOfferIDs, strSellingAssetTypes, - strSellingAssetCodes, strSellingIssuers, strBuyingAssetTypes, - strBuyingAssetCodes, strBuyingIssuers, strAmounts, strPriceNs, - strPriceDs, strPrices, strFlags, strLastModifieds; + std::string strSellerIDs, strOfferIDs, strSellingAssets, + strBuyingAssets, strAmounts, strPriceNs, strPriceDs, strPrices, + strFlags, strLastModifieds; PGconn* conn = pg->conn_; marshalToPGArray(conn, strSellerIDs, mSellerIDs); marshalToPGArray(conn, strOfferIDs, mOfferIDs); - marshalToPGArray(conn, strSellingAssetTypes, mSellingAssetTypes); - marshalToPGArray(conn, strSellingAssetCodes, mSellingAssetCodes, - &mSellingAssetCodeInds); - marshalToPGArray(conn, strSellingIssuers, mSellingIssuers, - &mSellingIssuerInds); - - marshalToPGArray(conn, strBuyingAssetTypes, mBuyingAssetTypes); - marshalToPGArray(conn, strBuyingAssetCodes, mBuyingAssetCodes, - &mBuyingAssetCodeInds); - marshalToPGArray(conn, strBuyingIssuers, mBuyingIssuers, - &mBuyingIssuerInds); + marshalToPGArray(conn, strSellingAssets, mSellingAssets); + marshalToPGArray(conn, strBuyingAssets, mBuyingAssets); marshalToPGArray(conn, strAmounts, mAmounts); marshalToPGArray(conn, strPriceNs, mPriceNs); @@ -571,35 +399,25 @@ class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation marshalToPGArray(conn, strLastModifieds, mLastModifieds); std::string sql = "WITH r AS (SELECT " - "unnest(:sellerids::TEXT[]), " - "unnest(:offerids::BIGINT[]), " - "unnest(:v1::INT[]), " - "unnest(:v2::TEXT[]), " + "unnest(:v1::TEXT[]), " + "unnest(:v2::BIGINT[]), " "unnest(:v3::TEXT[]), " - "unnest(:v4::INT[]), " - "unnest(:v5::TEXT[]), " - "unnest(:v6::TEXT[]), " - "unnest(:v7::BIGINT[]), " - "unnest(:v8::INT[]), " + "unnest(:v4::TEXT[]), " + "unnest(:v5::BIGINT[]), " + "unnest(:v6::INT[]), " + "unnest(:v7::INT[]), " + "unnest(:v8::DOUBLE PRECISION[]), " "unnest(:v9::INT[]), " - "unnest(:v10::DOUBLE PRECISION[]), " - "unnest(:v11::INT[]), " - "unnest(:v12::INT[]) " + "unnest(:v10::INT[]) " ")" "INSERT INTO offers ( " - "sellerid, offerid, " - "sellingassettype, sellingassetcode, sellingissuer, " - "buyingassettype, buyingassetcode, buyingissuer, " + "sellerid, offerid, sellingasset, buyingasset, " "amount, pricen, priced, price, flags, lastmodified " ") SELECT * from r " "ON CONFLICT (offerid) DO UPDATE SET " "sellerid = excluded.sellerid, " - "sellingassettype = excluded.sellingassettype, " - "sellingassetcode = excluded.sellingassetcode, " - "sellingissuer = excluded.sellingissuer, " - "buyingassettype = excluded.buyingassettype, " - "buyingassetcode = excluded.buyingassetcode, " - "buyingissuer = excluded.buyingissuer, " + "sellingasset = excluded.sellingasset, " + "buyingasset = excluded.buyingasset, " "amount = excluded.amount, " "pricen = excluded.pricen, " "priced = excluded.priced, " @@ -610,12 +428,8 @@ class BulkUpsertOffersOperation : public DatabaseTypeSpecificOperation soci::statement& st = prep.statement(); st.exchange(soci::use(strSellerIDs)); st.exchange(soci::use(strOfferIDs)); - st.exchange(soci::use(strSellingAssetTypes)); - st.exchange(soci::use(strSellingAssetCodes)); - st.exchange(soci::use(strSellingIssuers)); - st.exchange(soci::use(strBuyingAssetTypes)); - st.exchange(soci::use(strBuyingAssetCodes)); - st.exchange(soci::use(strBuyingIssuers)); + st.exchange(soci::use(strSellingAssets)); + st.exchange(soci::use(strBuyingAssets)); st.exchange(soci::use(strAmounts)); st.exchange(soci::use(strPriceNs)); st.exchange(soci::use(strPriceDs)); @@ -769,23 +583,16 @@ class BulkLoadOffersOperation std::vector executeAndFetch(soci::statement& st) { - std::string sellerID, sellingAssetCode, sellingIssuer, buyingAssetCode, - buyingIssuer; + std::string sellerID, sellingAsset, buyingAsset; int64_t amount; uint64_t offerID; - uint32_t sellingAssetType, buyingAssetType, flags, lastModified; + uint32_t flags, lastModified; Price price; - soci::indicator sellingAssetCodeIndicator, buyingAssetCodeIndicator, - sellingIssuerIndicator, buyingIssuerIndicator; st.exchange(soci::into(sellerID)); st.exchange(soci::into(offerID)); - st.exchange(soci::into(sellingAssetType)); - st.exchange(soci::into(sellingAssetCode, sellingAssetCodeIndicator)); - st.exchange(soci::into(sellingIssuer, sellingIssuerIndicator)); - st.exchange(soci::into(buyingAssetType)); - st.exchange(soci::into(buyingAssetCode, buyingAssetCodeIndicator)); - st.exchange(soci::into(buyingIssuer, buyingIssuerIndicator)); + st.exchange(soci::into(sellingAsset)); + st.exchange(soci::into(buyingAsset)); st.exchange(soci::into(amount)); st.exchange(soci::into(price.n)); st.exchange(soci::into(price.d)); @@ -814,12 +621,8 @@ class BulkLoadOffersOperation oe.sellerID = pubKey; oe.offerID = offerID; - processAsset(oe.selling, (AssetType)sellingAssetType, - sellingIssuer, sellingIssuerIndicator, - sellingAssetCode, sellingAssetCodeIndicator); - processAsset(oe.buying, (AssetType)buyingAssetType, - buyingIssuer, buyingIssuerIndicator, - buyingAssetCode, buyingAssetCodeIndicator); + oe.selling = processAsset(sellingAsset); + oe.buying = processAsset(buyingAsset); oe.amount = amount; oe.price = price; @@ -856,8 +659,7 @@ class BulkLoadOffersOperation doSqliteSpecificOperation(soci::sqlite3_session_backend* sq) override { std::string sql = - "SELECT sellerid, offerid, sellingassettype, sellingassetcode, " - "sellingissuer, buyingassettype, buyingassetcode, buyingissuer, " + "SELECT sellerid, offerid, sellingasset, buyingasset, " "amount, pricen, priced, flags, lastmodified " "FROM offers WHERE offerid IN carray(?, ?, 'int64')"; @@ -881,8 +683,7 @@ class BulkLoadOffersOperation std::string sql = "WITH r AS (SELECT unnest(:v1::BIGINT[])) " - "SELECT sellerid, offerid, sellingassettype, sellingassetcode, " - "sellingissuer, buyingassettype, buyingassetcode, buyingissuer, " + "SELECT sellerid, offerid, sellingasset, buyingasset, " "amount, pricen, priced, flags, lastmodified " "FROM offers WHERE offerid IN (SELECT * FROM r)"; auto prep = mDb.getPreparedStatement(sql); @@ -901,4 +702,133 @@ LedgerTxnRoot::Impl::bulkLoadOffers( return populateLoadedEntries(keys, mDatabase.doDatabaseTypeSpecificOperation(op)); } + +static void +processAssetForSchemaUpgrade(Asset& asset, AssetType assetType, + std::string const& issuerStr, + soci::indicator const& issuerIndicator, + std::string const& assetCode, + soci::indicator const& assetCodeIndicator) +{ + asset.type(assetType); + if (assetType != ASSET_TYPE_NATIVE) + { + if ((assetCodeIndicator != soci::i_ok) || + (issuerIndicator != soci::i_ok)) + { + throw std::runtime_error("bad database state"); + } + + if (assetType == ASSET_TYPE_CREDIT_ALPHANUM12) + { + asset.alphaNum12().issuer = + KeyUtils::fromStrKey(issuerStr); + strToAssetCode(asset.alphaNum12().assetCode, assetCode); + } + else if (assetType == ASSET_TYPE_CREDIT_ALPHANUM4) + { + asset.alphaNum4().issuer = + KeyUtils::fromStrKey(issuerStr); + strToAssetCode(asset.alphaNum4().assetCode, assetCode); + } + else + { + throw std::runtime_error("bad database state"); + } + } +} + +static std::vector +loadAllOffersForSchemaUpgrade(Database& db) +{ + std::vector offers; + + std::string sql = "SELECT sellerid, offerid, " + "sellingassettype, sellingassetcode, sellingissuer, " + "buyingassettype, buyingassetcode, buyingissuer, " + "amount, pricen, priced, flags, lastmodified " + "FROM offers"; + auto prep = db.getPreparedStatement(sql); + + std::string actIDStrKey; + unsigned int sellingAssetType, buyingAssetType; + std::string sellingAssetCode, buyingAssetCode, sellingIssuerStrKey, + buyingIssuerStrKey; + soci::indicator sellingAssetCodeIndicator, buyingAssetCodeIndicator, + sellingIssuerIndicator, buyingIssuerIndicator; + + LedgerEntry le; + le.data.type(OFFER); + OfferEntry& oe = le.data.offer(); + + auto& st = prep.statement(); + st.exchange(soci::into(actIDStrKey)); + st.exchange(soci::into(oe.offerID)); + st.exchange(soci::into(sellingAssetType)); + st.exchange(soci::into(sellingAssetCode, sellingAssetCodeIndicator)); + st.exchange(soci::into(sellingIssuerStrKey, sellingIssuerIndicator)); + st.exchange(soci::into(buyingAssetType)); + st.exchange(soci::into(buyingAssetCode, buyingAssetCodeIndicator)); + st.exchange(soci::into(buyingIssuerStrKey, buyingIssuerIndicator)); + st.exchange(soci::into(oe.amount)); + st.exchange(soci::into(oe.price.n)); + st.exchange(soci::into(oe.price.d)); + st.exchange(soci::into(oe.flags)); + st.exchange(soci::into(le.lastModifiedLedgerSeq)); + st.define_and_bind(); + st.execute(true); + while (st.got_data()) + { + oe.sellerID = KeyUtils::fromStrKey(actIDStrKey); + processAssetForSchemaUpgrade(oe.selling, (AssetType)sellingAssetType, + sellingIssuerStrKey, + sellingIssuerIndicator, sellingAssetCode, + sellingAssetCodeIndicator); + processAssetForSchemaUpgrade(oe.buying, (AssetType)buyingAssetType, + buyingIssuerStrKey, buyingIssuerIndicator, + buyingAssetCode, buyingAssetCodeIndicator); + + offers.emplace_back(le); + st.fetch(); + } + + return offers; +} + +void +LedgerTxnRoot::Impl::writeOffersIntoSimplifiedOffersTable() +{ + throwIfChild(); + mEntryCache.clear(); + mBestOffersCache.clear(); + + CLOG(INFO, "Ledger") << "Loading all offers"; + auto const offers = stellar::loadAllOffersForSchemaUpgrade(mDatabase); + + mDatabase.getSession() << "DROP TABLE IF EXISTS offers"; + mDatabase.getSession() + << "CREATE TABLE offers" + "(" + "sellerid VARCHAR(56) NOT NULL," + "offerid BIGINT NOT NULL CHECK (offerid >= 0)," + "sellingasset TEXT NOT NULL," + "buyingasset TEXT NOT NULL," + "amount BIGINT NOT NULL CHECK (amount >= 0)," + "pricen INT NOT NULL," + "priced INT NOT NULL," + "price DOUBLE PRECISION NOT NULL," + "flags INT NOT NULL," + "lastmodified INT NOT NULL," + "PRIMARY KEY (offerid)" + ");"; + mDatabase.getSession() << "CREATE INDEX bestofferindex ON offers " + "(sellingasset,buyingasset,price);"; + + if (!offers.empty()) + { + BulkUpsertOffersOperation op(mDatabase, offers); + mDatabase.doDatabaseTypeSpecificOperation(op); + CLOG(INFO, "Ledger") << "Wrote " << offers.size() << " offer entries"; + } +} } From 9f1c960fcc664505960800d294d928685b4750f1 Mon Sep 17 00:00:00 2001 From: Jon Jove Date: Mon, 4 Mar 2019 14:17:48 -0600 Subject: [PATCH 4/7] Fix issue with large offer id --- src/ledger/LedgerTxnOfferSQL.cpp | 12 +++++-- src/transactions/OfferTests.cpp | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/ledger/LedgerTxnOfferSQL.cpp b/src/ledger/LedgerTxnOfferSQL.cpp index e94072d569..4eaf8a64c4 100644 --- a/src/ledger/LedgerTxnOfferSQL.cpp +++ b/src/ledger/LedgerTxnOfferSQL.cpp @@ -20,6 +20,11 @@ std::shared_ptr LedgerTxnRoot::Impl::loadOffer(LedgerKey const& key) const { uint64_t offerID = key.offer().offerID; + if (offerID > INT64_MAX) + { + return nullptr; + } + std::string actIDStrKey = KeyUtils::toStrKey(key.offer().sellerID); std::string sql = "SELECT sellerid, offerid, sellingasset, buyingasset, " @@ -645,8 +650,11 @@ class BulkLoadOffersOperation for (auto const& k : keys) { assert(k.type() == OFFER); - mSellerIDs.emplace_back(k.offer().sellerID); - mOfferIDs.emplace_back(unsignedToSigned(k.offer().offerID)); + if (k.offer().offerID <= INT64_MAX) + { + mSellerIDs.emplace_back(k.offer().sellerID); + mOfferIDs.emplace_back(unsignedToSigned(k.offer().offerID)); + } } for (size_t i = 0; i < mSellerIDs.size(); ++i) diff --git a/src/transactions/OfferTests.cpp b/src/transactions/OfferTests.cpp index 030f63909e..0a8684d224 100644 --- a/src/transactions/OfferTests.cpp +++ b/src/transactions/OfferTests.cpp @@ -2799,6 +2799,67 @@ TEST_CASE("create offer", "[tx][offers]") }); } } + + SECTION("large offer id") + { + SECTION("modify") + { + for_all_versions(*app, [&] { + auto const minBalance = + app->getLedgerManager().getLastMinBalance(3); + auto acc1 = root.create("acc1", minBalance + 10000); + + acc1.changeTrust(usd, INT64_MAX); + acc1.changeTrust(idr, INT64_MAX); + issuer.pay(acc1, usd, 1000); + issuer.pay(acc1, idr, 1000); + + TestMarket market(*app); + REQUIRE_THROWS_AS( + market.updateOffer(acc1, INT64_MAX, + {usd, idr, Price{1, 1}, 1}), + ex_MANAGE_OFFER_NOT_FOUND); + REQUIRE_THROWS_AS( + market.updateOffer(acc1, + static_cast(INT64_MAX) + 1, + {usd, idr, Price{1, 1}, 1}), + ex_MANAGE_OFFER_NOT_FOUND); + REQUIRE_THROWS_AS( + market.updateOffer(acc1, UINT64_MAX, + {usd, idr, Price{1, 1}, 1}), + ex_MANAGE_OFFER_NOT_FOUND); + }); + } + + SECTION("erase") + { + for_all_versions(*app, [&] { + auto const minBalance = + app->getLedgerManager().getLastMinBalance(3); + auto acc1 = root.create("acc1", minBalance + 10000); + + acc1.changeTrust(usd, INT64_MAX); + acc1.changeTrust(idr, INT64_MAX); + issuer.pay(acc1, usd, 1000); + issuer.pay(acc1, idr, 1000); + + TestMarket market(*app); + REQUIRE_THROWS_AS( + market.updateOffer(acc1, INT64_MAX, + {usd, idr, Price{1, 1}, 0}), + ex_MANAGE_OFFER_NOT_FOUND); + REQUIRE_THROWS_AS( + market.updateOffer(acc1, + static_cast(INT64_MAX) + 1, + {usd, idr, Price{1, 1}, 0}), + ex_MANAGE_OFFER_NOT_FOUND); + REQUIRE_THROWS_AS( + market.updateOffer(acc1, UINT64_MAX, + {usd, idr, Price{1, 1}, 0}), + ex_MANAGE_OFFER_NOT_FOUND); + }); + } + } } TEST_CASE("liabilities match created offer", "[tx][offers]") From aef707acee15cac279613fe7f87a0c776f5a8e32 Mon Sep 17 00:00:00 2001 From: Jon Jove Date: Mon, 4 Mar 2019 15:00:44 -0600 Subject: [PATCH 5/7] Remove mSellerIDs from BulkLoadOffersOperation --- src/ledger/LedgerTxnOfferSQL.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/ledger/LedgerTxnOfferSQL.cpp b/src/ledger/LedgerTxnOfferSQL.cpp index 4eaf8a64c4..b04310b5aa 100644 --- a/src/ledger/LedgerTxnOfferSQL.cpp +++ b/src/ledger/LedgerTxnOfferSQL.cpp @@ -581,7 +581,6 @@ class BulkLoadOffersOperation : public DatabaseTypeSpecificOperation> { Database& mDb; - std::vector mSellerIDs; std::vector mOfferIDs; std::unordered_map mSellerIDsByOfferID; @@ -645,22 +644,16 @@ class BulkLoadOffersOperation std::unordered_set const& keys) : mDb(db) { - mSellerIDs.reserve(keys.size()); mOfferIDs.reserve(keys.size()); for (auto const& k : keys) { assert(k.type() == OFFER); if (k.offer().offerID <= INT64_MAX) { - mSellerIDs.emplace_back(k.offer().sellerID); mOfferIDs.emplace_back(unsignedToSigned(k.offer().offerID)); + mSellerIDsByOfferID[mOfferIDs.back()] = k.offer().sellerID; } } - - for (size_t i = 0; i < mSellerIDs.size(); ++i) - { - mSellerIDsByOfferID[mOfferIDs[i]] = mSellerIDs[i]; - } } virtual std::vector From 8a987ca1b0c8b0beb2055607f8d00055bc073168 Mon Sep 17 00:00:00 2001 From: Jon Jove Date: Mon, 4 Mar 2019 16:08:10 -0600 Subject: [PATCH 6/7] Update db-schema.md for simplified offers schema --- docs/db-schema.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/docs/db-schema.md b/docs/db-schema.md index 6f74e944a8..2320813bb6 100644 --- a/docs/db-schema.md +++ b/docs/db-schema.md @@ -66,12 +66,8 @@ Field | Type | Description ------|------|--------------- sellerid | VARCHAR(56) NOT NULL | (STRKEY) offerid | BIGINT NOT NULL CHECK (offerid >= 0) | -sellingassettype | INT | selling.type -sellingassetcode | VARCHAR(12) | selling.*.assetCode -sellingissuer | VARCHAR(56) | selling.*.issuer -buyingassettype | INT | buying.type -buyingassetcode | VARCHAR(12) | buying.*.assetCode -buyingissuer | VARCHAR(56) | buying.*.issuer +sellingasset | TEXT NOT NULL | selling (BASE64) +buyingasset | TEXT NOT NULL | buying (BASE64) amount | BIGINT NOT NULL CHECK (amount >= 0) | pricen | INT NOT NULL | Price.n priced | INT NOT NULL | Price.d From d5f89967f0f559f6c4b96af9d0fbe42ba346266c Mon Sep 17 00:00:00 2001 From: Jon Jove Date: Tue, 5 Mar 2019 15:01:53 -0600 Subject: [PATCH 7/7] Warning police --- src/scp/SCPUnitTests.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/scp/SCPUnitTests.cpp b/src/scp/SCPUnitTests.cpp index e851e99c00..6abb765f8f 100644 --- a/src/scp/SCPUnitTests.cpp +++ b/src/scp/SCPUnitTests.cpp @@ -120,7 +120,8 @@ class TestNominationSCP : public SCPDriver Value const& getLatestCompositeCandidate(uint64 slotIndex) { - return {}; + static Value const emptyValue{}; + return emptyValue; } };