Skip to content

Commit 9874d47

Browse files
a1q123456a1q123456
authored
Decouple CredentialHelpers from xrpld/app/tx (XRPLF#5487)
This PR refactors `CredentialHelpers` and removes some unnecessary dependencies as a step of modularization. The ledger component is almost independent except that it references `MPTokenAuthorize` and `CredentialHelpers.h`, and the latter further references `Transactor.h`. This PR partially clears the path to modularizing the ledger component and decouples `CredentialHelpers` from xrpld.
1 parent c2f3e2e commit 9874d47

File tree

6 files changed

+87
-43
lines changed

6 files changed

+87
-43
lines changed

‎src/xrpld/app/misc/CredentialHelpers.cpp‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,15 @@ deleteSLE(
120120
}
121121

122122
NotTEC
123-
checkFields(PreflightContext const& ctx)
123+
checkFields(STTx const& tx, beast::Journal j)
124124
{
125-
if (!ctx.tx.isFieldPresent(sfCredentialIDs))
125+
if (!tx.isFieldPresent(sfCredentialIDs))
126126
return tesSUCCESS;
127127

128-
auto const& credentials = ctx.tx.getFieldV256(sfCredentialIDs);
128+
auto const& credentials = tx.getFieldV256(sfCredentialIDs);
129129
if (credentials.empty() || (credentials.size() > maxCredentialsArraySize))
130130
{
131-
JLOG(ctx.j.trace())
131+
JLOG(j.trace())
132132
<< "Malformed transaction: Credentials array size is invalid: "
133133
<< credentials.size();
134134
return temMALFORMED;
@@ -140,7 +140,7 @@ checkFields(PreflightContext const& ctx)
140140
auto [it, ins] = duplicates.insert(cred);
141141
if (!ins)
142142
{
143-
JLOG(ctx.j.trace())
143+
JLOG(j.trace())
144144
<< "Malformed transaction: duplicates in credentials.";
145145
return temMALFORMED;
146146
}
@@ -150,32 +150,36 @@ checkFields(PreflightContext const& ctx)
150150
}
151151

152152
TER
153-
valid(PreclaimContext const& ctx, AccountID const& src)
153+
valid(
154+
STTx const& tx,
155+
ReadView const& view,
156+
AccountID const& src,
157+
beast::Journal j)
154158
{
155-
if (!ctx.tx.isFieldPresent(sfCredentialIDs))
159+
if (!tx.isFieldPresent(sfCredentialIDs))
156160
return tesSUCCESS;
157161

158-
auto const& credIDs(ctx.tx.getFieldV256(sfCredentialIDs));
162+
auto const& credIDs(tx.getFieldV256(sfCredentialIDs));
159163
for (auto const& h : credIDs)
160164
{
161-
auto const sleCred = ctx.view.read(keylet::credential(h));
165+
auto const sleCred = view.read(keylet::credential(h));
162166
if (!sleCred)
163167
{
164-
JLOG(ctx.j.trace()) << "Credential doesn't exist. Cred: " << h;
168+
JLOG(j.trace()) << "Credential doesn't exist. Cred: " << h;
165169
return tecBAD_CREDENTIALS;
166170
}
167171

168172
if (sleCred->getAccountID(sfSubject) != src)
169173
{
170-
JLOG(ctx.j.trace())
174+
JLOG(j.trace())
171175
<< "Credential doesn't belong to the source account. Cred: "
172176
<< h;
173177
return tecBAD_CREDENTIALS;
174178
}
175179

176180
if (!(sleCred->getFlags() & lsfAccepted))
177181
{
178-
JLOG(ctx.j.trace()) << "Credential isn't accepted. Cred: " << h;
182+
JLOG(j.trace()) << "Credential isn't accepted. Cred: " << h;
179183
return tecBAD_CREDENTIALS;
180184
}
181185

@@ -352,35 +356,34 @@ verifyValidDomain(
352356

353357
TER
354358
verifyDepositPreauth(
355-
ApplyContext& ctx,
359+
STTx const& tx,
360+
ApplyView& view,
356361
AccountID const& src,
357362
AccountID const& dst,
358-
std::shared_ptr<SLE> const& sleDst)
363+
std::shared_ptr<SLE> const& sleDst,
364+
beast::Journal j)
359365
{
360366
// If depositPreauth is enabled, then an account that requires
361367
// authorization has at least two ways to get a payment in:
362368
// 1. If src == dst, or
363369
// 2. If src is deposit preauthorized by dst (either by account or by
364370
// credentials).
365371

366-
bool const credentialsPresent = ctx.tx.isFieldPresent(sfCredentialIDs);
372+
bool const credentialsPresent = tx.isFieldPresent(sfCredentialIDs);
367373

368374
if (credentialsPresent &&
369-
credentials::removeExpired(
370-
ctx.view(), ctx.tx.getFieldV256(sfCredentialIDs), ctx.journal))
375+
credentials::removeExpired(view, tx.getFieldV256(sfCredentialIDs), j))
371376
return tecEXPIRED;
372377

373378
if (sleDst && (sleDst->getFlags() & lsfDepositAuth))
374379
{
375380
if (src != dst)
376381
{
377-
if (!ctx.view().exists(keylet::depositPreauth(dst, src)))
382+
if (!view.exists(keylet::depositPreauth(dst, src)))
378383
return !credentialsPresent
379384
? tecNO_PERMISSION
380385
: credentials::authorizedDepositPreauth(
381-
ctx.view(),
382-
ctx.tx.getFieldV256(sfCredentialIDs),
383-
dst);
386+
view, tx.getFieldV256(sfCredentialIDs), dst);
384387
}
385388
}
386389

‎src/xrpld/app/misc/CredentialHelpers.h‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,16 @@
2020
#ifndef RIPPLE_APP_MISC_CREDENTIALHELPERS_H_INCLUDED
2121
#define RIPPLE_APP_MISC_CREDENTIALHELPERS_H_INCLUDED
2222

23-
#include <xrpld/app/tx/detail/Transactor.h>
23+
#include <xrpld/ledger/ApplyView.h>
24+
#include <xrpld/ledger/ReadView.h>
25+
26+
#include <xrpl/basics/Log.h>
27+
#include <xrpl/basics/base_uint.h>
28+
#include <xrpl/beast/utility/Journal.h>
29+
#include <xrpl/protocol/AccountID.h>
30+
#include <xrpl/protocol/STArray.h>
31+
#include <xrpl/protocol/STTx.h>
32+
#include <xrpl/protocol/TER.h>
2433

2534
namespace ripple {
2635
namespace credentials {
@@ -48,13 +57,17 @@ deleteSLE(
4857

4958
// Amendment and parameters checks for sfCredentialIDs field
5059
NotTEC
51-
checkFields(PreflightContext const& ctx);
60+
checkFields(STTx const& tx, beast::Journal j);
5261

5362
// Accessing the ledger to check if provided credentials are valid. Do not use
5463
// in doApply (only in preclaim) since it does not remove expired credentials.
5564
// If you call it in prelaim, you also must call verifyDepositPreauth in doApply
5665
TER
57-
valid(PreclaimContext const& ctx, AccountID const& src);
66+
valid(
67+
STTx const& tx,
68+
ReadView const& view,
69+
AccountID const& src,
70+
beast::Journal j);
5871

5972
// Check if subject has any credential maching the given domain. If you call it
6073
// in preclaim and it returns tecEXPIRED, you should call verifyValidDomain in
@@ -93,10 +106,12 @@ verifyValidDomain(
93106
// Check expired credentials and for existing DepositPreauth ledger object
94107
TER
95108
verifyDepositPreauth(
96-
ApplyContext& ctx,
109+
STTx const& tx,
110+
ApplyView& view,
97111
AccountID const& src,
98112
AccountID const& dst,
99-
std::shared_ptr<SLE> const& sleDst);
113+
std::shared_ptr<SLE> const& sleDst,
114+
beast::Journal j);
100115

101116
} // namespace ripple
102117

‎src/xrpld/app/tx/detail/DeleteAccount.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ DeleteAccount::preflight(PreflightContext const& ctx)
5858
// An account cannot be deleted and give itself the resulting XRP.
5959
return temDST_IS_SRC;
6060

61-
if (auto const err = credentials::checkFields(ctx); !isTesSuccess(err))
61+
if (auto const err = credentials::checkFields(ctx.tx, ctx.j);
62+
!isTesSuccess(err))
6263
return err;
6364

6465
return preflight2(ctx);
@@ -241,7 +242,8 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
241242
return tecDST_TAG_NEEDED;
242243

243244
// If credentials are provided - check them anyway
244-
if (auto const err = credentials::valid(ctx, account); !isTesSuccess(err))
245+
if (auto const err = credentials::valid(ctx.tx, ctx.view, account, ctx.j);
246+
!isTesSuccess(err))
245247
return err;
246248

247249
// if credentials then postpone auth check to doApply, to check for expired
@@ -376,7 +378,8 @@ DeleteAccount::doApply()
376378
if (ctx_.view().rules().enabled(featureDepositAuth) &&
377379
ctx_.tx.isFieldPresent(sfCredentialIDs))
378380
{
379-
if (auto err = verifyDepositPreauth(ctx_, account_, dstID, dst);
381+
if (auto err = verifyDepositPreauth(
382+
ctx_.tx, ctx_.view(), account_, dstID, dst, ctx_.journal);
380383
!isTesSuccess(err))
381384
return err;
382385
}

‎src/xrpld/app/tx/detail/Escrow.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,8 @@ EscrowFinish::preflight(PreflightContext const& ctx)
672672
}
673673
}
674674

675-
if (auto const err = credentials::checkFields(ctx); !isTesSuccess(err))
675+
if (auto const err = credentials::checkFields(ctx.tx, ctx.j);
676+
!isTesSuccess(err))
676677
return err;
677678

678679
return tesSUCCESS;
@@ -761,7 +762,8 @@ EscrowFinish::preclaim(PreclaimContext const& ctx)
761762
{
762763
if (ctx.view.rules().enabled(featureCredentials))
763764
{
764-
if (auto const err = credentials::valid(ctx, ctx.tx[sfAccount]);
765+
if (auto const err =
766+
credentials::valid(ctx.tx, ctx.view, ctx.tx[sfAccount], ctx.j);
765767
!isTesSuccess(err))
766768
return err;
767769
}
@@ -1107,7 +1109,8 @@ EscrowFinish::doApply()
11071109

11081110
if (ctx_.view().rules().enabled(featureDepositAuth))
11091111
{
1110-
if (auto err = verifyDepositPreauth(ctx_, account_, destID, sled);
1112+
if (auto err = verifyDepositPreauth(
1113+
ctx_.tx, ctx_.view(), account_, destID, sled, ctx_.journal);
11111114
!isTesSuccess(err))
11121115
return err;
11131116
}

‎src/xrpld/app/tx/detail/PayChan.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,8 @@ PayChanClaim::preflight(PreflightContext const& ctx)
473473
return temBAD_SIGNATURE;
474474
}
475475

476-
if (auto const err = credentials::checkFields(ctx); !isTesSuccess(err))
476+
if (auto const err = credentials::checkFields(ctx.tx, ctx.j);
477+
!isTesSuccess(err))
477478
return err;
478479

479480
return preflight2(ctx);
@@ -485,7 +486,8 @@ PayChanClaim::preclaim(PreclaimContext const& ctx)
485486
if (!ctx.view.rules().enabled(featureCredentials))
486487
return Transactor::preclaim(ctx);
487488

488-
if (auto const err = credentials::valid(ctx, ctx.tx[sfAccount]);
489+
if (auto const err =
490+
credentials::valid(ctx.tx, ctx.view, ctx.tx[sfAccount], ctx.j);
489491
!isTesSuccess(err))
490492
return err;
491493

@@ -554,7 +556,8 @@ PayChanClaim::doApply()
554556

555557
if (depositAuth)
556558
{
557-
if (auto err = verifyDepositPreauth(ctx_, txAccount, dst, sled);
559+
if (auto err = verifyDepositPreauth(
560+
ctx_.tx, ctx_.view(), txAccount, dst, sled, ctx_.journal);
558561
!isTesSuccess(err))
559562
return err;
560563
}

‎src/xrpld/app/tx/detail/Payment.cpp‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,8 @@ Payment::preflight(PreflightContext const& ctx)
238238
}
239239
}
240240

241-
if (auto const err = credentials::checkFields(ctx); !isTesSuccess(err))
241+
if (auto const err = credentials::checkFields(ctx.tx, ctx.j);
242+
!isTesSuccess(err))
242243
return err;
243244

244245
return preflight2(ctx);
@@ -358,7 +359,8 @@ Payment::preclaim(PreclaimContext const& ctx)
358359
}
359360
}
360361

361-
if (auto const err = credentials::valid(ctx, ctx.tx[sfAccount]);
362+
if (auto const err =
363+
credentials::valid(ctx.tx, ctx.view, ctx.tx[sfAccount], ctx.j);
362364
!isTesSuccess(err))
363365
return err;
364366

@@ -450,8 +452,13 @@ Payment::doApply()
450452
// 1. If Account == Destination, or
451453
// 2. If Account is deposit preauthorized by destination.
452454

453-
if (auto err =
454-
verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst);
455+
if (auto err = verifyDepositPreauth(
456+
ctx_.tx,
457+
ctx_.view(),
458+
account_,
459+
dstAccountID,
460+
sleDst,
461+
ctx_.journal);
455462
!isTesSuccess(err))
456463
return err;
457464
}
@@ -521,8 +528,13 @@ Payment::doApply()
521528
ter != tesSUCCESS)
522529
return ter;
523530

524-
if (auto err =
525-
verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst);
531+
if (auto err = verifyDepositPreauth(
532+
ctx_.tx,
533+
ctx_.view(),
534+
account_,
535+
dstAccountID,
536+
sleDst,
537+
ctx_.journal);
526538
!isTesSuccess(err))
527539
return err;
528540

@@ -644,8 +656,13 @@ Payment::doApply()
644656
if (dstAmount > dstReserve ||
645657
sleDst->getFieldAmount(sfBalance) > dstReserve)
646658
{
647-
if (auto err =
648-
verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst);
659+
if (auto err = verifyDepositPreauth(
660+
ctx_.tx,
661+
ctx_.view(),
662+
account_,
663+
dstAccountID,
664+
sleDst,
665+
ctx_.journal);
649666
!isTesSuccess(err))
650667
return err;
651668
}

0 commit comments

Comments
 (0)