refactor: Add initial wrapper classes for all SLEs - #7886
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new SLEBase<ViewT> template and a set of thin, per-ledger-entry wrapper classes (e.g., AccountRootEntry<ViewT>, OfferEntry<ViewT>) intended as the first step toward replacing direct use of raw STLedgerEntry/SLE across the codebase.
Changes:
- Added
include/xrpl/ledger/helpers/SLEBase.h, providing a view-parameterized wrapper with compile-time gated writable operations. - Added initial wrapper headers for a broad set of ledger entry (SLE) types, each providing a convenience constructor that resolves the entry from a
Keylet/keylet helper against a view. - Established
ReadOnlySLEandWritableSLEaliases as generic wrappers for unknown concrete ledger entry types.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 32 comments.
Show a summary per file
| File | Description |
|---|---|
| include/xrpl/ledger/helpers/SLEBase.h | Introduces the core view-parameterized base wrapper for SLE access/mutation. |
| include/xrpl/ledger/helpers/AccountRootEntry.h | Adds an AccountRoot SLE wrapper with a keylet::account constructor. |
| include/xrpl/ledger/helpers/AMMEntry.h | Adds an AMM SLE wrapper with a keylet::amm constructor. |
| include/xrpl/ledger/helpers/AmendmentsEntry.h | Adds an Amendments SLE wrapper with a keylet::amendments constructor. |
| include/xrpl/ledger/helpers/BridgeEntry.h | Adds a Bridge SLE wrapper with a keylet::bridge constructor. |
| include/xrpl/ledger/helpers/CheckEntry.h | Adds a Check SLE wrapper with a keylet::check constructor. |
| include/xrpl/ledger/helpers/CredentialEntry.h | Adds a Credential SLE wrapper with a keylet::credential constructor. |
| include/xrpl/ledger/helpers/DelegateEntry.h | Adds a Delegate SLE wrapper with a keylet::delegate constructor. |
| include/xrpl/ledger/helpers/DepositPreauthEntry.h | Adds a DepositPreauth SLE wrapper with a keylet::depositPreauth constructor. |
| include/xrpl/ledger/helpers/DIDEntry.h | Adds a DID SLE wrapper with a keylet::did constructor. |
| include/xrpl/ledger/helpers/DirectoryNodeEntry.h | Adds a DirectoryNode SLE wrapper with a keylet::ownerDir constructor. |
| include/xrpl/ledger/helpers/EscrowEntry.h | Adds an Escrow SLE wrapper with a keylet::escrow constructor. |
| include/xrpl/ledger/helpers/FeeSettingsEntry.h | Adds a FeeSettings SLE wrapper with a keylet::feeSettings constructor. |
| include/xrpl/ledger/helpers/LedgerHashesEntry.h | Adds a LedgerHashes/SkipList SLE wrapper with a keylet::skip constructor. |
| include/xrpl/ledger/helpers/LoanBrokerEntry.h | Adds a LoanBroker SLE wrapper with a keylet::loanBroker constructor. |
| include/xrpl/ledger/helpers/LoanEntry.h | Adds a Loan SLE wrapper with a keylet::loan constructor. |
| include/xrpl/ledger/helpers/MPTokenEntry.h | Adds an MPToken SLE wrapper with a keylet::mptoken constructor. |
| include/xrpl/ledger/helpers/MPTokenIssuanceEntry.h | Adds an MPTokenIssuance SLE wrapper with a keylet::mptokenIssuance constructor. |
| include/xrpl/ledger/helpers/NegativeUNLEntry.h | Adds a NegativeUNL SLE wrapper with a keylet::negativeUNL constructor. |
| include/xrpl/ledger/helpers/NFTokenOfferEntry.h | Adds an NFTokenOffer SLE wrapper with a keylet::nftokenOffer constructor. |
| include/xrpl/ledger/helpers/NFTokenPageEntry.h | Adds an NFTokenPage SLE wrapper with a keylet::nftokenPage constructor. |
| include/xrpl/ledger/helpers/OfferEntry.h | Adds an Offer SLE wrapper with a keylet::offer constructor. |
| include/xrpl/ledger/helpers/OracleEntry.h | Adds an Oracle SLE wrapper with a keylet::oracle constructor. |
| include/xrpl/ledger/helpers/PayChannelEntry.h | Adds a PayChannel SLE wrapper with a keylet::payChannel constructor. |
| include/xrpl/ledger/helpers/PermissionedDomainEntry.h | Adds a PermissionedDomain SLE wrapper with a keylet::permissionedDomain constructor. |
| include/xrpl/ledger/helpers/RippleStateEntry.h | Adds a RippleState SLE wrapper with a keylet::trustLine constructor. |
| include/xrpl/ledger/helpers/SignerListEntry.h | Adds a SignerList SLE wrapper with a keylet::signerList constructor. |
| include/xrpl/ledger/helpers/SponsorshipEntry.h | Adds a Sponsorship SLE wrapper with a keylet::sponsorship constructor. |
| include/xrpl/ledger/helpers/TicketEntry.h | Adds a Ticket SLE wrapper with a keylet::ticket constructor. |
| include/xrpl/ledger/helpers/VaultEntry.h | Adds a Vault SLE wrapper with a keylet::vault constructor. |
| include/xrpl/ledger/helpers/XChainOwnedClaimIDEntry.h | Adds an XChainOwnedClaimID SLE wrapper with a keylet::xChainClaimID constructor. |
| include/xrpl/ledger/helpers/XChainOwnedCreateAccountClaimIDEntry.h | Adds an XChainOwnedCreateAccountClaimID SLE wrapper with a keylet::xChainCreateAccountClaimID constructor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Constructor for read-only context | ||
| */ | ||
| explicit SLEBase( |
There was a problem hiding this comment.
Unchecked SLE type adoption — derived wrappers inherit constructors without validating entry type. Restrict inheritance in derived classes by removing using SLEBase<ViewT>::SLEBase; and providing Keylet-based constructors only, or add type-check assertion here:
protected:
/**
* Constructor for read-only context (internal use; derived wrappers should
* provide type-validated constructors or use Keylet-based constructors).
*/
explicit SLEBase(
SLE::const_pointer sle,
ReadView const& view,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()})
requires(!kIsWritable)
: view_(view), sle_(std::move(sle)), j_(j)
{
}
public:
| /** | ||
| * Constructor for writable context (from existing SLE) | ||
| */ | ||
| explicit SLEBase( |
There was a problem hiding this comment.
Null SLE pointer creates sentinel Keylet(ltANY), causing newSLE() to throw. Require Keylet explicitly to avoid invalid state:
explicit SLEBase(
std::shared_ptr<SLE> sle,
Keylet const& key,
ApplyView& view,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()})
requires kIsWritable
: view_(view)
, key_(key)
, sle_(std::move(sle))
, j_(j)
{
}
High Level Overview of Change
Title pretty much says it all - this PR adds some wrapper classes for all ledger entry types, intended to replace the use of raw SLEs.
Context of Change
A production-grade Step 1 version of #7791
API Impact
N/A