ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-06 17:27:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

Review feedback from @dangell7: early return & coverage

Browse Source 
- Exclude LogicError lines in ApplyView.cpp (specifically directory
  operations) from code coverage.
- Replace the ability to set the next page on a new directory page with
  an assert, because nothing uses it right now.
- Early return with success for batch inner transactions in preflight2.
This commit is contained in:
Ed Hennis
2025-12-01 16:18:12 -05:00
parent 060ee70eb0
commit ad190a8d6f
2 changed files with 49 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
src/libxrpl/ledger/ApplyView.cpp
View File

@@ -40,7 +40,10 @@ findPreviousPage(ApplyView& view, Keylet const& directory, SLE::ref start)
{
node = view.peek(keylet::page(directory, page));
if (!node)
{ // LCOV_EXCL_START
LogicError("Directory chain: root back-pointer broken.");
// LCOV_EXCL_STOP
}
}
auto indexes = node->getFieldV256(sfIndexes);
@@ -59,7 +62,7 @@ insertKey(
if (preserveOrder)
{
if (std::find(indexes.begin(), indexes.end(), key) != indexes.end())
LogicError("dirInsert: double insertion");
LogicError("dirInsert: double insertion"); // LCOV_EXCL_LINE
indexes.push_back(key);
}
@@ -73,7 +76,7 @@ insertKey(
auto pos = std::lower_bound(indexes.begin(), indexes.end(), key);
if (pos != indexes.end() && key == *pos)
LogicError("dirInsert: double insertion");
LogicError("dirInsert: double insertion"); // LCOV_EXCL_LINE
indexes.insert(pos, key);
}
@@ -131,8 +134,15 @@ insertPage(
// it's the default.
if (page != 1)
node->setFieldU64(sfIndexPrevious, page - 1);
XRPL_ASSERT_PARTS(
!nextPage,
"ripple::directory::insertPage",
"nextPage has default value");
/* Reserved for future use when directory pages may be inserted in
* between two other pages instead of only at the end of the chain.
if (nextPage)
node->setFieldU64(sfIndexNext, nextPage);
*/
describe(node);
view.insert(node);
@@ -197,10 +207,10 @@ ApplyView::emptyDirDelete(Keylet const& directory)
auto nextPage = node->getFieldU64(sfIndexNext);
if (nextPage == rootPage && prevPage != rootPage)
LogicError("Directory chain: fwd link broken");
LogicError("Directory chain: fwd link broken"); // LCOV_EXCL_LINE
if (prevPage == rootPage && nextPage != rootPage)
LogicError("Directory chain: rev link broken");
LogicError("Directory chain: rev link broken"); // LCOV_EXCL_LINE
// Older versions of the code would, in some cases, allow the last
// page to be empty. Remove such pages:
@@ -209,7 +219,10 @@ ApplyView::emptyDirDelete(Keylet const& directory)
auto last = peek(keylet::page(directory, nextPage));
if (!last)
{ // LCOV_EXCL_START
LogicError("Directory chain: fwd link broken.");
// LCOV_EXCL_STOP
}
if (!last->getFieldV256(sfIndexes).empty())
return false;
@@ -281,10 +294,16 @@ ApplyView::dirRemove(
if (page == rootPage)
{
if (nextPage == page && prevPage != page)
{ // LCOV_EXCL_START
LogicError("Directory chain: fwd link broken");
// LCOV_EXCL_STOP
}
if (prevPage == page && nextPage != page)
{ // LCOV_EXCL_START
LogicError("Directory chain: rev link broken");
// LCOV_EXCL_STOP
}
// Older versions of the code would, in some cases,
// allow the last page to be empty. Remove such
@@ -293,7 +312,10 @@ ApplyView::dirRemove(
{
auto last = peek(keylet::page(directory, nextPage));
if (!last)
{ // LCOV_EXCL_START
LogicError("Directory chain: fwd link broken.");
// LCOV_EXCL_STOP
}
if (last->getFieldV256(sfIndexes).empty())
{
@@ -325,10 +347,10 @@ ApplyView::dirRemove(
// This can never happen for nodes other than the root:
if (nextPage == page)
LogicError("Directory chain: fwd link broken");
LogicError("Directory chain: fwd link broken"); // LCOV_EXCL_LINE
if (prevPage == page)
LogicError("Directory chain: rev link broken");
LogicError("Directory chain: rev link broken"); // LCOV_EXCL_LINE
// This node isn't the root, so it can either be in the
// middle of the list, or at the end. Unlink it first
@@ -336,14 +358,14 @@ ApplyView::dirRemove(
// root:
auto prev = peek(keylet::page(directory, prevPage));
if (!prev)
LogicError("Directory chain: fwd link broken.");
LogicError("Directory chain: fwd link broken."); // LCOV_EXCL_LINE
// Fix previous to point to its new next.
prev->setFieldU64(sfIndexNext, nextPage);
update(prev);
auto next = peek(keylet::page(directory, nextPage));
if (!next)
LogicError("Directory chain: rev link broken.");
LogicError("Directory chain: rev link broken."); // LCOV_EXCL_LINE
// Fix next to point to its new previous.
next->setFieldU64(sfIndexPrevious, prevPage);
update(next);
@@ -367,7 +389,10 @@ ApplyView::dirRemove(
// And the root points to the last page:
auto root = peek(keylet::page(directory, rootPage));
if (!root)
{ // LCOV_EXCL_START
LogicError("Directory chain: root link broken.");
// LCOV_EXCL_STOP
}
root->setFieldU64(sfIndexPrevious, prevPage);
update(root);

26
src/xrpld/app/tx/detail/Transactor.cpp
View File

@@ -205,17 +205,23 @@ Transactor::preflight2(PreflightContext const& ctx)
return *ret;
// Skip signature check on batch inner transactions
if (!ctx.tx.isFlag(tfInnerBatchTxn) || !ctx.rules.enabled(featureBatch))
{
auto const sigValid = checkValidity(
ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config());
if (sigValid.first == Validity::SigBad)
{ // LCOV_EXCL_START
JLOG(ctx.j.debug())
<< "preflight2: bad signature. " << sigValid.second;
return temINVALID;
} // LCOV_EXCL_STOP
if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatch))
return tesSUCCESS;
// Do not add any checks after this point that are relevant for
// batch inner transactions. They will be skipped.
auto const sigValid = checkValidity(
ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config());
if (sigValid.first == Validity::SigBad)
{ // LCOV_EXCL_START
JLOG(ctx.j.debug()) << "preflight2: bad signature. " << sigValid.second;
return temINVALID;
// LCOV_EXCL_STOP
}
// Do not add any checks after this point that are relevant for
// batch inner transactions. They will be skipped.
return tesSUCCESS;
}