Repository navigation
Add support for Single Asset Vault deposit-blocking flags/fields - #1341
cybele-ripple wants to merge 5 commits into
Conversation
…elds Adds Vault ledger flag decoding (lsfVaultDepositBlocked, lsfVaultOwnerCanBlockDeposit) and a "Deposits Allowed" indicator on the Vault page, new tf* flag entries for VaultCreate/VaultSet/VaultDeposit, and rendering for VaultDelete's new MemoData field, matching the upstream xrpl4j changes for DGE-7974. Also fixes long Vault ID/Owner values being truncated or cut off on the Vault page and across the Vault transaction Simple views, since there's normally enough room to show them in full. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![ripple-code-reviewer[bot]](https://avatars.githubusercontent.com/u/3165523?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a clean, well-tested change. The new Vault deposit-blocking flags are decoded and gated correctly (the "Deposits Allowed" row only renders when lsfVaultOwnerCanBlockDeposit is set, and the row's inverse-of-blocked logic matches the tests), the new tf* transaction flag entries live in their own per-transaction-type namespaces with no bit collisions, and the truncation fixes (full Vault ID/Owner display, CSS wrapping via overflow-wrap: anywhere, and the new .vault-id selector) look consistent with the stated goal and are backed by updated tests. I did not find correctness or security issues in the diff. One minor doc nit below.
Covers the LendingProtocolV1_1 additions that were missing alongside the deposit-blocking work already on this branch: - VaultKind, SubscriptionDate and RedemptionDate on the Vault page and the VaultCreate simple view. Dates are rendered with the same convertRippleDate + localizeDate + DATE_OPTIONS treatment Escrow uses, so they read as timestamps rather than raw Ripple-epoch seconds. - CredentialIDs on VaultWithdraw, reusing the shared CredentialIDs row component already used by EscrowFinish and PaymentChannelClaim. VaultKind values live in a new shared vaultUtils so the Vault page and the transaction view agree on the labels. Fields absent from the xrpl package's types are read through a cast, matching the existing MemoData pattern. LEVersion is intentionally not surfaced: it selects legacy vs. cash-basis accounting for libraries decoding the ledger entry, and is noise for a human reading the explorer.
There was a problem hiding this comment.
This is a well-structured, well-tested MR that adds decoding for the new Vault deposit-blocking flags/fields, new tf* flag entries, VaultDelete MemoData rendering, and fixes truncation of long Vault IDs/Owner addresses. The logic for the new lsfVaultDepositBlocked/lsfVaultOwnerCanBlockDeposit flags is internally consistent (the 'Deposits Allowed' row is correctly gated on canBlockDeposit, and defaults sensibly when flags is undefined). The as-any casts for fields not yet in the xrpl package's types match the documented existing pattern (CredentialIDs). Test coverage for the new behavior (flag combinations, VaultKind branches, MemoData) is thorough. I did not find high-confidence correctness or security issues in the changed lines; only minor consistency/dead-code observations worth a second look.
… single-asset-vault-updates
There was a problem hiding this comment.
The change is clean overall: it correctly decodes the new Vault deposit-blocking flags, wires up new tf* flag entries consistently with the existing TX_FLAGS table, renders VaultDelete's MemoData using the same decodeHex/hexMatch pattern used elsewhere, and fixes the Vault ID/Owner truncation with reasonable CSS changes backed by before/after screenshots. Tests were updated/added for the new behavior and look thorough. I didn't find any correctness, security, or logic bugs in the added code — the as any casts are consistent with the documented pattern for fields ahead of the xrpl package's types. The one thing worth tidying is a duplicated date-formatting helper.
Collapse the multi-line union type casts flagged by prettier/prettier in the lint CI job.
There was a problem hiding this comment.
Overall a well-scoped feature addition (Vault deposit-blocking flags/fields) with good test coverage. No clear correctness, security, or data-handling bugs found in the changed lines. The main point worth double-checking is a className inconsistency on the new VaultDelete MemoData row, given this same MR is specifically fixing text-wrapping/truncation issues elsewhere.
|
Hi @cybele-ripple, I think this PR is based on an outdated issue: |
2 participants
High Level Overview of Change
Context of Change
Tracks upstream additions to Single Asset Vault deposit blocking (XLS-469, XLS-470, gated behind
LendingProtocolV1_1), mirrored in xrpl4j#832. The xrpl npm package doesn't have these fields yet, so access uses as any casts with a comment, matching the existing pattern for fields ahead of the package's types (e.g. CredentialIDs in EscrowFinish/PaymentChannelClaim).The truncation fixes were found while manually previewing this work — Vault IDs and owner addresses were either shortened unnecessarily or overflowing instead of wrapping, despite there being enough room to show them in full.
Type of Change
Codebase Modernization
Before / After
Note: The data in these screenshots is mock data that was created locally.
Single asset vault page - desktop
Single asset vault page - mobile
Vault delete - simple view
Vault delete - detailed view
Vault deposit
Test Plan
All tests pass including new/updated tests for
VaultHeaderandVaultCreate