ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-09-27 15:28:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

docs: Fix std::unexpected wording, split UNREACHABLE out of amendment section

Browse Source 
- ledger/helpers/AGENTS.md: std::expected<..., TER>/std::unexpected(tec*)
  is already the established idiom here (confirmed in AMMHelpers.cpp,
  CredentialHelpers.cpp), not something to avoid; reword so the rule
  doesn't read as discouraging it.
- transactors/AGENTS.md: the UNREACHABLE/LCOV_EXCL coverage rule isn't
  amendment-specific, so give it its own heading instead of nesting it
  under "Gating amendment-dependent code".
- Root AGENTS.md: comments should explain why, not what/how, since the
  code already shows that.
- Drop the pseudo-account exemption rule from transactors/AGENTS.md —
  needs refining before it's codified.

Addresses PR review comments:
https://github.com/XRPLF/rippled/pull/8198#discussion_r3962352164
https://github.com/XRPLF/rippled/pull/8198#discussion_r3967566278
This commit is contained in:
Mayukha Vadari
2026-09-09 13:58:12 -04:00
parent 6b9db2460c
commit 6dc958c136
3 changed files with 5 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
AGENTS.md
View File

@@ -31,6 +31,8 @@ See [CONTRIBUTING.md](./CONTRIBUTING.md#pre-commit-hooks) for `pre-commit` setup
New file placement and header levelization: see [CONTRIBUTING.md](./CONTRIBUTING.md#before-making-a-pull-request). Braces, whitespace, member order, and other conventions: see [docs/CodingStyle.md](./docs/CodingStyle.md). `XRPL_ASSERT`/`UNREACHABLE` contracts: see [CONTRIBUTING.md](./CONTRIBUTING.md#contracts-and-instrumentation). Commit messages: see [CONTRIBUTING.md](./CONTRIBUTING.md#good-commit-messages). New public functions/methods need a Doxygen-style comment.
Comments should explain _why_, not _what_/_how_ — the code already shows that. Only describe what/how when the code itself would otherwise be confusing (a non-obvious workaround, a subtle invariant, a surprising constraint).
## Architecture
See [ARCHITECTURE.md](./ARCHITECTURE.md) for the directory-by-directory map of the codebase.

2
src/libxrpl/ledger/helpers/AGENTS.md
View File

@@ -2,7 +2,7 @@
See the repo-level [AGENTS.md](../../../../AGENTS.md) for general guidance.
A helper that takes an `SLE`/`std::shared_ptr<SLE const>` should `XRPL_ASSERT` that it's non-null and of the expected ledger-entry type at entry, and keep a real runtime check/error-return alongside the assert (asserts compile out in release builds). Don't invent a new error-handling idiom for this (e.g. `std::unexpected`) — return the existing `tec`/`ter`/`tef` code used elsewhere in the codebase.
A helper that takes an `SLE`/`std::shared_ptr<SLE const>` should `XRPL_ASSERT` that it's non-null and of the expected ledger-entry type at entry, and keep a real runtime check/error-return alongside the assert (asserts compile out in release builds) — the established idiom in this directory is `std::expected<..., TER>`, returning `std::unexpected(tec*)` on failure. Don't invent a new error-handling idiom for this.
Prefer a single amendment-enabled block and a single disabled block over scattering `rules.enabled(...)` checks through a function, even if the two blocks are similar. When a file or function checks more than one amendment, name local enablement booleans per-amendment (e.g. `fix340Enabled` for `fixCleanup3_4_0`), not a generic `fixEnabled`.

4
src/libxrpl/tx/transactors/AGENTS.md
View File

@@ -2,8 +2,6 @@
See [tx/AGENTS.md](../AGENTS.md) for amendment-gating conventions that apply to all transactors, and the repo-level [AGENTS.md](../../../../AGENTS.md) for general guidance.
Pseudo-accounts (Vault, LoanBroker, AMM, ...) are exempt from `requireAuth` and freeze/deep-freeze checks as a class, not on a per-asset-type basis. Code that touches a pseudo-account (deposits, withdrawals, clawback, deletion, credential checks) must preserve that exemption rather than re-deriving it for each asset type.
Prefer a single object-level invariant over duplicating the same delta/balance check in every transactor that touches an object — e.g. one invariant asserting a Vault's pseudo-account balance and `assetsAvailable` always move together, rather than repeating that check in `VaultDeposit`, `VaultWithdraw`, `VaultClawback`, `LoanSet`, etc.
## Gating amendment-dependent code
@@ -12,4 +10,6 @@ Prefer a single amendment-enabled block and a single disabled block over scatter
When a file or function checks more than one amendment, name local enablement booleans per-amendment (e.g. `fix340Enabled` for `fixCleanup3_4_0`), not a generic `fixEnabled` — it becomes ambiguous once a second amendment is checked in the same scope.
## `UNREACHABLE` and test coverage
Only use `UNREACHABLE` for genuinely impossible paths, not to avoid writing a test for one that's reachable but rare. When a branch marked `UNREACHABLE` is excluded from coverage, wrap it in `LCOV_EXCL_START`/`LCOV_EXCL_STOP`.