Batch Amendment

[dangell7]

High Level Overview of Change

Context of Change

Type of Change

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Refactor (non-breaking change that only restructures code)
• [ ] Performance (increase or change in throughput and/or latency)
• [ ] Tests (you added tests for code that already exists, or your new feature included in this PR)
• [ ] Documentation update
• [ ] Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
• [ ] Release

API Impact

• [ ] Public API: New feature (new methods and/or new fields)
• [ ] Public API: Breaking change (in general, breaking changes should only impact the next api_version)
• [ ] libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
• [ ] Peer protocol change (must be backward compatible or bump the peer protocol version)

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 94.05204% with 32 lines in your changes missing coverage. Please review.

Project coverage is 78.6%. Comparing base (40ce8a8) to head (66d5f27). Report is 13 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/xrpld/app/tx/detail/applySteps.cpp	28.6%	15 Missing :warning:
src/libxrpl/protocol/STTx.cpp	92.0%	8 Missing :warning:
src/xrpld/app/misc/NetworkOPs.cpp	87.0%	3 Missing :warning:
src/xrpld/app/tx/detail/Transactor.cpp	95.7%	3 Missing :warning:
src/xrpld/app/tx/detail/apply.cpp	96.7%	2 Missing :warning:
src/xrpld/ledger/detail/OpenView.cpp	50.0%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5060     +/-   ##
=========================================
+ Coverage     78.5%   78.6%   +0.1%     
=========================================
  Files          813     816      +3     
  Lines        70084   70505    +421     
  Branches      8283    8291      +8     
=========================================
+ Hits         55031   55439    +408     
- Misses       15053   15066     +13     

Files with missing lines	Coverage Δ
include/xrpl/protocol/Batch.h	100.0% <100.0%> (ø)
include/xrpl/protocol/HashPrefix.h	100.0% <ø> (ø)
include/xrpl/protocol/STTx.h	100.0% <ø> (ø)
include/xrpl/protocol/TER.h	100.0% <100.0%> (ø)
include/xrpl/protocol/TxMeta.h	93.8% <100.0%> (+2.1%)	:arrow_up:
include/xrpl/protocol/detail/transactions.macro	100.0% <100.0%> (ø)
src/libxrpl/protocol/InnerObjectFormats.cpp	100.0% <100.0%> (ø)
src/libxrpl/protocol/TER.cpp	100.0% <ø> (ø)
src/libxrpl/protocol/TxMeta.cpp	96.5% <100.0%> (+0.3%)	:arrow_up:
src/xrpld/app/ledger/detail/BuildLedger.cpp	91.9% <100.0%> (+0.5%)	:arrow_up:
... and 24 more

... and 3 files with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ximinez]

Also, since it seems this PR is still under active development, could you convert it to a draft?

[tequdev]

AffectedNodes of the Outer Transaction doesn't includePreviousField.

https://explorer.xrplf.org/wss:batch.nerdnest.xyz/tx/4FACA5DC82834BDD8071C6F6EC57D9FE1E83E189D5BE52D6D4EA7AB3E498E4D5

[tequdev]

Looking at the ModifiedNode, the Sequence of AccountRoot has decreased from 30 to 29.

https://explorer.xrplf.org/wss:batch.nerdnest.xyz/tx/1CE0C3D9BB615B60CBD4287A3AF8187C90F459E5F554BFC1087362343A4B7C4C

[tequdev]

BatchTxn.BatchIndex is not validated, so any number can be specified, resulting in a jump in the Sequence in AccountRoot.

https://explorer.xrplf.org/wss:batch.nerdnest.xyz/tx/386795456D88DD9059780F1B83CC572DF211CC250A33018543FDFBBC69169641

[tequdev]

~~While Outer Transaction’s result is tecINVARIANT_FAILED, Inner Transactions are successfully submitted to network.~~

~~https://explorer.xrplf.org/wss:batch.nerdnest.xyz/tx/1CE0C3D9BB615B60CBD4287A3AF8187C90F459E5F554BFC1087362343A4B7C4C~~

[dangell7]

While Outer Transaction’s result is tecINVARIANT_FAILED, Inner Transactions are successfully submitted to network.

https://explorer.xrplf.org/wss:batch.nerdnest.xyz/tx/1CE0C3D9BB615B60CBD4287A3AF8187C90F459E5F554BFC1087362343A4B7C4C

This is the same issue as above. This will be fixed with a refactor of the stacking views.

[tequdev]

Some of the points I made in X are also posted here for the record.

• TxnIDs field is not used (Must be coordinated with Standard)
• SigningPubKey, Signature of Outer Transaction must be empty string if multi-account Batch.
• SigningPubKey of Inner transaction must be empty string

[ckeshava]

Where can I find information about the behavior of Batch transactions with respect to ter codes? If one of the inner transaction ends up with a ter result, will it be retried in a subsequent ledger?

[dangell7]

Where can I find information about the behavior of Batch transactions with respect to ter codes? If one of the inner transaction ends up with a ter result, will it be retried in a subsequent ledger?

These are the tests for the different batch types, there should be a ter error for each. No, inner batch transactions are not retried.

https://github.com/XRPLF/rippled/blob/a3e1396a5c55da43e850794809142cbdfa7895ac/src/test/app/Batch_test.cpp#L3606

[lmaisons]

From our performance review, the total ledger throughput does not appear to be impaired or hindered when ledger processing includes batch transactions. However, it should also be noted that batch transactions containing the full set of 8 inner transaction can apply significant leverage to ledger throughput. We should be mindful of the potential for increased impact when making on-going performance decisions that involve and / or impact batch transactions.

[mvadari]

@dangell7 is this ready to merge?

[dangell7]

@dangell7 is this ready to merge?

LGTM

- Specification: [XRPLF/XRPL-Standards 56](https://github.com/XRPLF/XRPL-Standards/blob/master/XLS-0056d-batch/README.md)
- Amendment: `Batch`
- Implements execution of multiple transactions within a single
 batch transaction with four execution modes: `tfAllOrNothing`,
 `tfOnlyOne`, `tfUntilFailure`, and `tfIndependent`.
- Enables atomic multi-party transactions where multiple accounts can
 participate in a single batch, with up to 8 inner transactions and
 8 batch signers per batch transaction.
- Inner transactions use `tfInnerBatchTxn` flag with zero fees, no
 signature, and empty signing public key.
- Inner transactions are applied after the outer batch succeeds via
 `applyBatchTransactions` function in apply.cpp.
- Network layer prevents relay of transactions with `tfInnerBatchTxn`
 flag - each peer applies inner transactions locally from the batch.
- Batch transactions are excluded from AccountDelegate permissions but
 inner transactions retain full delegation support.
- Metadata includes `ParentBatchID` linking inner transactions to their
 containing batch for traceability and auditing.
- Extended STTx with batch-specific signature verification methods
 and added protocol structures (`sfRawTransactions`, `sfBatchSigners`).

Feel free to edit this any way you see fit.

[bthomee]

@dangell7 can you please update your branch?

[bthomee]

@XRPLF/rpc-reviewers can you please check and approve the RPC-related changes?