[CAP-83] Add StellarValue type representing a skipped ledger#1904
[CAP-83] Add StellarValue type representing a skipped ledger#1904bboston7 wants to merge 1 commit intostellar:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Core Advancement Proposal (CAP) draft to the core/ protocol-spec documentation describing a StellarValue extension for explicitly skipping a ledger in SCP, and updates the CAP index to reference it.
Changes:
- Add draft CAP-0083 specifying
STELLAR_VALUE_SKIPand its intended semantics. - Update
core/README.mdto include CAP-0083 in the Draft Proposals table.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| core/README.md | Adds CAP-0083 to the draft CAP index table. |
| core/cap-0083.md | Introduces the CAP-0083 draft with proposed XDR changes and protocol semantics. |
| | [CAP-0060](cap-0060.md) | Update to Wasmi register machine| Graydon Hoare | Accepted | | ||
| | [CAP-0071](cap-0071.md) | Authentication delegation for custom accounts | Dmytro Kozhevin | Draft | | ||
| | [CAP-0072](cap-0072.md) | Contract signers for Stellar accounts | Dmytro Kozhevin | Draft | | ||
| | [CAP-0083](cap-0083.md) | Add `StellarValue` type representing a skipped leger | Brett Boston | Draft | |
There was a problem hiding this comment.
Spelling typo in the CAP index entry: "leger" should be "ledger" to match the CAP title and avoid broken searchability.
| | [CAP-0083](cap-0083.md) | Add `StellarValue` type representing a skipped leger | Brett Boston | Draft | | |
| | [CAP-0083](cap-0083.md) | Add `StellarValue` type representing a skipped ledger | Brett Boston | Draft | |
| ## Preamble | ||
|
|
||
| ``` | ||
| CAP: 0083 | ||
| Title: Add StellarValue type representing a skipped ledger |
There was a problem hiding this comment.
The PR description points readers to core/cap-0083.mkd, but the file added/linked in the repo is core/cap-0083.md. Please update the PR description (or rename the file) so the reference is correct.
| ## Simple Summary | ||
|
|
||
| This CAP introduces a new `StellarValue` type to allow validators to explicitly | ||
| skip a ledger being voted on. |
There was a problem hiding this comment.
Wording is grammatically off: "skip a ledger being voted on" reads awkwardly and could be misread. Consider changing to "skip the ledger being voted on" (or similar) for clarity.
| skip a ledger being voted on. | |
| skip the ledger that is being voted on. |
| This CAP introduces a new `StellarValue` type `STELLAR_VALUE_SKIP`. With this | ||
| change, validators may switch their ballot to a `STELLAR_VALUE_SKIP` during the | ||
| PREPARE phase of balloting prior to setting `c`[^1]. | ||
| `STELLAR_VALUE_SKIP` includes information about the value that a validator was | ||
| considering before switching to voting to skip the ledger. |
There was a problem hiding this comment.
Technical precision: STELLAR_VALUE_SKIP is an enum value / StellarValue union arm (selected by StellarValueType), not a new "StellarValue type". Using "variant" / "extension" terminology here would avoid confusion about what is being added.
| ### XDR changes | ||
|
|
||
| To enable ledger skipping, this CAP proposes the following change to | ||
| `Stellar-ledger.x`: | ||
|
|
||
| ```diff | ||
| diff --git a/Stellar-ledger.x b/Stellar-ledger.x | ||
| index a17036b..667f8e7 100644 | ||
| --- a/Stellar-ledger.x | ||
| +++ b/Stellar-ledger.x | ||
| @@ -13,7 +13,8 @@ typedef opaque UpgradeType<128>; |
There was a problem hiding this comment.
The XDR diff code fence is missing the mddiffcheck metadata (mddiffcheck.base=... and optionally mddiffcheck.fetch=...) that other CAPs use to pass the mddiffcheck CI job. Without it, mddiffcheck core/*.md may fail because it can’t validate/apply the patch against stellar-core/stellar-xdr.
|
|
||
| ### Downstream Impact | ||
|
|
||
| Downstream consumers of `StellarValue`s may require modification. Specifically, those that reason about `txSetHash` will need to handle the new `0x0` value, and those that reason about `ext` will need to handle the new `STELLAR_VALUE_SKIP` extension. |
There was a problem hiding this comment.
Hash is a 32-byte value; describing the skipped-ledger txSetHash as 0x0 is ambiguous. Please clarify it is an all-zero hash (and mirror that wording in the downstream impact section that mentions a "new 0x0 value").
| Downstream consumers of `StellarValue`s may require modification. Specifically, those that reason about `txSetHash` will need to handle the new `0x0` value, and those that reason about `ext` will need to handle the new `STELLAR_VALUE_SKIP` extension. | |
| Downstream consumers of `StellarValue`s may require modification. Specifically, those that reason about `txSetHash` will need to handle the new all-zero hash value (32 bytes of `0x00`), and those that reason about `ext` will need to handle the new `STELLAR_VALUE_SKIP` extension. |
| There are no backwards incompatibilities with this change. | ||
|
|
There was a problem hiding this comment.
This section contradicts the preceding "Downstream Impact" text: adding a new StellarValueType arm means older parsers/consumers that don’t recognize STELLAR_VALUE_SKIP will fail unless upgraded. Please clarify what is (and isn’t) backward compatible (protocol upgrade boundary vs. downstream tooling).
| There are no backwards incompatibilities with this change. | |
| This change is gated behind a new protocol version. Ledgers closed before activation are unaffected, but once the new protocol version is enabled all validators must run Stellar Core versions that understand the `STELLAR_VALUE_SKIP` `StellarValueType` arm. | |
| Downstream tooling and services that parse or reason about `StellarValue` XDR (for example via `LedgerHeader` or `LedgerCloseMeta`) must also be upgraded to handle `STELLAR_VALUE_SKIP`. Older parsers that do not recognize this arm will fail to decode or correctly interpret ledgers that were skipped, and therefore are not compatible with post-activation history. |
|
|
||
| When the network votes to skip the ledger, validators will externalize a `StellarValue` with fields set as follows: | ||
|
|
||
| * `txSetHash` : Set to `0x0` |
There was a problem hiding this comment.
txSetHash is a Hash (32 bytes). Saying it is set to 0x0 is ambiguous; consider specifying it as an all-zero 32-byte hash (e.g., 0x00..00) so implementers don’t interpret it as a single-byte/word value.
| * `txSetHash` : Set to `0x0` | |
| * `txSetHash` : Set to the all-zero 32-byte hash (`0x0000000000000000000000000000000000000000000000000000000000000000`) |
See
core/cap-0083.mkdfor details.