Repository navigation
Conversation
Pasting a LoanBroker or Loan ledger index into the search returned hash_not_found, although the vault page renders both and the objects resolve over RPC. A vault, a loan broker and a loan share the same 256-bit ledger index, so the vault lookup is replaced by a single ledger_entry call switched on LedgerEntryType. The request count is unchanged, and the next object type costs a case rather than a round trip. Loan brokers and loans have no page of their own, so both route to the vault that owns them.
|
@gamween Unfortunately, I hit an error while trying to use the custom Copilot setup steps configured for this repository and had to close this PR. The error I am seeing is: Once you or someone with the necessary access fixes the problem, please unassign and then reassign issue #0 to me and I'll retry. Thanks! |
There was a problem hiding this comment.
🟡 Changes recommended
Explicitly list LoanBroker in both English search-placeholder occurrences.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Extends search resolution for LoanBroker and Loan IDs to their owning vault pages.
Changes:
- Resolves vault, loan broker, and loan ledger entries.
- Adds routing and fallback tests.
- Updates English search guidance.
File summaries
| File | Summary |
|---|---|
src/containers/Header/test/Search.test.js |
Tests loan, broker, and fallback resolution. |
src/containers/Header/Search.tsx |
Routes supported ledger entries to vault pages. |
public/locales/en-US/translations.json |
Updates search-type guidance; the placeholder should explicitly mention LoanBroker. |
Review details
Suppressed comments (1)
public/locales/en-US/translations.json:77
- The not-found message likewise omits
LoanBroker, even though broker IDs now resolve through this search path. Please include Loan Brokers in the supported-type list so the message accurately describes what this search accepts.
"hash_not_found": "Could not find any Transactions, Vaults, Loans, Ledgers or NFTs that match the specified ID",
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "xrpl_org": "XRPL.org", | ||
| "github": "GitHub", | ||
| "header.search.placeholder": "Search by Token, Address, Ledger, Txn, VaultID or LedgerHash", | ||
| "header.search.placeholder": "Search by Token, Address, Ledger, Txn, Vault, Loan or LedgerHash", |
|
Both strings now name loan brokers explicitly,
Only |
High Level Overview of Change
Pasting a
LoanBrokerorLoanledger index into the search returnshash_not_found, although the explorer already renders both objects on the vault page and both resolve overledger_entry. This routes them to the vault that owns them.The vault lookup in
determineHashTypebecomes a singlegetLedgerEntrycall switched onLedgerEntryType:VaultLoanBrokerVaultIDLoanhash_not_found, unchangedThe request count is unchanged: one
ledger_entrycall served vaults before and serves all three now. ALoancosts one extra call to read its broker. The next ledger object type is acaserather than another round trip.Context of Change
The behaviour today. Type
055FDE074EC86E62F94F32D296898DA8A9D2586262AD1B912CA6066377D0BE9Finto the search box on devnet.xrpl.org and it navigates to/search/055FDE07…, the not-found page. Type the vault id6B79B084F9EA00FB3B8B368D5710F50A871D488DE43095500B38C9FAD6D2C13Cand it navigates to/vault/…, which renders that same broker under Loans, with its first-loss capital, debt ceiling and loan table. The loanA7920C443CA7AD71803F7F8E230FC7E1B53FC560EF10DD5EA59DE9D621F54165behaves the same way.Relationship to #1320. That PR deliberately made any non-
Vaultledger entry fail into not-found, after aPermissionedDomainid was landing on a vault page. This change preserves that exactly: thedefaultcase of the switch throws, so anything other than the three types falls through as before. There is a test for it, using aPermissionedDomainnode.Relationship to #1146.
feat: Add basic ledger entry pagehas been an open draft againststagingsince March 2025 and takes the general road: a/entry/:idroute for any ledger object. It predates the Vault page from #1281. The position here is narrower on purpose: a broker or a loan is better served by the vault page that already renders its cover, its debt ceiling and its loan table than by a generic field dump, and this change is a two-line switch that #1146 can absorb or replace. Happy to close this if you would rather land #1146 first.Known limitation. A vault may own more than one
LoanBroker. The vault page selects the first broker tab from local state, so a searched broker is not yet the selected tab. Anchoring it needs a tab or query parameter onVAULT_ROUTE; glad to add it here if you prefer.Translations. Only
en-USis touched. The other six locales keep their current strings, and #1291 already tracks re-translatingheader.search.placeholderafter #1281 changed it; this moves that key again and addshash_not_foundto the same list.Type of Change
Codebase Modernization
Both touched source files were already hooks and TypeScript; no conversion was needed.
Test Plan
src/containers/Header/test/Search.test.js: the file's 4 tests pass. The scenarios below are assertions added to its existingsearch valuestest, not new test cases:getLoanBrokeris called with the id named on the loanPermissionedDomainentry still falls through to/search/:id, the fix: Reject non-Vault ledger entries in getVault #1320 behaviourLoanBrokerwith noVaultIDalso falls through rather than building a broken routegetLedgerEntryin place ofgetVaultnpx eslint --max-warnings 0,npx prettier --checkandnpx tsc --buildare clean. The branch was also run against live Devnet with the objects above.Found while building a lending application on XLS-65 and XLS-66 at the XRPL Lending Protocol hackathon in Paris.