From 5a3ba97df50fc1b19a0ab34aaf4d3e88e364a2f0 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 16:03:40 -0400 Subject: [PATCH 01/14] improve Request.from_dict --- tests/unit/models/requests/test_requests.py | 48 ++++++++++++++++++--- xrpl/models/requests/request.py | 18 +++----- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/tests/unit/models/requests/test_requests.py b/tests/unit/models/requests/test_requests.py index 97a977c9a..709cd7efa 100644 --- a/tests/unit/models/requests/test_requests.py +++ b/tests/unit/models/requests/test_requests.py @@ -1,15 +1,53 @@ from unittest import TestCase -from xrpl.models.requests import Fee, GenericRequest +from xrpl.models.requests import Fee, GenericRequest, Request class TestRequest(TestCase): def test_to_dict_includes_method_as_string(self): - tx = Fee() - value = tx.to_dict()["method"] + req = Fee() + value = req.to_dict()["method"] self.assertEqual(type(value), str) def test_generic_request_to_dict_sets_command_as_method(self): command = "validator_list_sites" - tx = GenericRequest(command=command).to_dict() - self.assertDictEqual(tx, {"method": command}) + req = GenericRequest(command=command).to_dict() + self.assertDictEqual(req, {"method": command}) + + def test_from_dict(self): + req = {"method": "account_tx", "account": "rN6zcSynkRnf8zcgTVrRL8K7r4ovE7J4Zj"} + obj = Request.from_dict(req) + self.assertEqual(obj.__class__.__name__, "AccountTx") + expected = {**req, "binary": False, "forward": False} + self.assertDictEqual(obj.to_dict(), expected) + + def test_from_dict_noripple_check(self): + req = { + "method": "noripple_check", + "account": "rN6zcSynkRnf8zcgTVrRL8K7r4ovE7J4Zj", + "role": "user", + } + obj = Request.from_dict(req) + self.assertEqual(obj.__class__.__name__, "NoRippleCheck") + expected = {**req, "transactions": False, "limit": 300} + self.assertDictEqual(obj.to_dict(), expected) + + def test_from_dict_account_nfts(self): + req = { + "method": "account_nfts", + "account": "rN6zcSynkRnf8zcgTVrRL8K7r4ovE7J4Zj", + } + obj = Request.from_dict(req) + self.assertEqual(obj.__class__.__name__, "AccountNFTs") + expected = {**req} + self.assertDictEqual(obj.to_dict(), expected) + + def test_from_dict_amm_info(self): + req = { + "method": "amm_info", + "amm_account": "rN6zcSynkRnf8zcgTVrRL8K7r4ovE7J4Zj", + } + obj = Request.from_dict(req) + self.assertEqual(obj.__class__.__name__, "AMMInfo") + expected = {**req} + self.assertDictEqual(obj.to_dict(), expected) diff --git a/xrpl/models/requests/request.py b/xrpl/models/requests/request.py index e286b417e..007da929d 100644 --- a/xrpl/models/requests/request.py +++ b/xrpl/models/requests/request.py @@ -162,19 +162,11 @@ def get_method(cls: Type[Self], method: str) -> Type[Request]: # special case for NoRippleCheck and NFT methods if method == RequestMethod.NO_RIPPLE_CHECK: return xrpl.models.requests.NoRippleCheck - if method == RequestMethod.ACCOUNT_NFTS: - return xrpl.models.requests.AccountNFTs - if method == RequestMethod.NFT_BUY_OFFERS: - return xrpl.models.requests.NFTBuyOffers - if method == RequestMethod.NFT_SELL_OFFERS: - return xrpl.models.requests.NFTSellOffers - if method == RequestMethod.NFT_INFO: - return xrpl.models.requests.NFTInfo - if method == RequestMethod.NFT_HISTORY: - return xrpl.models.requests.NFTHistory - if method == RequestMethod.NFTS_BY_ISSUER: - return xrpl.models.requests.NFTsByIssuer - parsed_name = "".join([word.capitalize() for word in method.split("_")]) + if method == RequestMethod.AMM_INFO: + return xrpl.models.requests.AMMInfo + parsed_name = "".join( + [word.capitalize().replace("Nft", "NFT") for word in method.split("_")] + ) if parsed_name in xrpl.models.requests.__all__: return cast(Type[Request], getattr(xrpl.models.requests, parsed_name)) return xrpl.models.requests.GenericRequest From b16e84216c269955f3896bea114681c21e26215b Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 16:04:21 -0400 Subject: [PATCH 02/14] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90c4d8e4e..efe057a5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Allow empty strings for the purpose of removing fields in DIDSet transaction +- Added support for `amm_info` to `Request.from_dict` ### Removed - Remove deprecated `full`, `accounts`, and `type` parameters from ledger request model From c160a6e4d77c58251fe0b8f097eee70b2a278546 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 16:11:43 -0400 Subject: [PATCH 03/14] better erroring for amm_info --- CHANGELOG.md | 1 + tests/unit/models/requests/test_amm_info.py | 28 +++++++++++++++++++++ xrpl/models/requests/amm_info.py | 13 +++++++++- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efe057a5b..90bc0c528 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Allow empty strings for the purpose of removing fields in DIDSet transaction - Added support for `amm_info` to `Request.from_dict` +- Improved erroring for `amm_info` ### Removed - Remove deprecated `full`, `accounts`, and `type` parameters from ledger request model diff --git a/tests/unit/models/requests/test_amm_info.py b/tests/unit/models/requests/test_amm_info.py index a4d6a9e52..a2b73c3b4 100644 --- a/tests/unit/models/requests/test_amm_info.py +++ b/tests/unit/models/requests/test_amm_info.py @@ -1,10 +1,12 @@ from unittest import TestCase from xrpl.models.currencies import XRP, IssuedCurrency +from xrpl.models.exceptions import XRPLModelException from xrpl.models.requests import AMMInfo _ASSET = XRP() _ASSET_2 = IssuedCurrency(currency="USD", issuer="rN6zcSynkRnf8zcgTVrRL8K7r4ovE7J4Zj") +_ACCOUNT = _ASSET_2.issuer class TestAMMInfo(TestCase): @@ -14,3 +16,29 @@ def test_asset_asset2(self): asset2=_ASSET_2, ) self.assertTrue(request.is_valid()) + + def test_amount(self): + request = AMMInfo( + amm_account=_ACCOUNT, + ) + self.assertTrue(request.is_valid()) + + def test_all_three(self): + with self.assertRaises(XRPLModelException): + AMMInfo( + amm_account=_ACCOUNT, + asset=_ASSET, + asset2=_ASSET_2, + ) + + def test_only_asset(self): + with self.assertRaises(XRPLModelException): + AMMInfo( + asset=_ASSET, + ) + + def test_only_one_asset2(self): + with self.assertRaises(XRPLModelException): + AMMInfo( + asset2=_ASSET_2, + ) diff --git a/xrpl/models/requests/amm_info.py b/xrpl/models/requests/amm_info.py index 9c2e8a235..1dafa5fd8 100644 --- a/xrpl/models/requests/amm_info.py +++ b/xrpl/models/requests/amm_info.py @@ -1,8 +1,11 @@ """This request gets information about an Automated Market Maker (AMM) instance.""" + from __future__ import annotations from dataclasses import dataclass, field -from typing import Optional +from typing import Dict, Optional + +from typing_extensions import Self from xrpl.models.currencies import Currency from xrpl.models.requests.request import Request, RequestMethod @@ -33,3 +36,11 @@ class AMMInfo(Request): """ method: RequestMethod = field(default=RequestMethod.AMM_INFO, init=False) + + def _get_errors(self: Self) -> Dict[str, str]: + errors = super()._get_errors() + if (self.asset is None) != (self.asset2 is None): + errors["assets"] = "Must have both `asset` and `asset2` fields." + if (self.asset is None) == (self.amm_account is None): + errors["params"] = "Must not have both asset and `amm_account` fields." + return errors From de91678bc60917276ef5234a5cd3df4307c08f06 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 16:24:38 -0400 Subject: [PATCH 04/14] improve generalization --- xrpl/models/base_model.py | 2 ++ xrpl/models/requests/request.py | 15 +++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index c4dc9a767..44532d258 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -44,6 +44,8 @@ "unl": "UNL", "uri": "URI", "xchain": "XChain", + "nfts": "NFTs", + "noripple": "NoRipple", } diff --git a/xrpl/models/requests/request.py b/xrpl/models/requests/request.py index 007da929d..caaa6c03f 100644 --- a/xrpl/models/requests/request.py +++ b/xrpl/models/requests/request.py @@ -12,7 +12,7 @@ from typing_extensions import Self import xrpl.models.requests # bare import to get around circular dependency -from xrpl.models.base_model import BaseModel +from xrpl.models.base_model import ABBREVIATIONS, BaseModel from xrpl.models.exceptions import XRPLModelException from xrpl.models.required import REQUIRED from xrpl.models.utils import KW_ONLY_DATACLASS, require_kwargs_on_init @@ -160,12 +160,15 @@ def get_method(cls: Type[Self], method: str) -> Type[Request]: it will return a `GenericRequest`. """ # special case for NoRippleCheck and NFT methods - if method == RequestMethod.NO_RIPPLE_CHECK: - return xrpl.models.requests.NoRippleCheck - if method == RequestMethod.AMM_INFO: - return xrpl.models.requests.AMMInfo + # if method == RequestMethod.NO_RIPPLE_CHECK: + # return xrpl.models.requests.NoRippleCheck + # if method == RequestMethod.AMM_INFO: + # return xrpl.models.requests.AMMInfo parsed_name = "".join( - [word.capitalize().replace("Nft", "NFT") for word in method.split("_")] + [ + ABBREVIATIONS[word] if word in ABBREVIATIONS else word.capitalize() + for word in method.split("_") + ] ) if parsed_name in xrpl.models.requests.__all__: return cast(Type[Request], getattr(xrpl.models.requests, parsed_name)) From 06738612169acadc253517b2933a196bffef5382 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 16:33:23 -0400 Subject: [PATCH 05/14] complete codecov --- tests/unit/models/requests/test_requests.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unit/models/requests/test_requests.py b/tests/unit/models/requests/test_requests.py index 709cd7efa..d1be915cc 100644 --- a/tests/unit/models/requests/test_requests.py +++ b/tests/unit/models/requests/test_requests.py @@ -1,5 +1,6 @@ from unittest import TestCase +from xrpl.models.exceptions import XRPLModelException from xrpl.models.requests import Fee, GenericRequest, Request @@ -21,6 +22,16 @@ def test_from_dict(self): expected = {**req, "binary": False, "forward": False} self.assertDictEqual(obj.to_dict(), expected) + def test_from_dict_no_method(self): + req = {"account": "rN6zcSynkRnf8zcgTVrRL8K7r4ovE7J4Zj"} + with self.assertRaises(XRPLModelException): + Request.from_dict(req) + + def test_from_dict_wrong_method(self): + req = {"method": "account_tx"} + with self.assertRaises(XRPLModelException): + Fee.from_dict(req) + def test_from_dict_noripple_check(self): req = { "method": "noripple_check", @@ -51,3 +62,13 @@ def test_from_dict_amm_info(self): self.assertEqual(obj.__class__.__name__, "AMMInfo") expected = {**req} self.assertDictEqual(obj.to_dict(), expected) + + def test_from_dict_generic_request(self): + req = { + "method": "tx_history", + "start": 0, + } + obj = Request.from_dict(req) + self.assertEqual(obj.__class__.__name__, "GenericRequest") + expected = {**req} + self.assertDictEqual(obj.to_dict(), expected) From 0eb624119cd4053a84b3fde4d96639245f6ad94a Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 16:34:11 -0400 Subject: [PATCH 06/14] add TODO --- xrpl/models/requests/request.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xrpl/models/requests/request.py b/xrpl/models/requests/request.py index caaa6c03f..eff8899b3 100644 --- a/xrpl/models/requests/request.py +++ b/xrpl/models/requests/request.py @@ -122,6 +122,7 @@ def from_dict(cls: Type[Self], value: Dict[str, Any]) -> Self: Raises: XRPLModelException: If the dictionary provided is invalid. """ + # TODO: add support for "command" parameter and proper JSON RPC format if cls.__name__ == "Request": if "method" not in value: raise XRPLModelException("Request does not include method.") From 31604976fd4a2733fde442b5f74e728088f7efcc Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 16:59:01 -0400 Subject: [PATCH 07/14] remove commented out code --- xrpl/models/requests/request.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/xrpl/models/requests/request.py b/xrpl/models/requests/request.py index eff8899b3..a850334bc 100644 --- a/xrpl/models/requests/request.py +++ b/xrpl/models/requests/request.py @@ -160,11 +160,6 @@ def get_method(cls: Type[Self], method: str) -> Type[Request]: The request class with the given name. If the request doesn't exist, then it will return a `GenericRequest`. """ - # special case for NoRippleCheck and NFT methods - # if method == RequestMethod.NO_RIPPLE_CHECK: - # return xrpl.models.requests.NoRippleCheck - # if method == RequestMethod.AMM_INFO: - # return xrpl.models.requests.AMMInfo parsed_name = "".join( [ ABBREVIATIONS[word] if word in ABBREVIATIONS else word.capitalize() From 2228ee1661fe65d13886176abb5bed014eb7819e Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 16:59:48 -0400 Subject: [PATCH 08/14] clean up --- tests/unit/models/requests/test_requests.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/unit/models/requests/test_requests.py b/tests/unit/models/requests/test_requests.py index d1be915cc..be9ca25f0 100644 --- a/tests/unit/models/requests/test_requests.py +++ b/tests/unit/models/requests/test_requests.py @@ -50,8 +50,7 @@ def test_from_dict_account_nfts(self): } obj = Request.from_dict(req) self.assertEqual(obj.__class__.__name__, "AccountNFTs") - expected = {**req} - self.assertDictEqual(obj.to_dict(), expected) + self.assertDictEqual(obj.to_dict(), req) def test_from_dict_amm_info(self): req = { @@ -60,8 +59,7 @@ def test_from_dict_amm_info(self): } obj = Request.from_dict(req) self.assertEqual(obj.__class__.__name__, "AMMInfo") - expected = {**req} - self.assertDictEqual(obj.to_dict(), expected) + self.assertDictEqual(obj.to_dict(), req) def test_from_dict_generic_request(self): req = { @@ -70,5 +68,4 @@ def test_from_dict_generic_request(self): } obj = Request.from_dict(req) self.assertEqual(obj.__class__.__name__, "GenericRequest") - expected = {**req} - self.assertDictEqual(obj.to_dict(), expected) + self.assertDictEqual(obj.to_dict(), req) From 14e76f697ff779c03746491a621547690ef2ff95 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 17:09:56 -0400 Subject: [PATCH 09/14] improve comment --- xrpl/models/base_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index 44532d258..aa41789b7 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -34,7 +34,7 @@ f"(?:{_CAMEL_CASE_LEADING_LOWER}|{_CAMEL_CASE_ABBREVIATION}|{_CAMEL_CASE_TYPICAL})" ) # This is used to make exceptions when converting dictionary keys to xrpl JSON -# keys. We snake case keys, but some keys are abbreviations. +# keys. xrpl-py uses snake case keys, but some keys are abbreviations. ABBREVIATIONS: Final[Dict[str, str]] = { "amm": "AMM", "did": "DID", From a3d68d42292094b7ceb0f189eb18969c914be7f5 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 12 Jul 2024 11:43:08 -0400 Subject: [PATCH 10/14] fix(?) flakiness in snippet --- snippets/bridge_transfer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/snippets/bridge_transfer.py b/snippets/bridge_transfer.py index 7dc00a828..f9be9b888 100644 --- a/snippets/bridge_transfer.py +++ b/snippets/bridge_transfer.py @@ -55,6 +55,7 @@ f"Destination account {wallet2.classic_address} has been created via the " "bridge" ) + sleep(0.5) # to avoid flakiness initial_balance = get_balance(wallet2.classic_address, issuing_client) break From c8dc8313862fb130d81a040e8499f21cf68600a4 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 15 Jul 2024 17:14:46 -0700 Subject: [PATCH 11/14] improve comment --- xrpl/models/requests/request.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xrpl/models/requests/request.py b/xrpl/models/requests/request.py index a850334bc..3d1b1cf78 100644 --- a/xrpl/models/requests/request.py +++ b/xrpl/models/requests/request.py @@ -123,6 +123,7 @@ def from_dict(cls: Type[Self], value: Dict[str, Any]) -> Self: XRPLModelException: If the dictionary provided is invalid. """ # TODO: add support for "command" parameter and proper JSON RPC format + # This is already done in `GenericRequest`, for reference if cls.__name__ == "Request": if "method" not in value: raise XRPLModelException("Request does not include method.") From 398255336c9c4823ee20d01b97d3cbfc946786f2 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 16 Jul 2024 14:44:56 -0700 Subject: [PATCH 12/14] fix nft_history --- tests/unit/models/requests/test_requests.py | 10 ++++++++++ xrpl/models/base_model.py | 1 + 2 files changed, 11 insertions(+) diff --git a/tests/unit/models/requests/test_requests.py b/tests/unit/models/requests/test_requests.py index be9ca25f0..ceab6bde7 100644 --- a/tests/unit/models/requests/test_requests.py +++ b/tests/unit/models/requests/test_requests.py @@ -61,6 +61,16 @@ def test_from_dict_amm_info(self): self.assertEqual(obj.__class__.__name__, "AMMInfo") self.assertDictEqual(obj.to_dict(), req) + def test_from_dict_nft_history(self): + req = { + "method": "nft_history", + "nft_id": "00000000", + } + obj = Request.from_dict(req) + expected = {**req, "binary": False, "forward": False} + self.assertEqual(obj.__class__.__name__, "NFTHistory") + self.assertDictEqual(obj.to_dict(), expected) + def test_from_dict_generic_request(self): req = { "method": "tx_history", diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index aa41789b7..1851325da 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -46,6 +46,7 @@ "xchain": "XChain", "nfts": "NFTs", "noripple": "NoRipple", + "nft": "NFT", } From e309bbbd8afa045fa63eefca5a2684a7e28a96d0 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 17 Jul 2024 21:31:12 -0700 Subject: [PATCH 13/14] fix error message --- xrpl/models/requests/amm_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xrpl/models/requests/amm_info.py b/xrpl/models/requests/amm_info.py index 1dafa5fd8..d8862c77c 100644 --- a/xrpl/models/requests/amm_info.py +++ b/xrpl/models/requests/amm_info.py @@ -42,5 +42,5 @@ def _get_errors(self: Self) -> Dict[str, str]: if (self.asset is None) != (self.asset2 is None): errors["assets"] = "Must have both `asset` and `asset2` fields." if (self.asset is None) == (self.amm_account is None): - errors["params"] = "Must not have both asset and `amm_account` fields." + errors["params"] = "Must not have both `asset` and `amm_account` fields." return errors From 1946b6567d533fd5fd78eb99f7645525b5a7bd59 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 5 Aug 2024 17:26:04 -0400 Subject: [PATCH 14/14] alphabetize --- xrpl/models/base_model.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index 1851325da..f69293d18 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -40,13 +40,13 @@ "did": "DID", "id": "ID", "lp": "LP", + "nft": "NFT", "nftoken": "NFToken", + "nfts": "NFTs", + "noripple": "NoRipple", "unl": "UNL", "uri": "URI", "xchain": "XChain", - "nfts": "NFTs", - "noripple": "NoRipple", - "nft": "NFT", }