From 4cc67627ae9a5a36c96007c03d4ab642f47999dc Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 7 May 2024 15:54:59 -0400 Subject: [PATCH 01/16] add type validation --- xrpl/models/base_model.py | 87 ++++++++++++++++++++++++++++++++------- 1 file changed, 71 insertions(+), 16 deletions(-) diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index 583a564b2..23e773eaf 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -9,7 +9,7 @@ from enum import Enum from typing import Any, Dict, List, Pattern, Type, TypeVar, Union, cast, get_type_hints -from typing_extensions import Final, Literal, get_args, get_origin +from typing_extensions import Final, Literal, Self, get_args, get_origin from xrpl.models.exceptions import XRPLModelException from xrpl.models.required import REQUIRED @@ -242,11 +242,11 @@ def from_xrpl(cls: Type[BM], value: Union[str, Dict[str, Any]]) -> BM: return cls.from_dict(formatted_dict) - def __post_init__(self: BaseModel) -> None: + def __post_init__(self: Self) -> None: """Called by dataclasses immediately after __init__.""" self.validate() - def validate(self: BaseModel) -> None: + def validate(self: Self) -> None: """ Raises if this object is invalid. @@ -257,7 +257,7 @@ def validate(self: BaseModel) -> None: if len(errors) > 0: raise XRPLModelException(str(errors)) - def is_valid(self: BaseModel) -> bool: + def is_valid(self: Self) -> bool: """ Returns whether this BaseModel is valid. @@ -266,20 +266,75 @@ def is_valid(self: BaseModel) -> bool: """ return len(self._get_errors()) == 0 - def _get_errors(self: BaseModel) -> Dict[str, str]: + def _check_type( + self: Self, attr: str, value: Any, expected_type: Type[Any] + ) -> Dict[str, str]: + # returns error message if type is bad, None if type is good + expected_type_origin = get_origin(expected_type) + if expected_type_origin is Union: + if any( + self._check_type(attr, value, expected_type_option) is None + for expected_type_option in get_args(expected_type) + ): + return {attr: f"{attr} is {type(value)}, expected {expected_type}"} + return {} + + if expected_type is Any: + return {} + + if expected_type_origin is list: + # expected a List, received a List + # TODO: check for + if not isinstance(value, list): + return {"attr": f"{attr} is {type(value)}, expected {expected_type}"} + result = {} + for i in range(len(value)): + result.update( + self._check_type( + f"{attr}[{i}]", value[i], get_args(expected_type)[0] + ) + ) + return result + + if expected_type_origin is dict: + return ( + {} + if isinstance(value, dict) + else {attr: f"{attr} is {type(value)}, expected {expected_type}"} + ) + + if isinstance(expected_type, type) and issubclass(expected_type, Enum): + return ( + {} + if value in list(expected_type) + else { + attr: f"{attr} is {value}, expected member of {expected_type} enum" + } + ) + + if not isinstance(value, expected_type): + return {attr: f"{attr} is {type(value)}, expected {expected_type}"} + + return {} + + def _get_errors(self: Self) -> Dict[str, str]: """ Extended in subclasses to define custom validation logic. Returns: Dictionary of any errors found on self. """ - return { - attr: f"{attr} is not set" - for attr, value in self.__dict__.items() - if value is REQUIRED - } - - def to_dict(self: BaseModel) -> Dict[str, Any]: + class_types = get_type_hints(self.__class__) + result: Dict[str, str] = {} + for attr, value in self.__dict__.items(): + # print(attr, value, class_types[attr]) + if value is REQUIRED: + result[attr] = f"{attr} is not set" + else: + result.update(self._check_type(attr, value, class_types[attr])) + return result + + def to_dict(self: Self) -> Dict[str, Any]: """ Returns the dictionary representation of a BaseModel. @@ -296,7 +351,7 @@ def to_dict(self: BaseModel) -> Dict[str, Any]: if getattr(self, key) is not None } - def _to_dict_elem(self: BaseModel, elem: Any) -> Any: + def _to_dict_elem(self: Self, elem: Any) -> Any: if isinstance(elem, BaseModel): return elem.to_dict() if isinstance(elem, Enum): @@ -309,11 +364,11 @@ def _to_dict_elem(self: BaseModel, elem: Any) -> Any: ] return elem - def __eq__(self: BaseModel, other: object) -> bool: + def __eq__(self: Self, other: object) -> bool: """Compares a BaseModel to another object to determine if they are equal.""" return isinstance(other, BaseModel) and self.to_dict() == other.to_dict() - def __repr__(self: BaseModel) -> str: - """Returns a string representation of a BaseModel object""" + def __repr__(self: Self) -> str: + """Returns a string representation of a BaseModel object.""" repr_items = [f"{key}={repr(value)}" for key, value in self.to_dict().items()] return f"{type(self).__name__}({repr_items})" From 8dcd7ddafed98fa06e34e2368bdd0e12e6bf5389 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 7 May 2024 15:56:10 -0400 Subject: [PATCH 02/16] add tests, fix tests --- tests/unit/models/test_base_model.py | 19 +++++++++++++++++++ .../models/transactions/test_check_cash.py | 2 +- xrpl/models/transactions/transaction.py | 7 +------ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/tests/unit/models/test_base_model.py b/tests/unit/models/test_base_model.py index 7cea1d9c6..31983fd0f 100644 --- a/tests/unit/models/test_base_model.py +++ b/tests/unit/models/test_base_model.py @@ -82,6 +82,25 @@ def test_is_dict_of_model_when_not_true(self): ), ) + def test_bad_type(self): + transaction_dict = { + "account": 1, + "amount": 10, + "destination": 1, + } + with self.assertRaises(XRPLModelException): + Payment(**transaction_dict) + + def test_bad_type_enum(self): + path_find_dict = { + "subcommand": "blah", + "source_account": "raoV5dkC66XvGWjSzUhCUuuGM3YFTitMxT", + "destination_account": "rJjusz1VauNA9XaHxJoiwHe38bmQFz1sUV", + "destination_amount": "100", + } + with self.assertRaises(XRPLModelException): + PathFind(**path_find_dict) + class TestFromDict(TestCase): maxDiff = 2000 diff --git a/tests/unit/models/transactions/test_check_cash.py b/tests/unit/models/transactions/test_check_cash.py index 4fadb8537..aa3fca77d 100644 --- a/tests/unit/models/transactions/test_check_cash.py +++ b/tests/unit/models/transactions/test_check_cash.py @@ -6,7 +6,7 @@ _ACCOUNT = "r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ" _FEE = "0.00001" _SEQUENCE = 19048 -_CHECK_ID = 19048 +_CHECK_ID = "838766BA2B995C00744175F69A1B11E32C3DBC40E64801A4056FCBD657F57334" _AMOUNT = "300" diff --git a/xrpl/models/transactions/transaction.py b/xrpl/models/transactions/transaction.py index a9745e932..6b6a7bc3a 100644 --- a/xrpl/models/transactions/transaction.py +++ b/xrpl/models/transactions/transaction.py @@ -1,4 +1,5 @@ """The base model for all transactions and their nested object types.""" + from __future__ import annotations from dataclasses import dataclass @@ -253,9 +254,6 @@ class Transaction(BaseModel): """The network id of the transaction.""" def _get_errors(self: Transaction) -> Dict[str, str]: - # import must be here to avoid circular dependencies - from xrpl.wallet.main import Wallet - errors = super()._get_errors() if self.ticket_sequence is not None and ( (self.sequence is not None and self.sequence != 0) @@ -266,9 +264,6 @@ def _get_errors(self: Transaction) -> Dict[str, str]: ] = """If ticket_sequence is provided, account_txn_id must be None and sequence must be None or 0""" - if isinstance(self.account, Wallet): - errors["account"] = "Must pass in `wallet.address`, not `wallet`." - return errors def to_dict(self: Transaction) -> Dict[str, Any]: From cb15f44f69030554c3f76fcab0fb11bee581ae70 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 7 May 2024 17:00:00 -0400 Subject: [PATCH 03/16] improve test coverage of BaseModel, fix issues --- tests/unit/models/test_base_model.py | 98 ++++++++++++++++++- .../models/transactions/test_xchain_claim.py | 2 +- xrpl/models/base_model.py | 11 ++- xrpl/models/currencies/xrp.py | 1 + xrpl/models/requests/ledger_entry.py | 5 +- .../pseudo_transactions/enable_amendment.py | 4 +- xrpl/models/transactions/transaction.py | 7 +- 7 files changed, 115 insertions(+), 13 deletions(-) diff --git a/tests/unit/models/test_base_model.py b/tests/unit/models/test_base_model.py index 31983fd0f..8f04b00f8 100644 --- a/tests/unit/models/test_base_model.py +++ b/tests/unit/models/test_base_model.py @@ -27,6 +27,7 @@ SignerListSet, TrustSet, TrustSetFlag, + XChainAddAccountCreateAttestation, XChainClaim, ) from xrpl.models.transactions.transaction import Transaction @@ -91,6 +92,16 @@ def test_bad_type(self): with self.assertRaises(XRPLModelException): Payment(**transaction_dict) + def test_bad_type_flags(self): + transaction_dict = { + "account": 1, + "amount": 10, + "destination": 1, + "flags": "1234", + } + with self.assertRaises(XRPLModelException): + Payment(**transaction_dict) + def test_bad_type_enum(self): path_find_dict = { "subcommand": "blah", @@ -407,6 +418,81 @@ def test_from_dict_submit(self): actual = Request.from_dict(request) self.assertEqual(actual, expected) + def test_from_dict_nonexistent_field(self): + tx = { + "account": "rH6ZiHU1PGamME2LvVTxrgvfjQpppWKGmr", + "bad_field": "random", + "flags": 131072, + "limit_amount": { + "currency": "USD", + "issuer": "raoV5dkC66XvGWjSzUhCUuuGM3YFTitMxT", + "value": "100", + }, + } + with self.assertRaises(XRPLModelException): + TrustSet.from_dict(tx) + + def test_from_dict_bad_literal(self): + tx = { + "account": issuer, + "xchain_bridge": { + "locking_chain_door": issuer, + "locking_chain_issue": {"currency": "XRP"}, + "issuing_chain_door": issuer, + "issuing_chain_issue": {"currency": "XRP"}, + }, + "public_key": "0342E083EA762D91D621714C394", + "signature": "3044022053B26DAAC9C886192C95", + "other_chain_source": issuer, + "amount": amount_dict, + "attestation_reward_account": issuer, + "attestation_signer_account": issuer, + "was_locking_chain_send": 2, + "xchain_account_create_count": 12, + "destination": issuer, + "signature_reward": "200", + } + with self.assertRaises(XRPLModelException): + XChainAddAccountCreateAttestation.from_dict(tx) + + def test_from_dict_good_literal(self): + tx = { + "account": issuer, + "xchain_bridge": { + "locking_chain_door": issuer, + "locking_chain_issue": {"currency": "XRP"}, + "issuing_chain_door": issuer, + "issuing_chain_issue": {"currency": "XRP"}, + }, + "public_key": "0342E083EA762D91D621714C394", + "signature": "3044022053B26DAAC9C886192C95", + "other_chain_source": issuer, + "amount": "100", + "attestation_reward_account": issuer, + "attestation_signer_account": issuer, + "was_locking_chain_send": 1, + "xchain_account_create_count": 12, + "destination": issuer, + "signature_reward": "200", + } + expected_dict = { + **tx, + "xchain_bridge": XChainBridge.from_dict(tx["xchain_bridge"]), + } + expected = XChainAddAccountCreateAttestation( + **expected_dict, + ) + self.assertEqual(XChainAddAccountCreateAttestation.from_dict(tx), expected) + + def test_from_dict_enum(self): + path_find_dict = { + "subcommand": "create", + "source_account": "raoV5dkC66XvGWjSzUhCUuuGM3YFTitMxT", + "destination_account": "rJjusz1VauNA9XaHxJoiwHe38bmQFz1sUV", + "destination_amount": "100", + } + self.assertEqual(PathFind.from_dict(path_find_dict), PathFind(**path_find_dict)) + def test_from_xrpl(self): dirname = os.path.dirname(__file__) full_filename = "x-codec-fixtures.json" @@ -416,14 +502,18 @@ def test_from_xrpl(self): for test in fixtures_json["transactions"]: x_json = test["xjson"] r_json = test["rjson"] - with self.subTest(json=x_json): + with self.subTest(json=x_json, use_json=False): tx = Transaction.from_xrpl(x_json) translated_tx = tx.to_xrpl() self.assertEqual(x_json, translated_tx) - with self.subTest(json=r_json): + with self.subTest(json=r_json, use_json=False): tx = Transaction.from_xrpl(r_json) translated_tx = tx.to_xrpl() self.assertEqual(r_json, translated_tx) + with self.subTest(json=r_json, use_json=True): + tx = Transaction.from_xrpl(json.dumps(r_json)) + translated_tx = tx.to_xrpl() + self.assertEqual(r_json, translated_tx) def test_from_xrpl_signers(self): txn_sig1 = ( @@ -555,7 +645,9 @@ def test_to_xrpl_paths(self): issuer="rweYz56rfmQ98cAdRaeTxQS9wVMGnrdsFp", value="0.0000002831214446", ), - paths=paths_json, + paths=[ + [PathStep(**path_data) for path_data in path] for path in paths_json + ], sequence=290, ) tx_json = p.to_xrpl() diff --git a/tests/unit/models/transactions/test_xchain_claim.py b/tests/unit/models/transactions/test_xchain_claim.py index 82f9e690d..eb3d9efef 100644 --- a/tests/unit/models/transactions/test_xchain_claim.py +++ b/tests/unit/models/transactions/test_xchain_claim.py @@ -68,7 +68,7 @@ def test_successful_claim_destination_tag(self): xchain_bridge=_XRP_BRIDGE, xchain_claim_id=_CLAIM_ID, destination=_DESTINATION, - destination_tag="12345", + destination_tag=12345, amount=_XRP_AMOUNT, ) diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index 23e773eaf..455b30b0e 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -273,11 +273,11 @@ def _check_type( expected_type_origin = get_origin(expected_type) if expected_type_origin is Union: if any( - self._check_type(attr, value, expected_type_option) is None + len(self._check_type(attr, value, expected_type_option)) == 0 for expected_type_option in get_args(expected_type) ): - return {attr: f"{attr} is {type(value)}, expected {expected_type}"} - return {} + return {} + return {attr: f"{attr} is {type(value)}, expected {expected_type}"} if expected_type is Any: return {} @@ -312,6 +312,10 @@ def _check_type( } ) + if expected_type_origin is Literal: + arg = get_args(expected_type) + return {} if value in arg else {attr: f"{attr} is {value}, expected {arg}"} + if not isinstance(value, expected_type): return {attr: f"{attr} is {type(value)}, expected {expected_type}"} @@ -327,7 +331,6 @@ def _get_errors(self: Self) -> Dict[str, str]: class_types = get_type_hints(self.__class__) result: Dict[str, str] = {} for attr, value in self.__dict__.items(): - # print(attr, value, class_types[attr]) if value is REQUIRED: result[attr] = f"{attr} is not set" else: diff --git a/xrpl/models/currencies/xrp.py b/xrpl/models/currencies/xrp.py index 82dd47746..2ab4d3d49 100644 --- a/xrpl/models/currencies/xrp.py +++ b/xrpl/models/currencies/xrp.py @@ -8,6 +8,7 @@ See https://xrpl.org/currency-formats.html#specifying-currency-amounts """ + from __future__ import annotations from dataclasses import dataclass, field diff --git a/xrpl/models/requests/ledger_entry.py b/xrpl/models/requests/ledger_entry.py index 8fe2f8bea..f75afdfaf 100644 --- a/xrpl/models/requests/ledger_entry.py +++ b/xrpl/models/requests/ledger_entry.py @@ -5,6 +5,7 @@ different types of objects you can retrieve. `See ledger entry `_ """ + from __future__ import annotations from dataclasses import dataclass, field @@ -253,9 +254,9 @@ class LedgerEntry(Request, LookupByLedgerRequest): ticket: Optional[Union[str, Ticket]] = None bridge_account: Optional[str] = None bridge: Optional[XChainBridge] = None - xchain_claim_id: Optional[Union[str, XChainClaimID]] = None + xchain_claim_id: Optional[Union[int, str, XChainClaimID]] = None xchain_create_account_claim_id: Optional[ - Union[str, XChainCreateAccountClaimID] + Union[int, str, XChainCreateAccountClaimID] ] = None binary: bool = False diff --git a/xrpl/models/transactions/pseudo_transactions/enable_amendment.py b/xrpl/models/transactions/pseudo_transactions/enable_amendment.py index 178532fcc..9867a5407 100644 --- a/xrpl/models/transactions/pseudo_transactions/enable_amendment.py +++ b/xrpl/models/transactions/pseudo_transactions/enable_amendment.py @@ -2,7 +2,7 @@ from dataclasses import dataclass, field from enum import Enum -from typing import List, Optional, Union +from typing import Dict, List, Optional, Union from xrpl.models.flags import FlagInterface from xrpl.models.required import REQUIRED @@ -101,7 +101,7 @@ class EnableAmendment(PseudoTransaction): init=False, ) - flags: Union[int, List[int]] = 0 + flags: Union[Dict[str, bool], int, List[int]] = 0 """ The Flags value of the EnableAmendment pseudo-transaction indicates the status of the amendment at the time of the ledger including the pseudo-transaction. diff --git a/xrpl/models/transactions/transaction.py b/xrpl/models/transactions/transaction.py index 6b6a7bc3a..b1ea0fc88 100644 --- a/xrpl/models/transactions/transaction.py +++ b/xrpl/models/transactions/transaction.py @@ -368,6 +368,9 @@ def has_flag(self: Transaction, flag: int) -> bool: Returns: Whether the transaction has the given flag value set. + + Raises: + XRPLModelException: if `self.flags` is invalid. """ if isinstance(self.flags, int): return self.flags & flag != 0 @@ -376,8 +379,10 @@ def has_flag(self: Transaction, flag: int) -> bool: tx_type=self.transaction_type, tx_flags=self.flags, ) - else: # is List[int] + elif isinstance(self.flags, list): return flag in self.flags + else: + raise XRPLModelException("self.flags is not an int, dict, or list") def is_signed(self: Transaction) -> bool: """ From aa039d238b1886f17802633104da740fc11b9c52 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 8 May 2024 14:56:07 -0400 Subject: [PATCH 04/16] split out from_xrpl tests from from_dict --- tests/unit/models/test_base_model.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/models/test_base_model.py b/tests/unit/models/test_base_model.py index 8f04b00f8..3bc54c3fa 100644 --- a/tests/unit/models/test_base_model.py +++ b/tests/unit/models/test_base_model.py @@ -493,6 +493,8 @@ def test_from_dict_enum(self): } self.assertEqual(PathFind.from_dict(path_find_dict), PathFind(**path_find_dict)) + +class TestToFromXrpl(TestCase): def test_from_xrpl(self): dirname = os.path.dirname(__file__) full_filename = "x-codec-fixtures.json" From 8ae0e1ce4fdd6797a053e15192d6a50594394756 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 17 May 2024 16:31:22 -0400 Subject: [PATCH 05/16] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34d4a8bb1..4d175093b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Support for the Price Oracles amendment (XLS-47). +- Improved validation for models to also check param types ### Fixed - Added support for `XChainModifyBridge` flag maps (fixing an issue with `NFTokenCreateOffer` flag names) From 310363eabdfb93707e421252ec16c575b3b25286 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 17 May 2024 16:34:00 -0400 Subject: [PATCH 06/16] fix tests --- tests/unit/models/transactions/test_oracle_set.py | 2 +- xrpl/models/transactions/oracle_set.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/models/transactions/test_oracle_set.py b/tests/unit/models/transactions/test_oracle_set.py index d99903311..0dc0554ac 100644 --- a/tests/unit/models/transactions/test_oracle_set.py +++ b/tests/unit/models/transactions/test_oracle_set.py @@ -326,7 +326,7 @@ def test_early_last_update_time_field(self): self.assertEqual( err.exception.args[0], "{'last_update_time': 'LastUpdateTime" - + " must be greater than or equal to Ripple-Epoch 946684800.0 seconds'}", + + " must be greater than or equal to ripple epoch - 946684800 seconds'}", ) # Validity depends on the time of the Last Closed Ledger. This test verifies the diff --git a/xrpl/models/transactions/oracle_set.py b/xrpl/models/transactions/oracle_set.py index 91619ce5a..45a1d61ed 100644 --- a/xrpl/models/transactions/oracle_set.py +++ b/xrpl/models/transactions/oracle_set.py @@ -18,10 +18,10 @@ MAX_ORACLE_SYMBOL_CLASS = 16 # epoch offset must equal 946684800 seconds. It represents the diff between the -# genesis of Unix time and Ripple-Epoch time -EPOCH_OFFSET = ( - datetime.datetime(2000, 1, 1) - datetime.datetime(1970, 1, 1) -).total_seconds() +# genesis of Unix time and ripple epoch time +EPOCH_OFFSET = int( + (datetime.datetime(2000, 1, 1) - datetime.datetime(1970, 1, 1)).total_seconds() +) @require_kwargs_on_init @@ -136,7 +136,7 @@ def _get_errors(self: OracleSet) -> Dict[str, str]: if self.last_update_time < EPOCH_OFFSET: errors["last_update_time"] = ( "LastUpdateTime must be greater than or equal" - f" to Ripple-Epoch {EPOCH_OFFSET} seconds" + f" to ripple epoch - {EPOCH_OFFSET} seconds" ) return errors From 8f2460a297158257faf013b972c56a7bd7356331 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 17 May 2024 16:45:13 -0400 Subject: [PATCH 07/16] fix snippet --- snippets/paths.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/snippets/paths.py b/snippets/paths.py index 4161b6bb3..c795fa13d 100644 --- a/snippets/paths.py +++ b/snippets/paths.py @@ -1,6 +1,8 @@ """Example of how to find the best path to trade with""" + from xrpl.clients import JsonRpcClient from xrpl.models import XRP, IssuedCurrencyAmount, Payment, RipplePathFind +from xrpl.models.path import PathStep from xrpl.transaction import autofill_and_sign from xrpl.wallet import generate_faucet_wallet @@ -41,7 +43,7 @@ account=wallet.address, amount=destination_amount, destination=destination_account, - paths=paths, + paths=[[PathStep(**path_data) for path_data in path] for path in paths], ) print("signed: ", autofill_and_sign(payment_tx, client, wallet)) From eaf2f8b01b570c66d5efc00d185cfac4f0128f97 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 5 Jun 2024 11:48:10 -0400 Subject: [PATCH 08/16] respond to comments --- snippets/paths.py | 3 +-- xrpl/models/base_model.py | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/snippets/paths.py b/snippets/paths.py index c795fa13d..f871759ea 100644 --- a/snippets/paths.py +++ b/snippets/paths.py @@ -2,7 +2,6 @@ from xrpl.clients import JsonRpcClient from xrpl.models import XRP, IssuedCurrencyAmount, Payment, RipplePathFind -from xrpl.models.path import PathStep from xrpl.transaction import autofill_and_sign from xrpl.wallet import generate_faucet_wallet @@ -43,7 +42,7 @@ account=wallet.address, amount=destination_amount, destination=destination_account, - paths=[[PathStep(**path_data) for path_data in path] for path in paths], + paths=paths, ) print("signed: ", autofill_and_sign(payment_tx, client, wallet)) diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index 455b30b0e..c7337a533 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -284,14 +284,13 @@ def _check_type( if expected_type_origin is list: # expected a List, received a List - # TODO: check for if not isinstance(value, list): return {"attr": f"{attr} is {type(value)}, expected {expected_type}"} result = {} for i in range(len(value)): result.update( self._check_type( - f"{attr}[{i}]", value[i], get_args(expected_type)[0] + f"{attr}[{i}]", value[i], get_args(expected_type)[i] ) ) return result @@ -316,6 +315,9 @@ def _check_type( arg = get_args(expected_type) return {} if value in arg else {attr: f"{attr} is {value}, expected {arg}"} + if issubclass(expected_type, BaseModel) and isinstance(value, dict): + return {} + if not isinstance(value, expected_type): return {attr: f"{attr} is {type(value)}, expected {expected_type}"} From 0e04538f4378c1fd991bdd9d6cb1cd8f9562a881 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 5 Jun 2024 12:16:13 -0400 Subject: [PATCH 09/16] fix tests --- tests/unit/models/test_base_model.py | 42 +++++++++++++--------------- xrpl/models/base_model.py | 2 +- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/tests/unit/models/test_base_model.py b/tests/unit/models/test_base_model.py index 3bc54c3fa..0d8d92e4f 100644 --- a/tests/unit/models/test_base_model.py +++ b/tests/unit/models/test_base_model.py @@ -116,11 +116,11 @@ def test_bad_type_enum(self): class TestFromDict(TestCase): maxDiff = 2000 - def test_from_dict_basic(self): + def test_basic(self): amount = IssuedCurrencyAmount.from_dict(amount_dict) self.assertEqual(amount, IssuedCurrencyAmount(**amount_dict)) - def test_from_dict_recursive_amount(self): + def test_recursive_amount(self): check_create = CheckCreate.from_dict(check_create_dict) expected_dict = { @@ -131,7 +131,7 @@ def test_from_dict_recursive_amount(self): } self.assertEqual(expected_dict, check_create.to_dict()) - def test_from_dict_recursive_currency(self): + def test_recursive_currency(self): xrp = {"currency": "XRP"} issued_currency = { "currency": currency, @@ -150,7 +150,7 @@ def test_from_dict_recursive_currency(self): } self.assertEqual(expected_dict, book_offers.to_dict()) - def test_from_dict_recursive_transaction(self): + def test_recursive_transaction(self): transaction = CheckCreate.from_dict(check_create_dict) sign_dict = {"secret": secret, "transaction": transaction.to_dict()} sign = Sign.from_dict(sign_dict) @@ -166,7 +166,7 @@ def test_from_dict_recursive_transaction(self): del expected_dict["transaction"] self.assertEqual(expected_dict, sign.to_dict()) - def test_from_dict_recursive_transaction_tx_json(self): + def test_recursive_transaction_tx_json(self): transaction = CheckCreate.from_dict(check_create_dict) sign_dict = {"secret": secret, "tx_json": transaction.to_dict()} sign = Sign.from_dict(sign_dict) @@ -181,7 +181,7 @@ def test_from_dict_recursive_transaction_tx_json(self): } self.assertEqual(expected_dict, sign.to_dict()) - def test_from_dict_signer(self): + def test_signer(self): dictionary = { "account": "rpqBNcDpWaqZC2Rksayf8UyG66Fyv2JTQy", "fee": "10", @@ -212,7 +212,7 @@ def test_from_dict_signer(self): actual = SignerListSet.from_dict(dictionary) self.assertEqual(actual, expected) - def test_from_dict_trust_set(self): + def test_trust_set(self): dictionary = { "account": "rH6ZiHU1PGamME2LvVTxrgvfjQpppWKGmr", "fee": "10", @@ -236,7 +236,7 @@ def test_from_dict_trust_set(self): actual = TrustSet.from_dict(dictionary) self.assertEqual(actual, expected) - def test_from_dict_list_of_lists(self): + def test_list_of_lists(self): path_step_dict = {"account": "rH6ZiHU1PGamME2LvVTxrgvfjQpppWKGmr"} path_find_dict = { "subcommand": PathFindSubcommand.CREATE, @@ -256,7 +256,7 @@ def test_from_dict_list_of_lists(self): actual = PathFind.from_dict(path_find_dict) self.assertEqual(actual, expected) - def test_from_dict_any(self): + def test_any(self): account_channels_dict = { "account": "rH6ZiHU1PGamME2LvVTxrgvfjQpppWKGmr", "marker": "something", @@ -265,7 +265,7 @@ def test_from_dict_any(self): actual = AccountChannels.from_dict(account_channels_dict) self.assertEqual(actual, expected) - def test_from_dict_bad_str(self): + def test_bad_str(self): dictionary = { "account": "rH6ZiHU1PGamME2LvVTxrgvfjQpppWKGmr", "fee": 10, # this should be a str instead ("10") @@ -280,7 +280,7 @@ def test_from_dict_bad_str(self): with self.assertRaises(XRPLModelException): TrustSet.from_dict(dictionary) - def test_from_dict_explicit_none(self): + def test_explicit_none(self): dictionary = { "account": "rH6ZiHU1PGamME2LvVTxrgvfjQpppWKGmr", "fee": "10", @@ -303,7 +303,7 @@ def test_from_dict_explicit_none(self): actual = TrustSet.from_dict(dictionary) self.assertEqual(actual, expected) - def test_from_dict_with_str_enum_value(self): + def test_with_str_enum_value(self): dictionary = { "method": "account_channels", "account": "rH6ZiHU1PGamME2LvVTxrgvfjQpppWKGmr", @@ -316,7 +316,7 @@ def test_from_dict_with_str_enum_value(self): actual = AccountChannels.from_dict(dictionary) self.assertEqual(actual, expected) - def test_from_dict_bad_list(self): + def test_bad_list(self): dictionary = { "account": "rpqBNcDpWaqZC2Rksayf8UyG66Fyv2JTQy", "fee": "10", @@ -333,7 +333,7 @@ def test_from_dict_bad_list(self): with self.assertRaises(XRPLModelException): SignerListSet.from_dict(dictionary) - def test_from_dict_multisign(self): + def test_multisign(self): txn_sig1 = ( "F80E201FE295AA08678F8542D8FC18EA18D582A0BD19BE77B9A24479418ADBCF4CAD28E7BD" "96137F88DE7736827C7AC6204FBA8DDADB7394E6D704CD1F4CD609" @@ -405,7 +405,7 @@ def test_from_dict_multisign(self): actual = Request.from_dict(request) self.assertEqual(actual, expected) - def test_from_dict_submit(self): + def test_submit(self): blob = "SOISUSF9SD0839W8U98J98SF" id_val = "submit_786514" request = { @@ -418,7 +418,7 @@ def test_from_dict_submit(self): actual = Request.from_dict(request) self.assertEqual(actual, expected) - def test_from_dict_nonexistent_field(self): + def test_nonexistent_field(self): tx = { "account": "rH6ZiHU1PGamME2LvVTxrgvfjQpppWKGmr", "bad_field": "random", @@ -432,7 +432,7 @@ def test_from_dict_nonexistent_field(self): with self.assertRaises(XRPLModelException): TrustSet.from_dict(tx) - def test_from_dict_bad_literal(self): + def test_bad_literal(self): tx = { "account": issuer, "xchain_bridge": { @@ -455,7 +455,7 @@ def test_from_dict_bad_literal(self): with self.assertRaises(XRPLModelException): XChainAddAccountCreateAttestation.from_dict(tx) - def test_from_dict_good_literal(self): + def test_good_literal(self): tx = { "account": issuer, "xchain_bridge": { @@ -484,7 +484,7 @@ def test_from_dict_good_literal(self): ) self.assertEqual(XChainAddAccountCreateAttestation.from_dict(tx), expected) - def test_from_dict_enum(self): + def test_enum(self): path_find_dict = { "subcommand": "create", "source_account": "raoV5dkC66XvGWjSzUhCUuuGM3YFTitMxT", @@ -647,9 +647,7 @@ def test_to_xrpl_paths(self): issuer="rweYz56rfmQ98cAdRaeTxQS9wVMGnrdsFp", value="0.0000002831214446", ), - paths=[ - [PathStep(**path_data) for path_data in path] for path in paths_json - ], + paths=paths_json, sequence=290, ) tx_json = p.to_xrpl() diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index c7337a533..1b5d983da 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -290,7 +290,7 @@ def _check_type( for i in range(len(value)): result.update( self._check_type( - f"{attr}[{i}]", value[i], get_args(expected_type)[i] + f"{attr}[{i}]", value[i], get_args(expected_type)[0] ) ) return result From 84b29a767a874768816a2679b553804b8c4e13f6 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 5 Jun 2024 16:16:06 -0400 Subject: [PATCH 10/16] fix test --- tests/unit/models/test_base_model.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/models/test_base_model.py b/tests/unit/models/test_base_model.py index 0d8d92e4f..e5a52beba 100644 --- a/tests/unit/models/test_base_model.py +++ b/tests/unit/models/test_base_model.py @@ -94,17 +94,17 @@ def test_bad_type(self): def test_bad_type_flags(self): transaction_dict = { - "account": 1, - "amount": 10, - "destination": 1, - "flags": "1234", + "account": account, + "amount": value, + "destination": destination, + "flags": "1234", # should be an int } with self.assertRaises(XRPLModelException): Payment(**transaction_dict) def test_bad_type_enum(self): path_find_dict = { - "subcommand": "blah", + "subcommand": "blah", # this is invalid "source_account": "raoV5dkC66XvGWjSzUhCUuuGM3YFTitMxT", "destination_account": "rJjusz1VauNA9XaHxJoiwHe38bmQFz1sUV", "destination_amount": "100", From 3578749892fc4b1bc2ca22410a06ef80f921274c Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 5 Jun 2024 16:25:51 -0400 Subject: [PATCH 11/16] add comment --- tests/unit/models/test_base_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/models/test_base_model.py b/tests/unit/models/test_base_model.py index e5a52beba..d39163589 100644 --- a/tests/unit/models/test_base_model.py +++ b/tests/unit/models/test_base_model.py @@ -447,7 +447,7 @@ def test_bad_literal(self): "amount": amount_dict, "attestation_reward_account": issuer, "attestation_signer_account": issuer, - "was_locking_chain_send": 2, + "was_locking_chain_send": 2, # supposed to be 0 or 1 "xchain_account_create_count": 12, "destination": issuer, "signature_reward": "200", From e94d97942b95c2c581a638a03c3ca1929153e23a Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 8 Jul 2024 18:37:49 -0400 Subject: [PATCH 12/16] fix merge issue --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index baf33d9eb..d57c71f0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Support for the DeliverMax field in Payment transactions - Support for the `feature` RPC +- Improved validation for models to also check param types ### Fixed - Allow empty strings for the purpose of removing fields in DIDSet transaction From a3a4efe714f75bd651bd5a1bc09f5999f5c2a2c9 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 10 Sep 2024 13:29:59 -0400 Subject: [PATCH 13/16] fix mypy --- xrpl/models/base_model.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index 5aeda0384..65e3493a2 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -304,7 +304,8 @@ def _check_type( return {} return {attr: f"{attr} is {type(value)}, expected {expected_type}"} - if expected_type is Any: + # unsure what the problem with mypy is here + if expected_type is Any: # type: ignore[comparison-overlap] return {} if expected_type_origin is list: From e08ccdf92664961dea3d8c273ac745929e8222f1 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 10 Sep 2024 14:00:41 -0400 Subject: [PATCH 14/16] fix 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 65e3493a2..241705bc0 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -294,7 +294,7 @@ def is_valid(self: Self) -> bool: def _check_type( self: Self, attr: str, value: Any, expected_type: Type[Any] ) -> Dict[str, str]: - # returns error message if type is bad, None if type is good + # returns error dict if type is bad, None if type is good expected_type_origin = get_origin(expected_type) if expected_type_origin is Union: if any( From c745f6926a6649840dc3509a1277db86c37862fc Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 10 Sep 2024 14:14:41 -0400 Subject: [PATCH 15/16] fix other syntax issues --- xrpl/utils/xrp_conversions.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xrpl/utils/xrp_conversions.py b/xrpl/utils/xrp_conversions.py index d8aa2cf3c..5eaaa4829 100644 --- a/xrpl/utils/xrp_conversions.py +++ b/xrpl/utils/xrp_conversions.py @@ -1,4 +1,5 @@ """Conversions between XRP drops and native number types.""" + import re from decimal import Decimal, InvalidOperation, localcontext from typing import Pattern, Union @@ -35,7 +36,7 @@ def xrp_to_drops(xrp: Union[int, float, Decimal]) -> str: TypeError: if ``xrp`` is given as a string XRPRangeException: if the given amount of XRP is invalid """ - if type(xrp) == str: # type: ignore + if isinstance(xrp, str): # This protects people from passing drops to this function and getting # a million times as many drops back. raise TypeError( @@ -83,7 +84,7 @@ def drops_to_xrp(drops: str) -> Decimal: TypeError: if ``drops`` not given as a string XRPRangeException: if the given number of drops is invalid """ - if type(drops) != str: + if not isinstance(drops, str): raise TypeError(f"Drops must be provided as string (got {type(drops)})") drops = drops.strip() with localcontext(DROPS_DECIMAL_CONTEXT): From 786ad8dd59d332e098dcdde78823c930e19c528f Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 19 Sep 2024 16:04:30 -0400 Subject: [PATCH 16/16] fix issues --- xrpl/models/base_model.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/xrpl/models/base_model.py b/xrpl/models/base_model.py index 241705bc0..19ae1f911 100644 --- a/xrpl/models/base_model.py +++ b/xrpl/models/base_model.py @@ -294,7 +294,10 @@ def is_valid(self: Self) -> bool: def _check_type( self: Self, attr: str, value: Any, expected_type: Type[Any] ) -> Dict[str, str]: - # returns error dict if type is bad, None if type is good + """ + Returns error dictionary if the type of `value` does not match the + `expected_type`. + """ expected_type_origin = get_origin(expected_type) if expected_type_origin is Union: if any( @@ -311,7 +314,7 @@ def _check_type( if expected_type_origin is list: # expected a List, received a List if not isinstance(value, list): - return {"attr": f"{attr} is {type(value)}, expected {expected_type}"} + return {attr: f"{attr} is {type(value)}, expected {expected_type}"} result = {} for i in range(len(value)): result.update(