feat: Multi-Purpose Tokens (MPT) #732
Conversation
khancode
changed the title
khancode
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
tests/integration/transactions/test_clawback.py (1)
126-128: Document the reason for using max int64 valueThe initial token amount (9223372036854775807) appears to be the maximum value for a 64-bit integer. Consider adding a comment explaining why this specific value was chosen for the test.
amount=MPTAmount( - mpt_issuance_id=mpt_issuance_id, value="9223372036854775807" + mpt_issuance_id=mpt_issuance_id, + value="9223372036854775807" # Max int64 value to test upper bounds ),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
tests/integration/transactions/test_clawback.py(3 hunks)
🔇 Additional comments (5)
tests/integration/transactions/test_clawback.py (5)
4-4: LGTM! Import statements are properly organized.
The new imports for MPT-related functionality are correctly added and well-organized.
Also applies to: 14-14, 19-24
Line range hint 33-82: Skipping review of unchanged test method.
158-159:
Correct the assertion to check the clawback transaction result
The test is asserting the engine_result of auth_res (authorization response) instead of response (clawback response).
self.assertTrue(response.is_successful())
-self.assertEqual(auth_res.result["engine_result"], "tesSUCCESS")
+self.assertEqual(response.result["engine_result"], "tesSUCCESS")168-170:
Update balance assertion after clawback
The test asserts an incorrect balance after clawback. The final balance should be reduced by 500 tokens.
self.assertEqual(
- ledger_entry_res.result["node"]["MPTAmount"], "9223372036854775307"
+ ledger_entry_res.result["node"]["MPTAmount"], "9223372036854774807" # Initial amount (9223372036854775807) - clawback amount (500)
)144-156: Verify holder's address in clawback response
Consider adding validation to ensure the holder's address is correctly reflected in the clawback transaction response.
|
Same as the comment from the js library, am assuming the getBalanceChanges utility here would need to be modified in the same way (or have a comment and raise an issue to track the addition later on) |
xrpl-py doesn't support ledger objects yet so that's why they're excluded for now. |
Adds Multi-Purpose Tokens (MPT) feature.
3 participants
High Level Overview of Change
Adds Multi-Purpose Tokens (MPT) feature.
Context of Change
XLS-33 spec
Type of Change
Did you update CHANGELOG.md?
Test Plan
Adds unit/integ tests for new transactions.