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

Verifiable Credentials #759

Open
wants to merge 78 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
78 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
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
2 changes: 2 additions & 0 deletions .ci-config/rippled.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,5 @@ PriceOracle
fixEmptyDID
fixXChainRewardRounding
fixPreviousTxnID
fixAMMv1_1
Credentials
mvadari marked this conversation as resolved.
Show resolved Hide resolved
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
mvadari marked this conversation as resolved.
Show resolved Hide resolved

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):
ckeshava marked this conversation as resolved.
Show resolved Hide resolved
@test_async_and_sync(globals())
async def test_valid(self, client):
mvadari marked this conversation as resolved.
Show resolved Hide resolved
# 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")

mvadari marked this conversation as resolved.
Show resolved Hide resolved
# 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.

Comment on lines +95 to +121
Copy link
Collaborator

@mvadari mvadari Nov 1, 2024

Choose a reason for hiding this comment

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

This check is unnecessary. We're testing the library, not rippled. Checking just one is more than enough. You could even just check for whether the object exists with ledger_entry.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The choice between ledger_entry and account_objects is inconsequential, because both of them refer to the same ledger object.

There is no harm in additional check for the existence/deletion of ledger objects. It serves to explain the behavior of credentials better.

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")
mvadari marked this conversation as resolved.
Show resolved Hide resolved
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())
mvadari marked this conversation as resolved.
Show resolved Hide resolved
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.'}",
)
ckeshava marked this conversation as resolved.
Show resolved Hide resolved

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())
Comment on lines +52 to +60
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

Add validation for credential ID format.

The test verifies basic functionality but should also validate the credential ID format:

     def test_valid_account_delete_txn(self):
+        """Test valid AccountDelete transaction with properly formatted credential ID.
+        
+        The credential ID should be a 32-byte (64 character) hexadecimal string.
+        """
         tx = AccountDelete(
             account=_ACCOUNT,
             destination=_DESTINATION,
             credential_ids=[
                 "EA85602C1B41F6F1F5E83C0E6B87142FB8957BD209469E4CC347BA2D0C26F66A"
             ],
         )
         self.assertTrue(tx.is_valid())
+        self.assertEqual(len(tx.credential_ids[0]), 64)  # 32 bytes in hex
+        self.assertTrue(all(c in '0123456789ABCDEFabcdef' for c in tx.credential_ids[0]))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_valid_account_delete_txn(self):
tx = AccountDelete(
account=_ACCOUNT,
destination=_DESTINATION,
credential_ids=[
"EA85602C1B41F6F1F5E83C0E6B87142FB8957BD209469E4CC347BA2D0C26F66A"
],
)
self.assertTrue(tx.is_valid())
def test_valid_account_delete_txn(self):
"""Test valid AccountDelete transaction with properly formatted credential ID.
The credential ID should be a 32-byte (64 character) hexadecimal string.
"""
tx = AccountDelete(
account=_ACCOUNT,
destination=_DESTINATION,
credential_ids=[
"EA85602C1B41F6F1F5E83C0E6B87142FB8957BD209469E4CC347BA2D0C26F66A"
],
)
self.assertTrue(tx.is_valid())
self.assertEqual(len(tx.credential_ids[0]), 64) # 32 bytes in hex
self.assertTrue(all(c in '0123456789ABCDEFabcdef' for c in tx.credential_ids[0]))

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"
_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())

ckeshava marked this conversation as resolved.
Show resolved Hide resolved
# 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