Skip to content

feat: Remove pseudo account field filter - #8042

Open
a1q123456 wants to merge 3 commits into
developfrom
a1q123456/isPseudoAccount-remove-filter
Open

feat: Remove pseudo account field filter#8042
a1q123456 wants to merge 3 commits into
developfrom
a1q123456/isPseudoAccount-remove-filter

Conversation

@a1q123456

@a1q123456 a1q123456 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Collapse isPseudoAccount to a single no-filter function and migrate the internal call sites.

API change

bool isPseudoAccount(SLE::const_pointer sleAcct) now returns true iff sleAcct is an ltACCOUNT_ROOT and carries at least one field flagged with SField::kSmdPseudoAccount.

Migrations (behaviour-preserving)

  • MPTokenHelpers::requireAuth - pseudo-account exemption uses the no-filter check.
  • MPTokenAuthorize::preclaim - refuses to (un)authorize a pseudo-account holder via the no-filter check.
  • MPTInvariant::ValidMPTTransfer::isAuthorized - pseudo-account exemption uses the no-filter check.

Simpler any_of

The implementation moves from std::count_if(...) > 0 to std::ranges::any_of(...), matching the style used elsewhere in the codebase (MPTokenHelpers, PermissionedDEXHelpers, TokenHelpers, etc).

Context of Change

isPseudoAccount checks whether an ACCOUNT_ROOT SLE is a pseudo-account. It has offered two overloads:

  • one that takes an optional std::set<SField const*> filter to restrict the match to specific pseudo-account designator fields;
  • one convenience view wrapper mirroring that shape.

Auditing every internal call site of the filter-taking overload showed that all three callers passed the complete set of pseudo-account designator fields:

  • MPTokenHelpers.cpp - MPT requireAuth exemption
  • MPTokenAuthorize.cpp - reject MPTokenAuthorize targeting a pseudo-account
  • MPTInvariant.cpp - invariant-check exemption

The set {&sfVaultID, &sfLoanBrokerID, &sfAMMID} they passed is exactly the value returned by getPseudoAccountFields(), so passing the filter was behaviourally equivalent to passing no filter. The filter overload therefore added an API surface with no internal justification and a subtle forward-compatibility hazard: if a new pseudo-account designator field is introduced later (a new SField flagged with SField::kSmdPseudoAccount), any hardcoded filter list would silently miss it, whereas the no-filter form derives its check directly from getPseudoAccountFields() and picks up new types automatically.

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@a1q123456
a1q123456 marked this pull request as ready for review August 17, 2026 12:47

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean, behavior-preserving simplification of isPseudoAccount: drops the now-redundant filter parameter and switches to std::ranges::any_of. All three migrated call sites pass the same field set that was previously hardcoded, so semantics are preserved. No issues found on the changed lines.

@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread include/xrpl/ledger/helpers/AccountRootHelpers.h Outdated
Comment thread src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp
Comment thread src/libxrpl/ledger/helpers/AccountRootHelpers.cpp

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean, well-scoped refactor that collapses isPseudoAccount to a single no-filter overload and updates the three internal call sites to match. The filter previously passed at each call site ({&sfVaultID, &sfLoanBrokerID, &sfAMMID}) is exactly equal to getPseudoAccountFields(), so the migration is behavior-preserving as described. The std::count_if > 0 -> std::ranges::any_of simplification is a correct, equivalent transformation and matches existing codebase style. No security, correctness, or resource issues found in the changed lines.

@Tapanito Tapanito added this to the 3.4.0 milestone Aug 20, 2026

@gregtatcam gregtatcam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants