[TA-4885]: Add MPTCurrency Amount#140
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 📝 Walkthrough""" WalkthroughSupport for a new MPT (Multi-Party Token) currency amount type is introduced across the codebase. This includes serialization, deserialization, validation, and JSON handling in the binary codec, as well as new types and unmarshaling logic in the transaction layer. Comprehensive unit tests and changelog updates accompany these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Codec as BinaryCodec
participant XRPL as TransactionLayer
Client->>Codec: FromJSON({value, mpt_issuance_id})
Codec->>Codec: serializeMPTCurrencyAmount(value, mpt_issuance_id)
Codec-->>Client: []byte (MPT amount)
Client->>Codec: ToJSON([]byte MPT amount)
Codec->>Codec: deserializeMPTAmount([]byte)
Codec-->>Client: {value, mpt_issuance_id}
Client->>XRPL: UnmarshalCurrencyAmount(JSON with mpt_issuance_id)
XRPL->>XRPL: Unmarshal into MPTCurrencyAmount
XRPL-->>Client: MPTCurrencyAmount instance
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CHANGELOG.md (1)
14-17: Minor style improvement: Consider varying sentence beginnings.While the content accurately documents the MPT additions, consider varying the sentence structure to improve readability by replacing some instances of "Adds" with alternatives like "Introduces" or "Implements".
- Adds `MPToken` definitions. + Introduces `MPToken` definitions. - Adds functions to serialize and deserialize `MPTCurrencyAmount`. + Implements functions to serialize and deserialize `MPTCurrencyAmount`. - Adds unit tests for `MPTCurrencyAmount`. + Provides unit tests for `MPTCurrencyAmount`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)binary-codec/types/amount.go(8 hunks)binary-codec/types/amount_test.go(4 hunks)xrpl/transaction/types/currency_amount.go(3 hunks)xrpl/transaction/types/currency_amount_test.go(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[style] ~16-~16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...definitions. - AddsHash192` type. - Adds functions to serialize and deserialize ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~17-~17: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... and deserialize MPTCurrencyAmount. - Adds unit tests for MPTCurrencyAmount. ##...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 golangci-lint (1.64.8)
binary-codec/types/amount.go
[high] 558-558: G115: integer overflow conversion uint64 -> uint32
(gosec)
[high] 559-559: G115: integer overflow conversion uint64 -> uint32
(gosec)
🪛 GitHub Actions: XRPL-GO Lint and Test
binary-codec/types/amount.go
[error] 558-559: golangci-lint (gosec): G115 integer overflow conversion uint64 -> uint32 at lines 558 and 559.
🔇 Additional comments (16)
CHANGELOG.md (1)
22-23: Documentation accurately reflects the new MPT features.The changelog entries correctly document the addition of MPTCurrencyAmount support and corresponding unit tests.
xrpl/transaction/types/currency_amount_test.go (3)
57-62: Well-implemented unit test for MPT currency kind.The test correctly verifies that MPTCurrencyAmount.Kind() returns the MPT enum value.
64-128: Comprehensive test coverage for Flatten() method.Excellent test coverage with proper edge cases:
- Empty MPT amount
- Partial data (ID only, value only)
- Complete data (both ID and value)
The assertion logic properly validates both the structure and content of the flattened output.
130-143: Proper JSON unmarshaling test for MPT amounts.The test correctly validates that JSON containing
mpt_issuance_idis properly unmarshaled into an MPTCurrencyAmount instance with correct field values.xrpl/transaction/types/currency_amount.go (3)
13-13: Correct addition of MPT currency kind.The MPT enum value is properly added to the CurrencyKind type, maintaining the established pattern.
27-38: Well-implemented MPT detection in unmarshaling logic.The logic correctly identifies MPT amounts by checking for the presence of
mpt_issuance_idkey in the JSON object and properly unmarshals into the MPTCurrencyAmount struct.
132-150: Properly implemented MPTCurrencyAmount type.The struct definition and interface implementation follow established patterns:
- Clear field names with appropriate JSON tags
- Kind() method returns correct enum value
- Flatten() method conditionally includes non-empty fields
binary-codec/types/amount_test.go (3)
108-156: Comprehensive MPT value validation testing.Excellent test coverage including:
- Valid integer values (including zero and large numbers)
- Invalid cases (decimals, negative numbers, non-numeric, high bit set)
- Proper error type validation
489-819: Thorough serialization and deserialization test coverage.The test suite comprehensively covers:
- Valid value serialization with expected byte outputs
- Issuance ID encoding/decoding with proper length validation
- Complete MPT amount round-trip testing
- Error handling for malformed inputs
All test cases include proper assertions for both success and error scenarios.
895-929: Well-integrated MPT support in Amount JSON tests.The existing Amount JSON marshalling/unmarshalling tests are properly extended to include MPT currency amounts, ensuring full integration with the Amount type's JSON handling.
Also applies to: 1012-1028
binary-codec/types/amount.go (6)
32-38: Well-defined MPT constants and error variables.The constants properly define MPT-specific byte lengths, markers, and masks. The error variables provide clear, specific error messages for MPT validation failures.
Also applies to: 50-52
96-99: Correct MPT detection logic in FromJSON.The logic properly distinguishes between issued currency amounts (with "issuer" key) and MPT amounts (with "mpt_issuance_id" key), delegating to the appropriate serialization function.
116-122: Proper MPT detection in ToJSON deserialization.The bit masking logic correctly identifies MPT amounts using the 0x20 bit pattern and properly handles the 33-byte MPT amount structure.
215-270: Well-implemented MPT deserialization functions.The deserialization logic properly:
- Handles sign bits and extracts 64-bit mantissa using big.Int operations
- Validates byte lengths and extracts issuance IDs
- Combines components into proper map structure
329-353: Comprehensive MPT value validation.The validation function properly checks:
- No decimal points (integers only)
- Non-negative values
- High bit not set to avoid conflicts with other amount types
565-574: Solid MPT serialization helper functions.The issuance ID and complete amount serialization functions properly:
- Validate hex decoding and byte lengths
- Combine components with correct marker byte placement
- Handle error propagation appropriately
Also applies to: 578-598
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…d improve naming conventions
2 participants
[TA-4885]: Add MPTCurrency Amount
Changes 🛠️
binary-codec/types
MPTCurrencyAmountxrpl/transaction
MPTCurrencyAmountfor currency kindsChangelog
Summary by CodeRabbit
New Features
Tests
Documentation