refactor: Rename (mostly keylet) functions to more closely match the docs#7059
Open
refactor: Rename (mostly keylet) functions to more closely match the docs#7059
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors internal naming (primarily keylet::* helpers) to better match documentation terminology, updating call sites across RPC handlers, ledger/tx logic, helpers, and tests to use the new names.
Changes:
- Renamed multiple
keylet::*functions (e.g.line→rippleState,fees→feeSettings,signers→signerList,payChan→payChannel, NFT page/offer helpers,mptIssuance→mptokenIssuance,loanbroker→loanBroker) and updated usages repo-wide. - Renamed NFT ID helper
nft::getSerialtonft::getSequenceand updated usages. - Updated related documentation references in protocol ledger-entry definitions.
Reviewed changes
Copilot reviewed 107 out of 107 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/xrpld/rpc/handlers/orderbook/NFTOffersHelpers.h | Update NFT offer keylet helper name. |
| src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp | Update fee settings, loan broker, MPT issuance, and ripple state keylet names. |
| src/xrpld/rpc/handlers/account/AccountObjects.cpp | Update NFT page keylet helper names. |
| src/xrpld/rpc/handlers/account/AccountNFTs.cpp | Update NFT page keylets and NFT sequence helper usage. |
| src/xrpld/rpc/handlers/account/AccountInfo.cpp | Update signer list keylet helper name. |
| src/xrpld/rpc/handlers/VaultInfo.cpp | Update MPT issuance keylet helper name. |
| src/xrpld/rpc/detail/RPCHelpers.cpp | Update signer list keylet helper name. |
| src/xrpld/rpc/detail/Pathfinder.cpp | Update trustline (ripple state) keylet helper name. |
| src/xrpld/rpc/detail/AssetCache.cpp | Update MPT issuance keylet helper name. |
| src/xrpld/app/main/Application.cpp | Update fee settings keylet helper name in invariants/asserts. |
| src/xrpld/app/ledger/detail/LedgerPersistence.cpp | Update fee settings keylet helper name in asserts. |
| src/xrpld/app/ledger/detail/InboundLedger.cpp | Update fee settings keylet helper name in asserts. |
| src/xrpld/app/ledger/detail/BuildLedger.cpp | Update fee settings keylet helper name in asserts. |
| src/tests/libxrpl/helpers/TxTest.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/rpc/Subscribe_test.cpp | Update NFT offer keylet helper name. |
| src/test/rpc/LedgerEntry_test.cpp | Update fee settings, NFT offer, NFT page, and pay channel keylet helper names. |
| src/test/rpc/AccountTx_test.cpp | Update pay channel keylet helper name. |
| src/test/jtx/impl/mpt.cpp | Update MPT issuance keylet helper name. |
| src/test/jtx/impl/balance.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/jtx/impl/TestHelpers.cpp | Update ripple state, MPT issuance, and pay channel keylet helper names. |
| src/test/jtx/impl/Env.cpp | Update ripple state and MPT issuance keylet helper names. |
| src/test/app/Vault_test.cpp | Update MPT issuance, ripple state, and loan broker keylet helper names. |
| src/test/app/SetAuth_test.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/app/RCLValidations_test.cpp | Update fee settings keylet helper name. |
| src/test/app/PayStrand_test.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/app/PayChan_test.cpp | Update pay channel keylet helper name. |
| src/test/app/Path_test.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/app/Offer_test.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/app/NFTokenDir_test.cpp | Update NFT offer keylet helper name. |
| src/test/app/NFTokenBurn_test.cpp | Update NFT offer and NFT page keylet helper names. |
| src/test/app/NFTokenAuth_test.cpp | Update NFT offer and trustline (ripple state) keylet helper names. |
| src/test/app/MultiSign_test.cpp | Update signer list keylet helper name. |
| src/test/app/MPToken_test.cpp | Update MPT issuance keylet helper name. |
| src/test/app/Loan_test.cpp | Update loan broker and trustline (ripple state) keylet helper names. |
| src/test/app/LoanBroker_test.cpp | Update loan broker keylet helper name. |
| src/test/app/LPTokenTransfer_test.cpp | Update NFT offer keylet helper name. |
| src/test/app/Invariants_test.cpp | Update ripple state, MPT issuance, NFT page, and loan broker keylet helper names. |
| src/test/app/Freeze_test.cpp | Update NFT offer keylet helper name. |
| src/test/app/Flow_test.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/app/FixNFTokenPageLinks_test.cpp | Update NFT offer and NFT page keylet helper names. |
| src/test/app/FeeVote_test.cpp | Update fee settings keylet helper name. |
| src/test/app/EscrowToken_test.cpp | Update MPT issuance keylet helper name. |
| src/test/app/Clawback_test.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/app/Check_test.cpp | Update trustline (ripple state) keylet helper name. |
| src/test/app/Batch_test.cpp | Update loan broker keylet helper name. |
| src/test/app/AccountDelete_test.cpp | Update signer list and pay channel keylet helper names. |
| src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/vault/VaultSet.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/vault/VaultDeposit.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/vault/VaultDelete.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/vault/VaultClawback.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/token/TrustSet.cpp | Update trustline (ripple state) keylet helper name. |
| src/libxrpl/tx/transactors/token/MPTokenIssuanceSet.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/token/MPTokenIssuanceDestroy.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/token/Clawback.cpp | Update ripple state and MPT issuance keylet helper names. |
| src/libxrpl/tx/transactors/system/Change.cpp | Update fee settings keylet helper name. |
| src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp | Update pay channel keylet helper name. |
| src/libxrpl/tx/transactors/nft/NFTokenCancelOffer.cpp | Update NFT offer keylet helper name. |
| src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp | Update NFT offer and ripple state keylet helper names. |
| src/libxrpl/tx/transactors/lending/LoanSet.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/transactors/lending/LoanPay.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/transactors/lending/LoanManage.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/transactors/lending/LoanDelete.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/transactors/lending/LoanBrokerDelete.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/transactors/lending/LoanBrokerCoverClawback.cpp | Update loan broker and MPT issuance keylet helper names. |
| src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp | Update ripple state and MPT issuance keylet helper names. |
| src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/dex/OfferCreate.cpp | Update ripple state keylet helper name. |
| src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp | Update ripple state and MPT issuance keylet helper names. |
| src/libxrpl/tx/transactors/dex/AMMDeposit.cpp | Update ripple state keylet helper name. |
| src/libxrpl/tx/transactors/dex/AMMCreate.cpp | Update MPT issuance and ripple state keylet helper names. |
| src/libxrpl/tx/transactors/dex/AMMClawback.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/transactors/check/CheckCreate.cpp | Update ripple state keylet helper name. |
| src/libxrpl/tx/transactors/check/CheckCash.cpp | Update ripple state keylet helper name. |
| src/libxrpl/tx/transactors/bridge/XChainBridge.cpp | Update signer list keylet helper name. |
| src/libxrpl/tx/transactors/account/SignerListSet.cpp | Update signer list keylet helper name. |
| src/libxrpl/tx/transactors/account/SetRegularKey.cpp | Update signer list keylet helper name. |
| src/libxrpl/tx/transactors/account/AccountSet.cpp | Update signer list keylet helper name. |
| src/libxrpl/tx/transactors/account/AccountDelete.cpp | Update NFT page keylet helper names. |
| src/libxrpl/tx/paths/XRPEndpointStep.cpp | Update ripple state keylet helper name. |
| src/libxrpl/tx/paths/MPTEndpointStep.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/tx/paths/DirectStep.cpp | Update ripple state keylet helper name. |
| src/libxrpl/tx/paths/BookStep.cpp | Update ripple state keylet helper name. |
| src/libxrpl/tx/invariants/VaultInvariant.cpp | Update MPT issuance and ripple state keylet helper names. |
| src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp | Update loan broker keylet helper name. |
| src/libxrpl/tx/invariants/InvariantCheck.cpp | Update NFT page keylet helper names. |
| src/libxrpl/tx/Transactor.cpp | Update signer list and NFT offer keylet helper names. |
| src/libxrpl/protocol/Indexes.cpp | Rename keylet functions at definition site. |
| src/libxrpl/ledger/helpers/TokenHelpers.cpp | Update ripple state and MPT issuance keylet helper names. |
| src/libxrpl/ledger/helpers/RippleStateHelpers.cpp | Update ripple state keylet helper name throughout. |
| src/libxrpl/ledger/helpers/NFTokenHelpers.cpp | Update NFT page/offer keylet helper names and ripple state usage. |
| src/libxrpl/ledger/helpers/MPTokenHelpers.cpp | Update MPT issuance keylet helper name throughout. |
| src/libxrpl/ledger/helpers/AMMHelpers.cpp | Update ripple state keylet helper name. |
| src/libxrpl/ledger/View.cpp | Update MPT issuance keylet helper name. |
| src/libxrpl/ledger/Ledger.cpp | Update fee settings keylet helper name. |
| include/xrpl/tx/paths/detail/StepChecks.h | Update ripple state keylet helper name. |
| include/xrpl/protocol/nft.h | Rename NFT ID accessor from serial to sequence. |
| include/xrpl/protocol/detail/ledger_entries.macro | Update docs cross-references to renamed keylets. |
| include/xrpl/protocol/Indexes.h | Update keylet declarations (note: contains a broken inline wrapper). |
| include/xrpl/ledger/helpers/EscrowHelpers.h | Update ripple state and MPT issuance keylet helper names. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 107 out of 107 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
include/xrpl/protocol/nft.h:60
- This header is installed/packaged (public API). Renaming
nft::getSerialtonft::getSequencewill break downstream code that includes this header. If API stability is a goal, consider providing a deprecatedgetSerial()wrapper that forwards togetSequence()(and update the PR description/API impact to reflect the public API change).
inline std::uint32_t
getSequence(uint256 const& id)
{
std::uint32_t seq = 0;
memcpy(&seq, id.begin() + 28, 4);
return boost::endian::big_to_native(seq);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
69
to
102
| @@ -93,12 +93,12 @@ static book_t const book{}; | |||
| */ | |||
| /** @{ */ | |||
| Keylet | |||
| line(AccountID const& id0, AccountID const& id1, Currency const& currency) noexcept; | |||
| rippleState(AccountID const& id0, AccountID const& id1, Currency const& currency) noexcept; | |||
|
|
|||
| inline Keylet | |||
| line(AccountID const& id, Issue const& issue) noexcept | |||
| rippleState(AccountID const& id, Issue const& issue) noexcept | |||
| { | |||
| return line(id, issue.account, issue.currency); | |||
| return rippleState(id, issue.account, issue.currency); | |||
| } | |||
There was a problem hiding this comment.
The PR description says these are “internal names” with “API Impact: N/A”, but this file is part of the installed/packaged public headers (see cmake/XrplInstall.cmake:35-39 and conanfile.py:144-151). Renaming exported keylet functions like fees/line/signers to feeSettings/rippleState/signerList is therefore a breaking API change for downstream consumers; please update the PR description/API impact accordingly and consider keeping deprecated compatibility wrappers (e.g., [[deprecated]] aliases) if backwards compatibility is required.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7059 +/- ##
=========================================
- Coverage 82.1% 82.1% -0.0%
=========================================
Files 1010 1010
Lines 76064 76066 +2
Branches 7390 7383 -7
=========================================
- Hits 62438 62415 -23
- Misses 13626 13651 +25
... and 10 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
Tapanito
reviewed
May 4, 2026
| /** A keylet for the owner's first possible NFT page. */ | ||
| Keylet | ||
| nftpage_min(AccountID const& owner); | ||
| nftokenPage_min(AccountID const& owner); |
Collaborator
There was a problem hiding this comment.
This should be cammel case, as per the new clang-tidy rules.
Tapanito
reviewed
May 4, 2026
| /** A keylet for the owner's last possible NFT page. */ | ||
| Keylet | ||
| nftpage_max(AccountID const& owner); | ||
| nftokenPage_max(AccountID const& owner); |
Tapanito
reviewed
May 4, 2026
|
|
||
| inline Keylet | ||
| line(AccountID const& id, Issue const& issue) noexcept | ||
| rippleState(AccountID const& id, Issue const& issue) noexcept |
Collaborator
There was a problem hiding this comment.
Could you rename this to trustLine? While rippleState is the official object name, everywhere else it's referred to as trustline.
Contributor
|
We recently merged a refactor to One-time setupIf you don't already have clang-tidy working in your env, on macOS: brew install llvm@21
# Follow brew's hint to put $(brew --prefix llvm@21)/bin on PATH so run-clang-tidy is found.Workflow on your branch (before merging develop)1. Grab the new git remote -v # should show 'upstream' among others; if not:
# git remote set-url upstream git@github.com:XRPLF/rippled.git
git fetch upstream
git checkout upstream/develop -- .clang-tidy2. Reconfigure conan/cmake so 3. Apply renames for the files modified in your PR: git diff --name-only $(git merge-base HEAD upstream/develop) HEAD \
| grep -E '\.(cpp|h|hpp|ipp)$' \
| xargs run-clang-tidy -p build -fix -allow-no-checks
# or -p .build, or whatever your build dir is called4. Build + test, then commit as a single dedicated commit: cmake --build build -j8
git commit -am "refactor: Align identifier naming with develop"5. Now merge develop: git merge upstream/developExtraRun clang-tidy once more after the merge to catch any stragglers introduced from develop's side: run-clang-tidy -p build -fix -allow-no-checks src tests
# or -p .build, or whatever your build dir is called |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
As the title says, this PR renames a bunch of functions (mostly keylet functions) to match what the docs refer to these as, so that it's easier to know what function to use when you want to use a specific keylet.
Full list of renames:
keylet::line->keylet::rippleStatekeylet::fees->keylet::feeSettingskeylet::signers->keylet::signerListkeylet::payChan->keylet::payChannelkeylet::nftpage->keylet::nftokenPagekeylet::nftoffer->keylet::nftokenOfferkeylet::mptIssuance->keylet::mptokenIssuancekeylet::loanbroker->keylet::loanBrokerThere are no functionality changes in this PR, only renames (and resulting linter changes).
Context of Change
Repo cleanup
API Impact
N/A, these are all internal names