Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the rendering issues of some XDR fields #2876

Merged
merged 4 commits into from Mar 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/catchup/ApplyCheckpointWork.cpp
Expand Up @@ -257,11 +257,12 @@ ApplyCheckpointWork::onRun()
{
auto& lm = mApp.getLedgerManager();

CLOG_DEBUG(History, "LedgerManager LCL:\n{}",
xdr_to_string(lm.getLastClosedLedgerHeader()));
CLOG_DEBUG(History, "{}",
xdr_to_string(lm.getLastClosedLedgerHeader(),
"LedgerManager LCL"));

CLOG_DEBUG(History, "Replay header:\n{}",
xdr_to_string(mHeaderHistoryEntry));
CLOG_DEBUG(History, "{}",
xdr_to_string(mHeaderHistoryEntry, "Replay header"));
if (lm.getLastClosedLedgerHeader().hash !=
mHeaderHistoryEntry.hash)
{
Expand Down
28 changes: 15 additions & 13 deletions src/catchup/simulation/TxSimApplyTransactionsWork.cpp
Expand Up @@ -66,9 +66,9 @@ checkOperationResults(xdr::xvector<OperationResult> const& expected,
{
if (expected[i].code() != actual[i].code())
{
CLOG_ERROR(History, "Expected operation result {} but got {}",
xdr_to_string(expected[i].code()),
xdr_to_string(actual[i].code()));
CLOG_ERROR(History, "Expected {} but got {}",
xdr_to_string(expected[i].code(), "OperationResultCode"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we're printing OperationResultCode, you can change the log to be Expected {} but got {}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

xdr_to_string(actual[i].code(), "OperationResultCode"));
continue;
}

Expand Down Expand Up @@ -154,10 +154,10 @@ checkOperationResults(xdr::xvector<OperationResult> const& expected,

if (!match)
{
CLOG_ERROR(History, "Expected operation result: {}",
xdr_to_string(expectedOpRes));
CLOG_ERROR(History, "Actual operation result: {}",
xdr_to_string(actualOpRes));
CLOG_ERROR(History, "Expected {}",
xdr_to_string(expectedOpRes, "OperationResult"));
CLOG_ERROR(History, "Actual {}",
xdr_to_string(actualOpRes, "OperationResult"));
}
}
}
Expand All @@ -179,9 +179,9 @@ checkResults(Application& app, uint32_t ledger,
if (dbRes.code() != archiveRes.code())
{
CLOG_ERROR(
History,
"Expected result code {} does not agree with {} for tx {}",
xdr_to_string(archiveRes.code()), xdr_to_string(dbRes.code()),
History, "Expected {} does not agree with {} for tx {}",
xdr_to_string(archiveRes.code(), "TransactionResultCode"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"result code" is now redundant, change to "Expected {} does not agree with {} for tx {}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

xdr_to_string(dbRes.code(), "TransactionResultCode"),
binToHex(results[i].transactionHash));
}
else if (dbRes.code() == txFEE_BUMP_INNER_FAILED ||
Expand All @@ -193,11 +193,13 @@ checkResults(Application& app, uint32_t ledger,
{
CLOG_ERROR(
History,
"Expected result code {} does not agree with {} for "
"Expected {} does not agree with {} for "
"fee-bump inner tx {}",
xdr_to_string(
archiveRes.innerResultPair().result.result.code()),
xdr_to_string(dbRes.innerResultPair().result.result.code()),
archiveRes.innerResultPair().result.result.code(),
"TransactionResultCode"),
xdr_to_string(dbRes.innerResultPair().result.result.code(),
"TransactionResultCode"),
binToHex(archiveRes.innerResultPair().transactionHash));
}
else if (dbRes.innerResultPair().result.result.code() == txFAILED ||
Expand Down
16 changes: 9 additions & 7 deletions src/herder/TxSetFrame.cpp
Expand Up @@ -298,12 +298,13 @@ TxSetFrame::checkOrTrim(Application& app,
{
if (justCheck)
{
CLOG_DEBUG(Herder,
"Got bad txSet: {} tx invalid lastSeq:{} tx: {} "
"result: {}",
hexAbbrev(mPreviousLedgerHash), lastSeq,
xdr_to_string(tx->getEnvelope()),
tx->getResultCode());
CLOG_DEBUG(
Herder,
"Got bad txSet: {} tx invalid lastSeq:{} tx: {} "
"result: {}",
hexAbbrev(mPreviousLedgerHash), lastSeq,
xdr_to_string(tx->getEnvelope(), "TransactionEnvelope"),
tx->getResultCode());
return false;
}
trimmed.emplace_back(tx);
Expand Down Expand Up @@ -343,7 +344,8 @@ TxSetFrame::checkOrTrim(Application& app,
CLOG_DEBUG(Herder,
"Got bad txSet: {} account can't pay fee tx: {}",
hexAbbrev(mPreviousLedgerHash),
xdr_to_string(tx->getEnvelope()));
xdr_to_string(tx->getEnvelope(),
"TransactionEnvelope"));
return false;
}
while (iter != kv.second.end())
Expand Down
7 changes: 4 additions & 3 deletions src/invariant/InvariantManagerImpl.cpp
Expand Up @@ -115,9 +115,10 @@ InvariantManagerImpl::checkOnOperationApply(Operation const& operation,
continue;
}

auto message = fmt::format(
R"(Invariant "{}" does not hold on operation: {}{}{})",
invariant->getName(), result, "\n", xdr_to_string(operation));
auto message =
fmt::format(R"(Invariant "{}" does not hold on operation: {}{}{})",
invariant->getName(), result, "\n",
xdr_to_string(operation, "Operation"));
onInvariantFailure(invariant, message,
ltxDelta.header.current.ledgerSeq);
}
Expand Down
25 changes: 12 additions & 13 deletions src/invariant/LiabilitiesMatchOffers.cpp
Expand Up @@ -100,7 +100,7 @@ checkAuthorized(LedgerEntry const* current, LedgerEntry const* previous)
{
return fmt::format(
"Liabilities increased on unauthorized trust line {}",
xdr_to_string(trust));
xdr_to_string(trust, "TrustLineEntry"));
}
}
else
Expand All @@ -110,7 +110,7 @@ checkAuthorized(LedgerEntry const* current, LedgerEntry const* previous)
{
return fmt::format(
"Unauthorized trust line has liabilities {}",
xdr_to_string(trust));
xdr_to_string(trust, "TrustLineEntry"));
}
}
}
Expand Down Expand Up @@ -256,8 +256,8 @@ checkBalanceAndLimit(LedgerHeader const& header, LedgerEntry const* current,
(INT64_MAX - account.balance < liabilities.buying))
{
return fmt::format(
"Balance not compatible with liabilities for account {}",
xdr_to_string(account));
"Balance not compatible with liabilities for {}",
xdr_to_string(account, "AccountEntry"));
}
}
}
Expand All @@ -273,9 +273,8 @@ checkBalanceAndLimit(LedgerHeader const& header, LedgerEntry const* current,
if ((trust.balance < liabilities.selling) ||
(trust.limit - trust.balance < liabilities.buying))
{
return fmt::format(
"Balance not compatible with liabilities for trustline {}",
xdr_to_string(trust));
return fmt::format("Balance not compatible with liabilities for {}",
xdr_to_string(trust, "TrustLineEntry"));
}
}
return {};
Expand Down Expand Up @@ -346,20 +345,20 @@ LiabilitiesMatchOffers::checkOnOperationApply(Operation const& operation,
return fmt::format(
"Change in buying liabilities differed from "
"change in total buying liabilities of "
"offers by {} for account {} in asset {}",
"offers by {} for {} in {}",
assetLiabilities.second.buying,
xdr_to_string(accLiabilities.first),
xdr_to_string(assetLiabilities.first));
xdr_to_string(accLiabilities.first, "account"),
xdr_to_string(assetLiabilities.first, "asset"));
}
else if (assetLiabilities.second.selling != 0)
{
return fmt::format(
"Change in selling liabilities differed from "
"change in total selling liabilities of "
"offers by {} for account {} in asset {}",
"offers by {} for {} in {}",
assetLiabilities.second.selling,
xdr_to_string(accLiabilities.first),
xdr_to_string(assetLiabilities.first));
xdr_to_string(accLiabilities.first, "account"),
xdr_to_string(assetLiabilities.first, "asset"));
}
}
}
Expand Down
30 changes: 17 additions & 13 deletions src/ledger/InternalLedgerEntry.cpp
Expand Up @@ -282,13 +282,16 @@ InternalLedgerKey::toString() const
switch (mType)
{
case InternalLedgerEntryType::LEDGER_ENTRY:
return xdr_to_string(ledgerKey());
return xdr_to_string(ledgerKey(), "LedgerKey");

case InternalLedgerEntryType::SPONSORSHIP:
return fmt::format("{{\n sponsoredID = {}\n}}\n",
xdr_to_string(sponsorshipKey().sponsoredID));
return fmt::format(
"{{\n {}\n}}\n",
xdr_to_string(sponsorshipKey().sponsoredID, "sponsoredID"));
case InternalLedgerEntryType::SPONSORSHIP_COUNTER:
return fmt::format("{{\n sponsoringID = {}\n}}\n",
xdr_to_string(sponsorshipCounterKey().sponsoringID));
return fmt::format("{{\n {}\n}}\n",
xdr_to_string(sponsorshipCounterKey().sponsoringID,
"sponsoringID"));
default:
abort();
}
Expand Down Expand Up @@ -558,16 +561,17 @@ InternalLedgerEntry::toString() const
switch (mType)
{
case InternalLedgerEntryType::LEDGER_ENTRY:
return xdr_to_string(ledgerEntry());
return xdr_to_string(ledgerEntry(), "LedgerEntry");
case InternalLedgerEntryType::SPONSORSHIP:
return fmt::format("{{\n sponsoredID = {},\n sponsoringID = {}\n}}\n",
xdr_to_string(sponsorshipEntry().sponsoredID),
xdr_to_string(sponsorshipEntry().sponsoringID));
case InternalLedgerEntryType::SPONSORSHIP_COUNTER:
return fmt::format(
"{{\n sponsoringID = {},\n numSponsoring = {}\n}}\n",
xdr_to_string(sponsorshipCounterEntry().sponsoringID),
sponsorshipCounterEntry().numSponsoring);
"{{\n {},\n {}\n}}\n",
xdr_to_string(sponsorshipEntry().sponsoredID, "sponsoredID"),
xdr_to_string(sponsorshipEntry().sponsoringID, "sponsoringID"));
case InternalLedgerEntryType::SPONSORSHIP_COUNTER:
return fmt::format("{{\n {},\n numSponsoring = {}\n}}\n",
xdr_to_string(sponsorshipCounterEntry().sponsoringID,
"sponsoringID"),
sponsorshipCounterEntry().numSponsoring);
default:
abort();
}
Expand Down
6 changes: 3 additions & 3 deletions src/ledger/LedgerManagerImpl.cpp
Expand Up @@ -555,8 +555,8 @@ LedgerManagerImpl::closeLedger(LedgerCloseData const& ledgerData)
txSet->previousLedgerHash()),
ledgerAbbrev(getLastClosedLedgerHeader()));

CLOG_ERROR(Ledger, "Full LCL: {}",
xdr_to_string(getLastClosedLedgerHeader()));
CLOG_ERROR(Ledger, "{}",
xdr_to_string(getLastClosedLedgerHeader(), "Full LCL"));
CLOG_ERROR(Ledger, "{}", POSSIBLY_CORRUPTED_LOCAL_DATA);

throw std::runtime_error("txset mismatch");
Expand Down Expand Up @@ -623,7 +623,7 @@ LedgerManagerImpl::closeLedger(LedgerCloseData const& ledgerData)
case Upgrades::UpgradeValidity::INVALID:
throw std::runtime_error(
fmt::format(FMT_STRING("Invalid upgrade at index {}: {}"), i,
xdr_to_string(lupgrade)));
xdr_to_string(lupgrade, "LedgerUpgrade")));
}

try
Expand Down
3 changes: 2 additions & 1 deletion src/main/CommandHandler.cpp
Expand Up @@ -951,7 +951,8 @@ CommandHandler::testTx(std::string const& params, std::string& retStr)
root["status"] = TX_STATUS_STRING[static_cast<int>(status)];
if (status == TransactionQueue::AddResult::ADD_STATUS_ERROR)
{
root["detail"] = xdr_to_string(txFrame->getResult().result.code());
root["detail"] = xdr_to_string(txFrame->getResult().result.code(),
"TransactionResultCode");
}
}
else
Expand Down
4 changes: 2 additions & 2 deletions src/simulation/LoadGenerator.cpp
Expand Up @@ -647,8 +647,8 @@ LoadGenerator::TxInfo::execute(Application& app, bool isCreate,
{
CLOG_INFO(LoadGen, "tx rejected '{}': {} ===> {}",
TX_STATUS_STRING[static_cast<int>(status)],
xdr_to_string(txf->getEnvelope()),
xdr_to_string(txf->getResult()));
xdr_to_string(txf->getEnvelope(), "TransactionEnvelope"),
xdr_to_string(txf->getResult(), "TransactionResult"));
if (status == TransactionQueue::AddResult::ADD_STATUS_ERROR)
{
code = txf->getResultCode();
Expand Down
19 changes: 11 additions & 8 deletions src/test/FuzzerImpl.cpp
Expand Up @@ -789,11 +789,11 @@ applySetupOperations(LedgerTxn& ltx, PublicKey const& sourceAccount,

if (txFramePtr->getResultCode() != txSUCCESS)
{
auto const msg =
fmt::format(FMT_STRING("Error {} while setting up fuzzing -- "
"transaction result {}"),
txFramePtr->getResultCode(),
xdr_to_string(txFramePtr->getResult()));
auto const msg = fmt::format(
FMT_STRING("Error {} while setting up fuzzing -- "
"{}"),
txFramePtr->getResultCode(),
xdr_to_string(txFramePtr->getResult(), "TransactionResult"));
LOG_FATAL(DEFAULT_LOG, "{}", msg);
throw std::runtime_error(msg);
}
Expand All @@ -816,8 +816,9 @@ applySetupOperations(LedgerTxn& ltx, PublicKey const& sourceAccount,
{
auto const msg = fmt::format(
FMT_STRING("Manage offer result {} while setting "
"up fuzzing -- operation is {}"),
xdr_to_string(tr), xdr_to_string(op));
"up fuzzing -- {}"),
xdr_to_string(tr, "Operation"),
xdr_to_string(op, "Operation"));
LOG_FATAL(DEFAULT_LOG, "{}", msg);
throw std::runtime_error(msg);
}
Expand Down Expand Up @@ -1280,7 +1281,9 @@ TransactionFuzzer::inject(std::string const& filename)
}

resetTxInternalState(*mApp);
LOG_TRACE(DEFAULT_LOG, "Fuzz ops ({}): {}", ops.size(), xdr_to_string(ops));
LOG_TRACE(DEFAULT_LOG, "{}",
xdr_to_string(ops, fmt::format("Fuzz ops ({})", ops.size())));

LedgerTxn ltx(mApp->getLedgerTxnRoot());
applyFuzzOperations(ltx, mSourceAccountID, ops.begin(), ops.end(), *mApp);
}
Expand Down
11 changes: 6 additions & 5 deletions src/test/TestPrinter.cpp
Expand Up @@ -13,11 +13,12 @@ namespace Catch
std::string
StringMaker<stellar::OfferState>::convert(stellar::OfferState const& os)
{
return fmt::format(
"selling: {}, buying: {}, price: {}, amount: {}, type: {}",
xdr_to_string(os.selling), xdr_to_string(os.buying),
xdr_to_string(os.price), os.amount,
os.type == stellar::OfferType::PASSIVE ? "passive" : "active");
return fmt::format("{}, {}, {}, amount: {}, type: {}",
xdr_to_string(os.selling, "selling"),
xdr_to_string(os.buying, "buying"),
xdr_to_string(os.price, "price"), os.amount,
os.type == stellar::OfferType::PASSIVE ? "passive"
: "active");
}

std::string
Expand Down
2 changes: 1 addition & 1 deletion src/test/TestPrinter.h
Expand Up @@ -23,7 +23,7 @@ struct StringMaker<T, typename std::enable_if<xdr::xdr_traits<T>::valid>::type>
static std::string
convert(T const& val)
{
return xdr_to_string(val);
return xdr_to_string(val, "value");
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/transactions/OperationFrame.cpp
Expand Up @@ -122,15 +122,15 @@ OperationFrame::apply(SignatureChecker& signatureChecker,
bool res;
if (Logging::logTrace("Tx"))
{
CLOG_TRACE(Tx, "Operation: {}", xdr_to_string(mOperation));
CLOG_TRACE(Tx, "{}", xdr_to_string(mOperation, "Operation"));
}
res = checkValid(signatureChecker, ltx, true);
if (res)
{
res = doApply(ltx);
if (Logging::logTrace("Tx"))
{
CLOG_TRACE(Tx, "Operation result: {}", xdr_to_string(mResult));
CLOG_TRACE(Tx, "{}", xdr_to_string(mResult, "OperationResult"));
}
}

Expand Down
16 changes: 6 additions & 10 deletions src/transactions/TransactionFrame.cpp
Expand Up @@ -817,19 +817,15 @@ TransactionFrame::applyOperations(SignatureChecker& signatureChecker,
}
catch (std::exception& e)
{
CLOG_ERROR(Tx,
"Exception while applying operations (fullHash= {}, "
"contentsHash= {}): {}",
xdr_to_string(getFullHash()),
xdr_to_string(getContentsHash()), e.what());
CLOG_ERROR(Tx, "Exception while applying operations ({}, {}): {}",
xdr_to_string(getFullHash(), "fullHash"),
xdr_to_string(getContentsHash(), "contentsHash"), e.what());
}
catch (...)
{
CLOG_ERROR(Tx,
"Unknown exception while applying operations (fullHash= {}, "
"contentsHash= {})",
xdr_to_string(getFullHash()),
xdr_to_string(getContentsHash()));
CLOG_ERROR(Tx, "Unknown exception while applying operations ({}, {})",
xdr_to_string(getFullHash(), "fullHash"),
xdr_to_string(getContentsHash(), "contentsHash"));
}
// This is only reachable if an exception is thrown
getResult().result.code(txINTERNAL_ERROR);
Expand Down