Tidy up offer crossing:

* Clarify use of cancel view in OfferCreate transactor
* Reduce OfferStream public interface
* Reduce severity of some developer-only logging from ERROR to DEBUG
This commit is contained in:
Nik Bougalis
2015-07-16 06:28:57 -07:00
committed by Vinnie Falco
parent b1b98fa3b0
commit af36942e1f
4 changed files with 78 additions and 98 deletions

View File

@@ -139,13 +139,13 @@ log_one(Ledger::pointer ledger, uint256 const& tx, char const* msg)
if (metaData != nullptr)
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": " << msg << " is missing this transaction:\n" <<
metaData->getJson (0);
}
else
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": " << msg << " is missing this transaction.";
}
}
@@ -185,31 +185,31 @@ log_metadata_difference(Ledger::pointer builtLedger, Ledger::pointer validLedger
{
if (result_diff && index_diff)
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": Different result and index!";
WriteLog (lsERROR, LedgerMaster) << " Built:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Built:" <<
" Result: " << builtMetaData->getResult () <<
" Index: " << builtMetaData->getIndex ();
WriteLog (lsERROR, LedgerMaster) << " Valid:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Valid:" <<
" Result: " << validMetaData->getResult () <<
" Index: " << validMetaData->getIndex ();
}
else if (result_diff)
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": Different result!";
WriteLog (lsERROR, LedgerMaster) << " Built:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Built:" <<
" Result: " << builtMetaData->getResult ();
WriteLog (lsERROR, LedgerMaster) << " Valid:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Valid:" <<
" Result: " << validMetaData->getResult ();
}
else if (index_diff)
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": Different index!";
WriteLog (lsERROR, LedgerMaster) << " Built:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Built:" <<
" Index: " << builtMetaData->getIndex ();
WriteLog (lsERROR, LedgerMaster) << " Valid:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Valid:" <<
" Index: " << validMetaData->getIndex ();
}
}
@@ -217,42 +217,42 @@ log_metadata_difference(Ledger::pointer builtLedger, Ledger::pointer validLedger
{
if (result_diff && index_diff)
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": Different result, index and nodes!";
WriteLog (lsERROR, LedgerMaster) << " Built:\n" <<
WriteLog (lsDEBUG, LedgerMaster) << " Built:\n" <<
builtMetaData->getJson (0);
WriteLog (lsERROR, LedgerMaster) << " Valid:\n" <<
WriteLog (lsDEBUG, LedgerMaster) << " Valid:\n" <<
validMetaData->getJson (0);
}
else if (result_diff)
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": Different result and nodes!";
WriteLog (lsERROR, LedgerMaster) << " Built:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Built:" <<
" Result: " << builtMetaData->getResult () <<
" Nodes:\n" << builtNodes.getJson (0);
WriteLog (lsERROR, LedgerMaster) << " Valid:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Valid:" <<
" Result: " << validMetaData->getResult () <<
" Nodes:\n" << validNodes.getJson (0);
}
else if (index_diff)
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": Different index and nodes!";
WriteLog (lsERROR, LedgerMaster) << " Built:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Built:" <<
" Index: " << builtMetaData->getIndex () <<
" Nodes:\n" << builtNodes.getJson (0);
WriteLog (lsERROR, LedgerMaster) << " Valid:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Valid:" <<
" Index: " << validMetaData->getIndex () <<
" Nodes:\n" << validNodes.getJson (0);
}
else // nodes_diff
{
WriteLog (lsERROR, LedgerMaster) << "MISMATCH on TX " << tx <<
WriteLog (lsDEBUG, LedgerMaster) << "MISMATCH on TX " << tx <<
": Different nodes!";
WriteLog (lsERROR, LedgerMaster) << " Built:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Built:" <<
" Nodes:\n" << builtNodes.getJson (0);
WriteLog (lsERROR, LedgerMaster) << " Valid:" <<
WriteLog (lsDEBUG, LedgerMaster) << " Valid:" <<
" Nodes:\n" << validNodes.getJson (0);
}
}

View File

@@ -563,8 +563,7 @@ CreateOffer::applyGuts (ApplyView& view, ApplyView& view_cancel)
// sequence to determine the transaction. Why is the offer sequence
// number insufficient?
auto const sleCreator = view.peek (
keylet::account(account_));
auto const sleCreator = view.peek (keylet::account(account_));
deprecatedWrongOwnerCount_ = sleCreator->getFieldU32(sfOwnerCount);
@@ -831,9 +830,12 @@ CreateOffer::doApply()
// This is the ledger view that we work against. Transactions are applied
// as we go on processing transactions.
Sandbox view (&ctx_.view());
// This is a checkpoint with just the fees paid. If something goes wrong
// with this transaction, we roll back to this ledger.
// This is a ledger with just the fees paid and any unfunded or expired
// offers we encounter removed. It's used when handling Fill-or-Kill offers,
// if the order isn't going to be placed, to avoid wasting the work we did.
Sandbox viewCancel (&ctx_.view());
auto const result = applyGuts(view, viewCancel);
if (result.second)
view.apply(ctx_.rawView());

View File

@@ -19,18 +19,19 @@
#include <BeastConfig.h>
#include <ripple/app/tx/impl/OfferStream.h>
#include <ripple/basics/Log.h>
namespace ripple {
OfferStream::OfferStream (ApplyView& view, ApplyView& view_cancel,
OfferStream::OfferStream (ApplyView& view, ApplyView& cancelView,
BookRef book, Clock::time_point when,
Config const& config, beast::Journal journal)
: j_ (journal)
, m_view (view)
, m_view_cancel (view_cancel)
, m_book (book)
, m_when (when)
, m_tip (view, book)
, view_ (view)
, cancelView_ (cancelView)
, book_ (book)
, expire_ (when)
, tip_ (view, book_)
, config_ (config)
{
}
@@ -44,24 +45,24 @@ OfferStream::erase (ApplyView& view)
// correctly remove the directory if its the last entry.
// Unfortunately this is a protocol breaking change.
auto p = view.peek (keylet::page(m_tip.dir()));
auto p = view.peek (keylet::page(tip_.dir()));
if (p == nullptr)
{
if (j_.error) j_.error <<
"Missing directory " << m_tip.dir() <<
" for offer " << m_tip.index();
JLOG(j_.error) <<
"Missing directory " << tip_.dir() <<
" for offer " << tip_.index();
return;
}
auto v (p->getFieldV256 (sfIndexes));
auto it (std::find (v.begin(), v.end(), m_tip.index()));
auto it (std::find (v.begin(), v.end(), tip_.index()));
if (it == v.end())
{
if (j_.error) j_.error <<
"Missing offer " << m_tip.index() <<
" for directory " << m_tip.dir();
JLOG(j_.error) <<
"Missing offer " << tip_.index() <<
" for directory " << tip_.dir();
return;
}
@@ -69,9 +70,9 @@ OfferStream::erase (ApplyView& view)
p->setFieldV256 (sfIndexes, v);
view.update (p);
if (j_.trace) j_.trace <<
"Missing offer " << m_tip.index() <<
" removed from directory " << m_tip.dir();
JLOG(j_.trace) <<
"Missing offer " << tip_.index() <<
" removed from directory " << tip_.dir();
}
bool
@@ -84,52 +85,48 @@ OfferStream::step ()
{
// BookTip::step deletes the current offer from the view before
// advancing to the next (unless the ledger entry is missing).
if (! m_tip.step())
if (! tip_.step())
return false;
std::shared_ptr<SLE> entry = m_tip.entry();
std::shared_ptr<SLE> entry = tip_.entry();
// Remove if missing
if (! entry)
{
erase (view());
erase (view_cancel());
erase (view_);
erase (cancelView_);
continue;
}
// Remove if expired
if (entry->isFieldPresent (sfExpiration) &&
entry->getFieldU32 (sfExpiration) <= m_when)
entry->getFieldU32 (sfExpiration) <= expire_)
{
if (j_.trace) j_.trace <<
JLOG(j_.trace) <<
"Removing expired offer " << entry->getIndex();
offerDelete (view_cancel(),
view_cancel().peek(
keylet::offer(entry->key())));
offerDelete (cancelView_,
cancelView_.peek(keylet::offer(entry->key())));
continue;
}
m_offer = Offer (entry, m_tip.quality());
offer_ = Offer (entry, tip_.quality());
Amounts const amount (m_offer.amount());
Amounts const amount (offer_.amount());
// Remove if either amount is zero
if (amount.empty())
{
if (j_.warning) j_.warning <<
JLOG(j_.warning) <<
"Removing bad offer " << entry->getIndex();
offerDelete (view_cancel(),
view_cancel().peek(
keylet::offer(entry->key())));
m_offer = Offer{};
offerDelete (cancelView_,
cancelView_.peek(keylet::offer(entry->key())));
offer_ = Offer{};
continue;
}
// Calculate owner funds
// NIKB NOTE The calling code also checks the funds, how expensive is
// looking up the funds twice?
auto const owner_funds = accountFunds(view(),
m_offer.owner(), amount.out, fhZERO_IF_FROZEN,
auto const owner_funds = accountFunds(view_,
offer_.owner(), amount.out, fhZERO_IF_FROZEN,
config_);
// Check for unfunded offer
@@ -138,23 +135,23 @@ OfferStream::step ()
// If the owner's balance in the pristine view is the same,
// we haven't modified the balance and therefore the
// offer is "found unfunded" versus "became unfunded"
auto const original_funds = accountFunds(view_cancel(),
m_offer.owner(), amount.out, fhZERO_IF_FROZEN,
auto const original_funds = accountFunds(cancelView_,
offer_.owner(), amount.out, fhZERO_IF_FROZEN,
config_);
if (original_funds == owner_funds)
{
offerDelete (view_cancel(), view_cancel().peek(
offerDelete (cancelView_, cancelView_.peek(
keylet::offer(entry->key())));
if (j_.trace) j_.trace <<
JLOG(j_.trace) <<
"Removing unfunded offer " << entry->getIndex();
}
else
{
if (j_.trace) j_.trace <<
JLOG(j_.trace) <<
"Removing became unfunded offer " << entry->getIndex();
}
m_offer = Offer{};
offer_ = Offer{};
continue;
}

View File

@@ -21,12 +21,11 @@
#define RIPPLE_APP_BOOK_OFFERSTREAM_H_INCLUDED
#include <ripple/app/tx/impl/BookTip.h>
#include <ripple/app/tx/impl/Offer.h>
#include <ripple/app/tx/impl/Offer.h>
#include <ripple/ledger/View.h>
#include <ripple/protocol/Quality.h>
#include <beast/utility/Journal.h>
#include <beast/utility/noexcept.h>
#include <functional>
namespace ripple {
@@ -51,41 +50,23 @@ class OfferStream
{
private:
beast::Journal j_;
std::reference_wrapper <ApplyView> m_view;
std::reference_wrapper <ApplyView> m_view_cancel;
Book m_book;
Clock::time_point m_when;
BookTip m_tip;
Offer m_offer;
ApplyView& view_;
ApplyView& cancelView_;
Book book_;
Clock::time_point const expire_;
BookTip tip_;
Offer offer_;
Config const& config_;
void
erase (ApplyView& view);
public:
OfferStream (ApplyView& view, ApplyView& view_cancel,
OfferStream (ApplyView& view, ApplyView& cancelView,
BookRef book, Clock::time_point when,
Config const& config,
beast::Journal journal);
ApplyView&
view () noexcept
{
return m_view;
}
ApplyView&
view_cancel () noexcept
{
return m_view_cancel;
}
Book const&
book () const noexcept
{
return m_book;
}
/** Returns the offer at the tip of the order book.
Offers are always presented in decreasing quality.
Only valid if step() returned `true`.
@@ -93,7 +74,7 @@ public:
Offer const&
tip () const
{
return m_offer;
return offer_;
}
/** Advance to the next valid offer.