Skip to content

Commit

Permalink
Move DepositPreauth diApply checks to credentials::verify()
Browse files Browse the repository at this point in the history
  • Loading branch information
@oleks-rip
oleks-rip committed Nov 6, 2024
1 parent 67d8af7 commit e34a1c8
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 96 deletions.
43 changes: 43 additions & 0 deletions src/xrpld/app/misc/CredentialHelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,49 @@ authorized(ApplyContext const& ctx, AccountID const& dst)
return tesSUCCESS;
}

TER
verify(
ApplyContext& ctx,
AccountID const& src,
AccountID const& dst,
bool requireAuth)
{
// If depositPreauth is enabled, then an account that requires
// authorization has at least two ways to get a payment in:
// 1. If src == dst, or
// 2. If src is deposit preauthorized by dst.

bool const credentialsPresent = ctx.tx.isFieldPresent(sfCredentialIDs);

if (credentialsPresent &&
credentials::removeExpired(ctx.view(), ctx.tx, ctx.journal))
return tecEXPIRED;

if (!requireAuth || (src == dst))
return tesSUCCESS;

if (ctx.view().exists(keylet::depositPreauth(dst, src)))
return tesSUCCESS;

if (!credentialsPresent)
return tecNO_PERMISSION;

return credentials::authorized(ctx, dst);
}

TER
verify(
ApplyContext& ctx,
AccountID const& src,
AccountID const& dst,
std::optional<std::reference_wrapper<std::shared_ptr<SLE> const>> sleDstOpt)
{
std::shared_ptr<SLE const> const& sleDst =
sleDstOpt ? *sleDstOpt : ctx.view().peek(keylet::account(dst));
return verify(
ctx, src, dst, sleDst && (sleDst->getFlags() & lsfDepositAuth));
}

std::set<std::pair<AccountID, Slice>>
makeSorted(STArray const& in)
{
Expand Down
18 changes: 18 additions & 0 deletions src/xrpld/app/misc/CredentialHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

#include <xrpld/app/tx/detail/Transactor.h>

#include <optional>

namespace ripple {
namespace credentials {

Expand Down Expand Up @@ -58,6 +60,22 @@ valid(PreclaimContext const& ctx, AccountID const& src);
TER
authorized(ApplyContext const& ctx, AccountID const& dst);

// Check expired credentials and for existing DepositPreauth ledger object
TER
verify(
ApplyContext& ctx,
AccountID const& src,
AccountID const& dst,
std::optional<std::reference_wrapper<std::shared_ptr<SLE> const>>
sleDstOpt = {});

TER
verify(
ApplyContext& ctx,
AccountID const& src,
AccountID const& dst,
bool requireAuth);

// return empty set if there are duplicates
std::set<std::pair<AccountID, Slice>>
makeSorted(STArray const& in);
Expand Down
21 changes: 7 additions & 14 deletions src/xrpld/app/tx/detail/DeleteAccount.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
if (auto const err = credentials::valid(ctx, account); !isTesSuccess(err))
return err;

// if credentials then postpone auth check to doApply
// if credentials then postpone auth check to doApply, to check for expired
// credentials
if (!ctx.tx.isFieldPresent(sfCredentialIDs))
{
// Check whether the destination account requires deposit authorization.
Expand Down Expand Up @@ -353,20 +354,12 @@ DeleteAccount::doApply()
if (!src || !dst)
return tefBAD_LEDGER;

if (ctx_.tx.isFieldPresent(sfCredentialIDs))
if (ctx_.view().rules().enabled(featureDepositAuth) &&
ctx_.tx.isFieldPresent(sfCredentialIDs))
{
if (credentials::removeExpired(view(), ctx_.tx, j_))
return tecEXPIRED;

// Check whether the destination account requires deposit authorization.
if (ctx_.view().rules().enabled(featureDepositAuth) &&
(dst->getFlags() & lsfDepositAuth) &&
!ctx_.view().exists(keylet::depositPreauth(dstID, account_)))
{
if (auto err = credentials::authorized(ctx_, dstID);
!isTesSuccess(err))
return err;
}
if (auto err = credentials::verify(ctx_, account_, dstID, dst);
!isTesSuccess(err))
return err;
}

Keylet const ownerDirKeylet{keylet::ownerDir(account_)};
Expand Down
25 changes: 3 additions & 22 deletions src/xrpld/app/tx/detail/Escrow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,28 +477,9 @@ EscrowFinish::doApply()

if (ctx_.view().rules().enabled(featureDepositAuth))
{
// Is EscrowFinished authorized?
if (sled->getFlags() & lsfDepositAuth)
{
// A destination account that requires authorization has two
// ways to get an EscrowFinished into the account:
// 1. If Account == Destination, or
// 2. If Account is deposit preauthorized by destination.
if (account_ != destID)
{
if (!view().exists(keylet::depositPreauth(destID, account_)))
{
if (!ctx_.tx.isFieldPresent(sfCredentialIDs))
return tecNO_PERMISSION;

if (credentials::removeExpired(view(), ctx_.tx, j_))
return tecEXPIRED;
if (auto err = credentials::authorized(ctx_, destID);
!isTesSuccess(err))
return err;
}
}
}
if (auto err = credentials::verify(ctx_, account_, destID, sled);
!isTesSuccess(err))
return err;
}

AccountID const account = (*slep)[sfAccount];
Expand Down
25 changes: 5 additions & 20 deletions src/xrpld/app/tx/detail/PayChan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,27 +537,12 @@ PayChanClaim::doApply()
(txAccount == src && (sled->getFlags() & lsfDisallowXRP)))
return tecNO_TARGET;

// Check whether the destination account requires deposit authorization.
if (depositAuth && (sled->getFlags() & lsfDepositAuth))
if (depositAuth)
{
// A destination account that requires authorization has two
// ways to get a Payment Channel Claim into the account:
// 1. If Account == Destination, or
// 2. If Account is deposit preauthorized by destination.
if (txAccount != dst)
{
if (!view().exists(keylet::depositPreauth(dst, txAccount)))
{
if (!ctx_.tx.isFieldPresent(sfCredentialIDs))
return tecNO_PERMISSION;

if (credentials::removeExpired(view(), ctx_.tx, j_))
return tecEXPIRED;
if (auto err = credentials::authorized(ctx_, dst);
!isTesSuccess(err))
return err;
}
}
if (auto err = credentials::verify(
ctx_, txAccount, dst, sled->getFlags() & lsfDepositAuth);
!isTesSuccess(err))
return err;
}

(*slep)[sfBalance] = ctx_.tx[sfBalance];
Expand Down
60 changes: 20 additions & 40 deletions src/xrpld/app/tx/detail/Payment.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,9 @@ Payment::doApply()
}

// Determine whether the destination requires deposit authorization.
bool const reqDepositAuth = sleDst->getFlags() & lsfDepositAuth &&
view().rules().enabled(featureDepositAuth);
bool const depositAuth = view().rules().enabled(featureDepositAuth);
bool const reqDepositAuth =
sleDst->getFlags() & lsfDepositAuth && depositAuth;

bool const depositPreauth = view().rules().enabled(featureDepositPreauth);

Expand All @@ -387,34 +388,22 @@ Payment::doApply()
if (!depositPreauth && ripple && reqDepositAuth)
return tecNO_PERMISSION;

bool const credentialsPresent = ctx_.tx.isFieldPresent(sfCredentialIDs);

if (ripple)
{
// Ripple payment with at least one intermediate step and uses
// transitive balances.

if (depositPreauth && reqDepositAuth)
if (depositPreauth && depositAuth)
{
// If depositPreauth is enabled, then an account that requires
// authorization has two ways to get an IOU Payment in:
// 1. If Account == Destination, or
// 2. If Account is deposit preauthorized by destination.
if (dstAccountID != account_)
{
if (!view().exists(
keylet::depositPreauth(dstAccountID, account_)))
{
if (!credentialsPresent)
return tecNO_PERMISSION;

if (credentials::removeExpired(view(), ctx_.tx, j_))
return tecEXPIRED;
if (auto err = credentials::authorized(ctx_, dstAccountID);
!isTesSuccess(err))
return err;
}
}

if (auto err = credentials::verify(
ctx_, account_, dstAccountID, reqDepositAuth);
!isTesSuccess(err))
return err;
}

path::RippleCalc::Input rcInput;
Expand Down Expand Up @@ -570,7 +559,7 @@ Payment::doApply()

// The source account does have enough money. Make sure the
// source account has authority to deposit to the destination.
if (reqDepositAuth)
if (depositAuth)
{
// If depositPreauth is enabled, then an account that requires
// authorization has three ways to get an XRP Payment in:
Expand All @@ -590,26 +579,17 @@ Payment::doApply()
// We choose the base reserve as our bound because it is
// a small number that seldom changes but is always sufficient
// to get the account un-wedged.
if (dstAccountID != account_)

// Get the base reserve.
XRPAmount const dstReserve{view().fees().accountReserve(0)};

if (dstAmount > dstReserve ||
sleDst->getFieldAmount(sfBalance) > dstReserve)
{
if (!view().exists(keylet::depositPreauth(dstAccountID, account_)))
{
// Get the base reserve.
XRPAmount const dstReserve{view().fees().accountReserve(0)};

if (dstAmount > dstReserve ||
sleDst->getFieldAmount(sfBalance) > dstReserve)
{
if (!credentialsPresent)
return tecNO_PERMISSION;

if (credentials::removeExpired(view(), ctx_.tx, j_))
return tecEXPIRED;
if (auto err = credentials::authorized(ctx_, dstAccountID);
!isTesSuccess(err))
return err;
}
}
if (auto err = credentials::verify(
ctx_, account_, dstAccountID, reqDepositAuth);
!isTesSuccess(err))
return err;
}
}

Expand Down

0 comments on commit e34a1c8

Please sign in to comment.