Skip to content

update: make spec changes from BatchV1_1 - #563

Open
mvadari wants to merge 5 commits into
masterfrom
56-updates
Open

update: make spec changes from BatchV1_1#563
mvadari wants to merge 5 commits into
masterfrom
56-updates

Conversation

@mvadari

@mvadari mvadari commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

High Level Overview of Change

Title says it all

Context of Change

XRPLF/rippled#6446

Type of Change

  • XLS Update (changes to an existing XLS)

Copilot AI review requested due to automatic review settings June 16, 2026 20:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the XLS-0056 Batch specification to align with the “BatchV1_1” implementation changes referenced in the linked rippled PR, primarily by tightening BatchSigners rules and clarifying inner-transaction common-field semantics.

Changes:

  • Updates XLS metadata and removes the tentative amendment name from the overview.
  • Adds/clarifies BatchSigners requirements (sorting, signing payload definition, additional failure conditions).
  • Expands “Transaction Common Fields” to specify tfInnerBatchTxn fee and validation/failure behavior.

Comment thread XLS-0056-batch/README.md
Comment on lines 174 to +177
12. One of the inner transactions fails its preflight checks (i.e. is invalid, irrespective of ledger state) (`temINVALID_INNER_BATCH`).
13. Either both or neither of `TicketSequence` and `Sequence` are set (`temSEQ_AND_TICKET`).
1. `BatchSigners`
1. The length of `BatchSigners` is greater than the number of transactions in `RawTransactions` (`temARRAY_TOO_LARGE`).
1. The length of `BatchSigners` is greater than 8 (`temARRAY_TOO_LARGE`).
Comment thread XLS-0056-batch/README.md
Comment on lines +181 to +182
5. The `BatchSigners` field contains a signature from an account that does not have any inner transactions (`temBAD_SIGNER`).
6. The `BatchSigners` field is missing a signature from an account that has inner transactions (`temBAD_SIGNER`).
Comment thread XLS-0056-batch/README.md Outdated
Comment thread XLS-0056-batch/README.md Outdated
Comment thread XLS-0056-batch/README.md Outdated
Comment thread XLS-0056-batch/README.md
mvadari and others added 4 commits July 22, 2026 13:26
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@dangell7

dangell7 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Checked this against develop, mostly lines up but a few of the failure conditions drifted from the current implementation:

  • 2.1.3 item 7, "BatchSigners is required but is not present (tefBAD_AUTH)": there's no tefBAD_AUTH path for a missing BatchSigners field anymore. If signers are required and the field is absent it falls out of the required-signers match in preflight as temBAD_SIGNER, so this is really the same case as item 6.
  • item 8, "contains an invalid signature (temBAD_SIGNATURE)": batch signer sigs are verified in STTx::checkSign now (via checkValidity), so a bad signature surfaces as SigBad -> temINVALID in preflight2, not temBAD_SIGNATURE. That was true when Batch::preflight verified them directly but not anymore.
  • the cap: develop has kMaxBatchSigners = kMaxBatchTxCount * 3 = 24, not 8, since an inner can require up to 3 signers now. idk if the spec should track that or stay stock XLS-56, but "greater than 8" doesn't match develop either way.

The signing payload section is spot on though, matches serializeBatch exactly. If the intent is to document BatchV1_1 as released then most of this is fine, just flagging where it diverges from 3.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants