Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credentials #759

Merged
merged 120 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
120 commits
Select commit Hold shift + click to select a range
c7e1a15
Update definitions.json -- generated from the tip of rippled codebase…
ckeshava Oct 15, 2024
9d549c1
Specify the CredentialCreate transaction
ckeshava Oct 16, 2024
8375c42
Files relevant to the CredentialAccept transaction
ckeshava Oct 17, 2024
8d810fc
Files pertaining to the CredentialDelete transaction
ckeshava Oct 17, 2024
0a33121
Files pertaining to DepositPreauth transaction are included in this c…
ckeshava Oct 18, 2024
fd8157d
FIX: Update deposit_preauth integration tests to validate the transac…
ckeshava Oct 18, 2024
1d85bbb
Include account_objects RPC call to verify that cred ledger-object is…
ckeshava Oct 21, 2024
8ea12c4
Updates to DepositAuthorized RPC
ckeshava Oct 21, 2024
e4293be
address PR comments
ckeshava Oct 21, 2024
60b7663
FIX: Use sum() method for error validation in DepositPreauth transact…
ckeshava Oct 22, 2024
509ecd1
Update the definitions.json file without changing the format of the d…
ckeshava Oct 22, 2024
c0151c2
Update base_model.py -- Use mypy with Python 3.8 suggestions
ckeshava Oct 22, 2024
48b8abc
include fixAMM1_1 in the rippled.cfg file
ckeshava Oct 24, 2024
f476a51
rearrange the LedgerEntryTypes in definitions.json
ckeshava Oct 24, 2024
8b82c3d
include amendments which are not in majority consensus yet
ckeshava Oct 25, 2024
a24ca5b
Update tests/unit/models/transactions/test_account_delete.py
ckeshava Oct 30, 2024
4402f6d
Update tests/unit/models/transactions/test_credential_accept.py
ckeshava Oct 30, 2024
22de7a3
refactor DepositPreauth transaction validity checks
ckeshava Oct 30, 2024
f848bbd
refactor suggestions from code-rabbit
ckeshava Oct 30, 2024
cb63c6d
Merge branch 'main' into cred
ckeshava Oct 30, 2024
ffcf1bd
Update xrpl/models/transactions/deposit_preauth.py
ckeshava Oct 31, 2024
e498721
Update xrpl/models/transactions/payment_channel_claim.py
ckeshava Oct 31, 2024
e5a0779
Update xrpl/models/transactions/payment_channel_claim.py
ckeshava Oct 31, 2024
ad4331a
remove unnecessary comments
ckeshava Oct 31, 2024
1a974fb
update error msg in CredentialDelete txn
ckeshava Oct 31, 2024
880a670
Update xrpl/models/transactions/credential_accept.py
ckeshava Oct 31, 2024
af67d78
Update xrpl/models/transactions/credential_accept.py
ckeshava Oct 31, 2024
c50d611
Update xrpl/models/transactions/credential_accept.py
ckeshava Oct 31, 2024
2c887aa
Update xrpl/models/transactions/credential_create.py
ckeshava Oct 31, 2024
8468fcd
Update xrpl/models/transactions/credential_create.py
ckeshava Oct 31, 2024
bba054d
Update xrpl/models/transactions/credential_create.py
ckeshava Oct 31, 2024
6c0b958
Update xrpl/models/transactions/credential_create.py
ckeshava Oct 31, 2024
7f68440
Update xrpl/models/transactions/credential_create.py
ckeshava Oct 31, 2024
1485ae9
Update xrpl/models/transactions/credential_create.py
ckeshava Oct 31, 2024
27f8719
Update xrpl/models/transactions/credential_accept.py
ckeshava Oct 31, 2024
485d392
modified the structure of error collection
ckeshava Oct 31, 2024
562aa49
modified error msg in DepositPreauth txn
ckeshava Oct 31, 2024
d83f0d7
use _MAX_URI_LENGTH variable
ckeshava Oct 31, 2024
a188c1a
Update all the unit tests as per the changes in model files
ckeshava Oct 31, 2024
b97d6f3
Use List instead of Set to store sequence of credentials
ckeshava Oct 31, 2024
379620a
simplify DepositPreauth unit tests
ckeshava Oct 31, 2024
e63b49d
unify the regex used in DIDSet and Credentials
ckeshava Oct 31, 2024
5425b37
remove the use of lambda in collecting errors
ckeshava Oct 31, 2024
3e8b825
Update xrpl/models/transactions/account_delete.py
ckeshava Nov 1, 2024
4106236
Use variables instead of magic numbers, credential length checks
ckeshava Nov 1, 2024
6e42798
remove magic number in credential_delete txn model
ckeshava Nov 1, 2024
0719107
use filter construct for deposit_preauth validity check
ckeshava Nov 1, 2024
f1d2b4e
remove doc-strings alluding to de-duplication
ckeshava Nov 1, 2024
bf5114a
refactor credential_ids check into utils file
ckeshava Nov 1, 2024
6d8f1be
Update xrpl/models/transactions/escrow_finish.py
ckeshava Nov 4, 2024
8e496e1
address PR comments from Mayukha
ckeshava Nov 4, 2024
5656080
Update tests/integration/transactions/test_credential.py
ckeshava Nov 4, 2024
b43aa12
Update xrpl/models/utils.py
ckeshava Nov 4, 2024
6392351
Update xrpl/models/transactions/credential_create.py
ckeshava Nov 4, 2024
4ad028b
Update xrpl/models/transactions/credential_delete.py
ckeshava Nov 4, 2024
1f25b01
Update xrpl/models/transactions/deposit_preauth.py
ckeshava Nov 4, 2024
a870f2d
Merge branch 'main' into cred
ckeshava Nov 4, 2024
a6ac0f4
address PR comments; check for duplicates in the credential list
ckeshava Nov 7, 2024
111e12a
Update xrpl/models/transactions/credential_create.py
ckeshava Nov 7, 2024
8593111
Update xrpl/models/transactions/credential_delete.py
ckeshava Nov 7, 2024
7fa47e1
Update xrpl/models/transactions/credential_create.py
ckeshava Nov 7, 2024
3d66a36
address PR comments
ckeshava Nov 7, 2024
2d86ea2
address PR comments
ckeshava Nov 8, 2024
f58dc2c
update definitions.json to match rippled develop branch
ckeshava Nov 9, 2024
0f7f745
use short-circuit in an if-condition
ckeshava Nov 9, 2024
b02f902
[WIP] Fix CICD integration tests
ckeshava Nov 12, 2024
b70ac84
Merge branch 'main' into cred
ckeshava Nov 12, 2024
7f79596
CI/CD Fix: Update docker-stop command
ckeshava Nov 12, 2024
3814a94
LedgerEntry RPC: Introduce Credential functionality
ckeshava Nov 12, 2024
c721a87
Update Contributing guidelines
ckeshava Nov 12, 2024
c76b755
fix black linter errors
ckeshava Nov 12, 2024
9b7d17c
fix mypy errors
ckeshava Nov 12, 2024
5068266
re-introduce docker health checks with server_info RPC
ckeshava Nov 12, 2024
c7a6e2e
remove extraneous comments
ckeshava Nov 15, 2024
344bb49
[WIP] use single docker run command
ckeshava Nov 15, 2024
ee01fc9
[WIP] Revert some changes to the CICD integration tests
ckeshava Nov 15, 2024
09b7d18
[refactor] address Ashray PR suggestion
ckeshava Nov 15, 2024
87d2af7
update release notes in CHANGELOG
ckeshava Nov 15, 2024
084bc04
Update CHANGELOG.md
ckeshava Dec 16, 2024
5828d8e
Update xrpl/models/transactions/deposit_preauth.py
ckeshava Dec 16, 2024
c27ab41
address PR comments from @mvadari
ckeshava Dec 16, 2024
1195417
Update .ci-config/rippled.cfg
ckeshava Dec 17, 2024
1a5fc8b
Update xrpl/models/utils.py
ckeshava Dec 17, 2024
7d1781a
Update tests/unit/models/transactions/test_credential_create.py
ckeshava Dec 17, 2024
ae55a83
Update tests/unit/models/transactions/test_credential_accept.py
ckeshava Dec 17, 2024
bc71c4c
Update xrpl/models/utils.py
ckeshava Dec 17, 2024
c1adc90
Update xrpl/models/utils.py
ckeshava Dec 17, 2024
cf6e618
Update tests/unit/models/transactions/test_credential_delete.py
ckeshava Dec 17, 2024
a6d4f1e
Update tests/unit/models/transactions/test_credential_create.py
ckeshava Dec 17, 2024
cbfb27b
Update tests/unit/models/transactions/test_payment.py
ckeshava Dec 17, 2024
ed6f6ef
Update xrpl/models/requests/deposit_authorized.py
ckeshava Dec 17, 2024
36e2fd6
Update tests/unit/models/transactions/test_credential_delete.py
ckeshava Dec 17, 2024
a039075
Update xrpl/models/utils.py
ckeshava Dec 17, 2024
eb4e176
Update xrpl/models/utils.py
ckeshava Dec 17, 2024
7bf9a37
Update xrpl/models/utils.py
ckeshava Dec 17, 2024
657af63
Update tests/unit/models/transactions/test_credential_create.py
ckeshava Dec 17, 2024
5669d4b
Update tests/unit/models/transactions/test_credential_create.py
ckeshava Dec 17, 2024
63336b3
Update tests/unit/models/transactions/test_credential_delete.py
ckeshava Dec 17, 2024
54d894d
shorten variable names _ACCOUNT_ISSUER
ckeshava Dec 17, 2024
d754702
use SHA512Half values for Credential IDs
ckeshava Dec 17, 2024
397d104
include multiple credential_ids in PayChanClaim unit tests
ckeshava Dec 17, 2024
1093164
Apply suggestions from code review
ckeshava Dec 17, 2024
46861f4
address PR comments: refactor _get_errors() method
ckeshava Dec 17, 2024
2593446
rectify all unit-test error messages
ckeshava Dec 17, 2024
98e0f79
Merge branch 'main' into cred
ckeshava Dec 17, 2024
70f9301
rewrite DepositPreauth unit test to use Credential object
ckeshava Dec 17, 2024
14893ee
pacify the linter
ckeshava Dec 17, 2024
24dcb32
Update xrpl/models/transactions/credential_accept.py
ckeshava Dec 20, 2024
383a325
Update xrpl/models/transactions/deposit_preauth.py
ckeshava Dec 20, 2024
42e8e71
Update xrpl/models/transactions/credential_delete.py
ckeshava Dec 20, 2024
e168dcb
Update xrpl/models/transactions/credential_create.py
ckeshava Dec 20, 2024
387f72b
addressed Omar's comments
ckeshava Dec 20, 2024
8e209a1
Update xrpl/models/transactions/credential_create.py
ckeshava Dec 20, 2024
8a93ed7
Update xrpl/models/transactions/credential_create.py
ckeshava Dec 20, 2024
a059003
pacify the linter
ckeshava Dec 20, 2024
d823911
Merge branch 'main' into cred
ckeshava Dec 20, 2024
aeabe63
add recent amendments in rippled
ckeshava Dec 21, 2024
26e1b13
Update .ci-config/rippled.cfg
ckeshava Dec 21, 2024
1e09926
Update CHANGELOG.md
ckeshava Dec 21, 2024
cd7f860
include None check in validate_creds method
ckeshava Dec 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .ci-config/rippled.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,4 @@ PriceOracle
fixEmptyDID
fixXChainRewardRounding
fixPreviousTxnID
Credentials
108 changes: 108 additions & 0 deletions tests/integration/transactions/test_credential.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
from tests.integration.integration_test_case import IntegrationTestCase
from tests.integration.it_utils import (
sign_and_reliable_submission_async,
test_async_and_sync,
)
from tests.integration.reusable_values import DESTINATION, WALLET
from xrpl.models import AccountObjects, AccountObjectType
from xrpl.models.response import ResponseStatus
from xrpl.models.transactions.credential_accept import CredentialAccept
from xrpl.models.transactions.credential_create import CredentialCreate
from xrpl.models.transactions.credential_delete import CredentialDelete
from xrpl.utils import str_to_hex

_URI = "www.my-id.com/username"


def is_cred_object_present(result, issuer, subject, cred_type) -> bool:
"""
Utility method that checks if the specified JSON contains the Credential ledger
object. The result JSON must be the output of account_objects RPC command.

Returns True, if the input JSON contains the Credential Ledger object
Returns False, otherwise
"""

for val in result["account_objects"]:
if (
val["Issuer"] == issuer
and val["Subject"] == subject
and val["CredentialType"] == cred_type
):
return True

return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance type safety and error handling.

The utility function could benefit from type hints and better error handling.

-def is_cred_object_present(result, issuer, subject, cred_type) -> bool:
+def is_cred_object_present(
+    result: dict,
+    issuer: str,
+    subject: str,
+    cred_type: str,
+) -> bool:
     """
     Utility method that checks if the specified JSON contains the Credential ledger
     object. The result JSON must be the output of account_objects RPC command.
 
-    Returns True, if the input JSON contains the Credential Ledger object
-    Returns False, otherwise
+    Args:
+        result: JSON response from account_objects RPC
+        issuer: Address of the credential issuer
+        subject: Address of the credential subject
+        cred_type: Type of the credential
+
+    Returns:
+        bool: True if credential exists, False otherwise
+
+    Raises:
+        KeyError: If result doesn't contain expected fields
     """
+    if "account_objects" not in result:
+        raise KeyError("Invalid result format: missing 'account_objects'")
 
     for val in result["account_objects"]:
         if (

Committable suggestion was skipped due to low confidence.



class TestCredentialCreate(IntegrationTestCase):
@test_async_and_sync(globals())
async def test_valid(self, client):
# Define entities helpful for Credential lifecycle
_ISSUER = WALLET.address
_SUBJECT = DESTINATION.address

# Disambiguate the sync/async, json/websocket tests with different
# credential type values -- this avoids tecDUPLICATE error
# self.value is defined inside the above decorator
cred_type = str_to_hex("Passport_" + str(self.value))

tx = CredentialCreate(
account=_ISSUER,
subject=_SUBJECT,
credential_type=cred_type,
uri=str_to_hex(_URI),
)
response = await sign_and_reliable_submission_async(tx, WALLET, client)
self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")

# Note: If it isn't too cluttered, verification tests pertaining to the
# existence of Credential ledger object on the Issuer's and Subject's directory
# pages can be included here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement the suggested verification tests.

The comment indicates missing verification tests. These should be implemented to ensure the credential object exists in both issuer's and subject's directory pages immediately after creation.

+        # Verify credential exists in issuer's directory
+        account_objects_response = await client.request(
+            AccountObjects(account=_ISSUER, type=AccountObjectType.CREDENTIAL)
+        )
+        self.assertTrue(
+            is_cred_object_present(
+                account_objects_response.result,
+                issuer=_ISSUER,
+                subject=_SUBJECT,
+                cred_type=cred_type,
+            )
+        )
+
+        # Verify credential exists in subject's directory
+        account_objects_response = await client.request(
+            AccountObjects(account=_SUBJECT, type=AccountObjectType.CREDENTIAL)
+        )
+        self.assertTrue(
+            is_cred_object_present(
+                account_objects_response.result,
+                issuer=_ISSUER,
+                subject=_SUBJECT,
+                cred_type=cred_type,
+            )
+        )

Committable suggestion was skipped due to low confidence.


# Execute the CredentialAccept transaction on the above Credential ledger object
tx = CredentialAccept(
issuer=_ISSUER, account=_SUBJECT, credential_type=cred_type
)
# CredentialAccept transaction is submitted by SUBJECT
response = await sign_and_reliable_submission_async(tx, DESTINATION, client)
self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")

# Execute the CredentialDelete transaction
# Subject initiates the deletion of the Credential ledger object
tx = CredentialDelete(
issuer=_ISSUER, account=_SUBJECT, credential_type=cred_type
)

response = await sign_and_reliable_submission_async(tx, DESTINATION, client)
self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")

# The credential ledger object must be deleted from both the Issuer and Subject
# account's directory pages
account_objects_response = await client.request(
AccountObjects(account=_ISSUER, type=AccountObjectType.CREDENTIAL)
)
self.assertFalse(
is_cred_object_present(
account_objects_response.result,
issuer=_ISSUER,
subject=_SUBJECT,
cred_type=cred_type,
)
)

# Verify that the Credential object has been deleted from the Subject's
# directory page as well
account_objects_response = await client.request(
AccountObjects(account=_SUBJECT, type=AccountObjectType.CREDENTIAL)
)
self.assertFalse(
is_cred_object_present(
account_objects_response.result,
issuer=_ISSUER,
subject=_SUBJECT,
cred_type=cred_type,
)
)
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add cleanup for test resilience.

The test should clean up any remaining credential objects in case of test failures. Consider using setUp and tearDown methods.

 class TestCredentialCreate(IntegrationTestCase):
+    async def setUp(self):
+        await super().setUp()
+        self._ISSUER = WALLET.address
+        self._SUBJECT = DESTINATION.address
+        self.cred_type = None
+
+    async def tearDown(self):
+        # Clean up any remaining credentials
+        if self.cred_type:
+            try:
+                tx = CredentialDelete(
+                    issuer=self._ISSUER,
+                    account=self._SUBJECT,
+                    credential_type=self.cred_type,
+                )
+                await sign_and_reliable_submission_async(tx, DESTINATION, self.client)
+            except Exception:
+                pass  # Best effort cleanup
+        await super().tearDown()

Committable suggestion was skipped due to low confidence.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the setup and teardown is performed by the CredentialCreate and CredentialDelete transactions. I don't want to abstract these transactions in separate util functions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ckeshava, understood. I'll keep in mind that the setup and teardown are performed using the CredentialCreate and CredentialDelete transactions, and that you prefer not to abstract these transactions into separate utility functions.

(_/)
( •_•)


✏️ Learnings added
Learnt from: ckeshava
PR: XRPLF/xrpl-py#759
File: tests/integration/transactions/test_credential.py:37-108
Timestamp: 2024-10-30T20:28:45.513Z
Learning: In integration tests, setup and teardown are performed using the `CredentialCreate` and `CredentialDelete` transactions, and we prefer not to abstract these transactions into separate utility functions.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

44 changes: 38 additions & 6 deletions tests/integration/transactions/test_deposit_preauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,65 @@
sign_and_reliable_submission_async,
test_async_and_sync,
)
from tests.integration.reusable_values import WALLET
from tests.integration.reusable_values import DESTINATION, WALLET
from xrpl.models.response import ResponseStatus
from xrpl.models.transactions import DepositPreauth
from xrpl.models.transactions.deposit_preauth import Credential
from xrpl.utils import str_to_hex

ACCOUNT = WALLET.address
ADDRESS = "rEhxGqkqPPSxQ3P25J66ft5TwpzV14k2de"


class TestDepositPreauth(IntegrationTestCase):
@test_async_and_sync(globals())
async def test_authorize(self, client):
async def test_authorize_unauthorize_fields(self, client):
deposit_preauth = DepositPreauth(
account=ACCOUNT,
authorize=ADDRESS,
authorize=DESTINATION.address,
)
response = await sign_and_reliable_submission_async(
deposit_preauth, WALLET, client
)
self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")

# validate the un-authorization of the same address as above
deposit_preauth = DepositPreauth(
account=ACCOUNT,
unauthorize=DESTINATION.address,
)
response = await sign_and_reliable_submission_async(
deposit_preauth, WALLET, client
)
self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")

@test_async_and_sync(globals())
async def test_unauthorize(self, client):
async def test_credentials_array_input_fields(self, client):
sample_credentials = [
Credential(
issuer=DESTINATION.address, credential_type=str_to_hex("SampleCredType")
)
]

# Test the authorize_credentials input field
deposit_preauth = DepositPreauth(
account=ACCOUNT,
authorize_credentials=sample_credentials,
)
response = await sign_and_reliable_submission_async(
deposit_preauth, WALLET, client
)
self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")

# Test the unauthorize_credentials input field
deposit_preauth = DepositPreauth(
account=ACCOUNT,
unauthorize=ADDRESS,
unauthorize_credentials=sample_credentials,
)
response = await sign_and_reliable_submission_async(
deposit_preauth, WALLET, client
)
self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")
15 changes: 15 additions & 0 deletions tests/unit/models/requests/test_deposit_authorized.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from unittest import TestCase

from xrpl.models import DepositAuthorized


class TestDepositAuthorized(TestCase):
def test_valid(self):
req = DepositAuthorized(
source_account="srcAccount",
destination_account="dstAccount",
credentials=[
"EA85602C1B41F6F1F5E83C0E6B87142FB8957BD209469E4CC347BA2D0C26F66A"
],
)
self.assertTrue(req.is_valid())
45 changes: 45 additions & 0 deletions tests/unit/models/transactions/test_account_delete.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
from unittest import TestCase

from xrpl.models.exceptions import XRPLModelException
from xrpl.models.transactions import AccountDelete

_ACCOUNT = "r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ"
_DESTINATION = "rf7HPydP4ihkFkSRHWFq34b4SXRc7GvPCR"


class TestAccountDelete(TestCase):
def test_creds_list_too_long(self):
with self.assertRaises(XRPLModelException) as err:
AccountDelete(
account=_ACCOUNT,
destination=_DESTINATION,
credential_ids=["credential_index_" + str(i) for i in range(9)],
)

self.assertEqual(
err.exception.args[0],
"{'credential_ids': 'CredentialIDs list cannot have more than 8 "
+ "elements.'}",
)

def test_creds_list_empty(self):
with self.assertRaises(XRPLModelException) as err:
AccountDelete(
account=_ACCOUNT,
destination=_DESTINATION,
credential_ids=[],
)
self.assertEqual(
err.exception.args[0],
"{'credential_ids': 'CredentialIDs list cannot be empty.'}",
)

def test_valid_account_delete_txn(self):
tx = AccountDelete(
account=_ACCOUNT,
destination=_DESTINATION,
credential_ids=[
"EA85602C1B41F6F1F5E83C0E6B87142FB8957BD209469E4CC347BA2D0C26F66A"
],
)
self.assertTrue(tx.is_valid())
59 changes: 59 additions & 0 deletions tests/unit/models/transactions/test_credential_accept.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
from unittest import TestCase

from xrpl.models.exceptions import XRPLModelException
from xrpl.models.transactions.credential_accept import CredentialAccept
from xrpl.utils import str_to_hex

_ACCOUNT_ISSUER = "r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ"
_ACCOUNT_SUBJECT = "rNdY9XDnQ4Dr1EgefwU3CBRuAjt3sAutGg"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The account prefix is unnecessary here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside the context of credentials, it is not immediately obvious if Subject refers to an "account". I prefer to retain the Account prefix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think of _ISSUER_ACCT and _SUBJECT_ACCT ? Is that more informative?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @mvadari , it's sufficient to just have _ISSUER and _SUBJECT since the context is obvious being inside a Credential test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_VALID_CREDENTIAL_TYPE = str_to_hex("Passport")


class TestCredentialAccept(TestCase):
def test_valid(self):
tx = CredentialAccept(
issuer=_ACCOUNT_ISSUER,
account=_ACCOUNT_SUBJECT,
credential_type=_VALID_CREDENTIAL_TYPE,
)
self.assertTrue(tx.is_valid())

# invalid inputs to the credential_type field
def test_cred_type_field_too_long(self):
with self.assertRaises(XRPLModelException) as error:
CredentialAccept(
issuer=_ACCOUNT_ISSUER,
account=_ACCOUNT_SUBJECT,
credential_type=str_to_hex("A" * 65),
)
self.assertEqual(
error.exception.args[0],
"{'credential_type': 'Length of credential_type field must not be greater "
+ "than 64 bytes. '}",
)

def test_cred_type_field_empty(self):
with self.assertRaises(XRPLModelException) as error:
CredentialAccept(
issuer=_ACCOUNT_ISSUER,
account=_ACCOUNT_SUBJECT,
credential_type="",
)
self.assertEqual(
error.exception.args[0],
"{'credential_type': 'Length of credential_type field must be greater than "
+ "0. credential_type field must be encoded in base-16 format. '}",
)

def test_cred_type_field_not_hex(self):
with self.assertRaises(XRPLModelException) as error:
CredentialAccept(
issuer=_ACCOUNT_ISSUER,
account=_ACCOUNT_SUBJECT,
credential_type="Passport",
)
self.assertEqual(
error.exception.args[0],
"{'credential_type': 'credential_type field must be encoded in base-16 "
+ "format. '}",
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding boundary test cases.

The current tests cover important invalid scenarios, but consider adding these additional test cases:

  1. Test with exactly 64 bytes (boundary condition)
  2. Test with special characters in hex string
  3. Test with mixed case hex string
def test_cred_type_field_exact_length(self):
    # Test with exactly 64 bytes
    tx = CredentialAccept(
        issuer=_ACCOUNT_ISSUER,
        account=_ACCOUNT_SUBJECT,
        credential_type=str_to_hex("A" * 32),  # 32 chars = 64 bytes in hex
    )
    self.assertTrue(tx.is_valid())

def test_cred_type_field_special_hex(self):
    # Test with valid hex containing special characters
    with self.assertRaises(XRPLModelException) as error:
        CredentialAccept(
            issuer=_ACCOUNT_ISSUER,
            account=_ACCOUNT_SUBJECT,
            credential_type="!@#$%^",
        )

def test_cred_type_field_mixed_case(self):
    # Test with mixed case hex string
    tx = CredentialAccept(
        issuer=_ACCOUNT_ISSUER,
        account=_ACCOUNT_SUBJECT,
        credential_type="aAbBcCdD",
    )
    self.assertTrue(tx.is_valid())

Loading
Loading