[TA-4976][TA-4977] Add Batch transaction type#146
Merged
GuillemGarciaDev merged 21 commits intov0.1.xfrom Jul 9, 2025
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes introduce a new Batch transaction type to the XRPL Go SDK, including its definition, validation logic, and flag management. Supporting features include a new transaction flag ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BatchTx
participant InnerTx
participant Validator
Client->>BatchTx: Create Batch transaction with RawTransactions
BatchTx->>InnerTx: Wrap each transaction with TfInnerBatchTxn flag
BatchTx->>Validator: Validate Batch (check flags, structure, signers)
Validator-->>BatchTx: Validation result (success/failure)
BatchTx->>Client: Flatten/serialize Batch for submission
Suggested labels
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
xrpl/transaction/validations_commons.go (1)
20-30: Simplify the redundant condition in isRecord function.The function logic is correct but contains redundant checks. Since
val.Kind()can only be one type at a time, the additional checks for slice and array are unnecessary.- // Check if it's an object (map or struct) but not an array/slice - return (val.Kind() == reflect.Map || val.Kind() == reflect.Struct) && - val.Kind() != reflect.Slice && val.Kind() != reflect.Array + // Check if it's an object (map or struct) + return val.Kind() == reflect.Map || val.Kind() == reflect.StructCHANGELOG.md (1)
27-30: Documentation is comprehensive and accurate.The changelog properly documents all the new batch transaction features. The repetitive "Adds" beginnings flagged by static analysis are common in changelogs and don't impact clarity.
If you want to address the style hint, you could vary the sentence beginnings:
- Adds `Batch` transaction type. - Adds `TfInnerBatchTxn` in `flags.go` file - Adds `isArray` and `isRecord` validation helpers. - Adds support for `tfInnerBatchTxn` flag on `SignTxBlob`. + Adds `Batch` transaction type. + Introduces `TfInnerBatchTxn` in `flags.go` file + Implements `isArray` and `isRecord` validation helpers. + Adds support for `tfInnerBatchTxn` flag on `SignTxBlob`.xrpl/transaction/batch_test.go (1)
29-29: Consider using named constants instead of hardcoded flag values.The tests use hardcoded numeric values (e.g., 65536, 1073741824) for flags. Using the named constants would improve readability and maintainability.
For example, replace:
-"Flags": 65536, +"Flags": tfAllOrNothing,And:
-"Flags": 1073741824, +"Flags": uint32(TfInnerBatchTxn),Also applies to: 37-37, 54-54, 77-77, 93-93, 116-116, 132-132, 159-159, 185-185, 235-235, 258-258, 267-267, 301-301, 323-323, 345-345, 367-367, 389-389
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CHANGELOG.md(1 hunks)binary-codec/definitions/definitions.json(2 hunks)xrpl/hash/tx.go(2 hunks)xrpl/transaction/batch.go(1 hunks)xrpl/transaction/batch_test.go(1 hunks)xrpl/transaction/flags.go(1 hunks)xrpl/transaction/transaction_metadata.go(1 hunks)xrpl/transaction/tx_type.go(1 hunks)xrpl/transaction/validations_commons.go(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: banasa44
PR: Peersyst/xrpl-go#145
File: binary-codec/codec.go:0-0
Timestamp: 2025-07-07T11:40:34.708Z
Learning: XRPL batch transactions have a maximum limit of 8 transactions per batch, as defined by the XRPL protocol. This constraint eliminates practical concerns about integer overflow when converting batch length to uint32.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: vriveraPeersyst
PR: Peersyst/xrpl-snap#129
File: packages/site/src/domain/transaction/controller/TransactionController.ts:98-111
Timestamp: 2025-05-17T14:29:55.132Z
Learning: In the TransactionController class of the XRPL Snap project, the `...rest` parameter in the `sendIOUTransaction` method does not contain flags from callers, so directly setting `flags: PARTIAL_PAYMENT_FLAG` without merging with existing flags is appropriate.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/flags.go (4)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: vriveraPeersyst
PR: Peersyst/xrpl-snap#129
File: packages/site/src/domain/transaction/controller/TransactionController.ts:98-111
Timestamp: 2025-05-17T14:29:55.132Z
Learning: In the TransactionController class of the XRPL Snap project, the `...rest` parameter in the `sendIOUTransaction` method does not contain flags from callers, so directly setting `flags: PARTIAL_PAYMENT_FLAG` without merging with existing flags is appropriate.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/tx_type.go (3)
Learnt from: banasa44
PR: Peersyst/xrpl-go#145
File: binary-codec/codec.go:0-0
Timestamp: 2025-07-07T11:40:34.708Z
Learning: XRPL batch transactions have a maximum limit of 8 transactions per batch, as defined by the XRPL protocol. This constraint eliminates practical concerns about integer overflow when converting batch length to uint32.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/transaction_metadata.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/validations_commons.go (3)
Learnt from: GuillemGarciaDev
PR: Peersyst/xrpl-go#131
File: xrpl/rpc/client.go:265-265
Timestamp: 2025-03-31T17:29:49.615Z
Learning: In the XRPL Go codebase, type assertions from maps should always use the two-return pattern (value, ok := map[key].(type)) with proper error handling to avoid potential panics.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
xrpl/hash/tx.go (5)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: vriveraPeersyst
PR: Peersyst/xrpl-snap#129
File: packages/site/src/domain/transaction/controller/TransactionController.ts:98-111
Timestamp: 2025-05-17T14:29:55.132Z
Learning: In the TransactionController class of the XRPL Snap project, the `...rest` parameter in the `sendIOUTransaction` method does not contain flags from callers, so directly setting `flags: PARTIAL_PAYMENT_FLAG` without merging with existing flags is appropriate.
Learnt from: AgustinMJ
PR: Peersyst/near-mobile#58
File: packages/shared/blockchain/core/src/signers/signer.ts:26-27
Timestamp: 2025-02-27T10:24:14.276Z
Learning: The Signer interface uses `Record<string, any>` for the transaction parameter in `signTransaction` method to allow implementing classes to add their own specific types extending this base type, providing flexibility for different blockchain implementations.
CHANGELOG.md (5)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: banasa44
PR: Peersyst/xrpl-go#145
File: binary-codec/codec.go:0-0
Timestamp: 2025-07-07T11:40:34.708Z
Learning: XRPL batch transactions have a maximum limit of 8 transactions per batch, as defined by the XRPL protocol. This constraint eliminates practical concerns about integer overflow when converting batch length to uint32.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: vriveraPeersyst
PR: Peersyst/xrpl-snap#129
File: packages/site/src/domain/transaction/controller/TransactionController.ts:98-111
Timestamp: 2025-05-17T14:29:55.132Z
Learning: In the TransactionController class of the XRPL Snap project, the `...rest` parameter in the `sendIOUTransaction` method does not contain flags from callers, so directly setting `flags: PARTIAL_PAYMENT_FLAG` without merging with existing flags is appropriate.
binary-codec/definitions/definitions.json (1)
Learnt from: banasa44
PR: Peersyst/xrpl-go#145
File: binary-codec/codec.go:0-0
Timestamp: 2025-07-07T11:40:34.708Z
Learning: XRPL batch transactions have a maximum limit of 8 transactions per batch, as defined by the XRPL protocol. This constraint eliminates practical concerns about integer overflow when converting batch length to uint32.
xrpl/transaction/batch_test.go (4)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: banasa44
PR: Peersyst/xrpl-go#145
File: binary-codec/codec.go:0-0
Timestamp: 2025-07-07T11:40:34.708Z
Learning: XRPL batch transactions have a maximum limit of 8 transactions per batch, as defined by the XRPL protocol. This constraint eliminates practical concerns about integer overflow when converting batch length to uint32.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/batch.go (4)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: banasa44
PR: Peersyst/xrpl-go#145
File: binary-codec/codec.go:0-0
Timestamp: 2025-07-07T11:40:34.708Z
Learning: XRPL batch transactions have a maximum limit of 8 transactions per batch, as defined by the XRPL protocol. This constraint eliminates practical concerns about integer overflow when converting batch length to uint32.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
🧬 Code Graph Analysis (2)
xrpl/hash/tx.go (1)
xrpl/transaction/flags.go (1)
TfInnerBatchTxn(8-8)
xrpl/transaction/batch_test.go (6)
xrpl/transaction/batch.go (5)
Batch(87-93)Batch(121-123)RawTransactionWrapper(55-57)BatchSigner(42-44)BatchSignerData(47-52)xrpl/transaction/tx_type.go (2)
BatchTx(16-16)TxType(3-3)xrpl/transaction/tx.go (1)
BaseTx(30-99)xrpl/transaction/flags.go (1)
TfInnerBatchTxn(8-8)xrpl/testutil/flatten.go (1)
CompareFlattenAndExpected(9-32)xrpl/transaction/signer.go (2)
Signer(7-9)SignerData(17-21)
🪛 LanguageTool
CHANGELOG.md
[style] ~27-~27: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...dds NFTokenModify transaction type. - Adds Batch transaction type. - Adds `TfInn...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~28-~28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ype. - Adds Batch transaction type. - Adds TfInnerBatchTxn in flags.go file - ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (9)
xrpl/transaction/transaction_metadata.go (1)
21-21: Field addition looks good.The
ParentBatchIDfield follows the existing struct patterns with appropriate type (string) and JSON tagging (omitempty). This supports the batch transaction functionality correctly.xrpl/transaction/flags.go (1)
5-9: Flag constant addition is well-implemented.The
TfInnerBatchTxnflag constant is properly defined with a clear comment explaining its purpose. The value0x40000000is appropriate for a flag constant, and the section organization with a descriptive header comment enhances code readability.xrpl/transaction/tx_type.go (1)
16-16: Transaction type constant addition is correct.The
BatchTxconstant follows the established pattern for transaction types, with appropriate naming and correct alphabetical placement in the constants list.xrpl/transaction/validations_commons.go (1)
10-18: isArray function implementation is correct.The function properly handles nil input and uses reflection correctly to check for both slice and array types. The logic and error handling are sound.
xrpl/hash/tx.go (1)
11-16: LGTM! Correct handling of inner batch transactions.The implementation properly skips signature validation for inner batch transactions marked with the
TfInnerBatchTxnflag, which is appropriate since these transactions are signed as part of the parent batch transaction.Also applies to: 27-43
binary-codec/definitions/definitions.json (1)
1457-1466: LGTM! Proper integration of batch transaction definitions.The additions correctly follow the existing patterns:
ParentBatchIDfield is properly configured as a Hash256 type with appropriate serialization settingsBatchtransaction type is assigned the next available ID (64)Also applies to: 3062-3062
xrpl/transaction/batch_test.go (1)
1-546: Comprehensive test coverage!The test suite thoroughly covers all aspects of the Batch transaction implementation, including positive and negative test cases for validation, flag handling, and serialization.
xrpl/transaction/batch.go (2)
87-93: Well-structured Batch transaction implementation!The Batch struct and supporting types are well-designed, with clear documentation and appropriate field tagging for JSON serialization.
129-159: Clear and well-documented flag methods!The flag setting methods are properly implemented with clear documentation explaining the behavior of each flag.
…tions and add comprehensive test cases
banasa44
commented
Jul 8, 2025
GuillemGarciaDev
requested changes
Jul 8, 2025
GuillemGarciaDev
requested changes
Jul 8, 2025
GuillemGarciaDev
force-pushed
the
xrpl/feat/add-batch-tx-type
branch
from
926b313 to
c7e555b
Compare
…n validation functions
…and IsTransactionObject for improved type validation
… type validation for RawTransaction
… type validation for RawTransaction
GuillemGarciaDev
changed the title
…syst/xrpl-go into xrpl/feat/add-batch-tx-type
GuillemGarciaDev
approved these changes
Jul 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[TA-4976] Add Batch transaction type
Description
This PR aims to add the transaction type
Batch.Type of change
Checklist:
Changes
Add
Batchtransaction type and tests.Add
isArrayandisRecordvalidation helpers.Add
ParentBatchIDinTxObjMeta.Add support
tfInnerBatchTxnflag onHashSignedTx.Notes
Although the support for
tfInnerBatchTxnflag onHashSignedTxwas intended for a separate task, as it usesTfInnerBatchTxnintroduced in this branch, i added the implementation.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation