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

XLS-80d Permissioned Domains #773

Merged
merged 100 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from 88 commits
Commits
Show all changes
100 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
8abf7b7
Models for PD transactions
ckeshava Nov 14, 2024
26ea9b6
Update CL
ckeshava Nov 15, 2024
e5ad6c1
Update CHANGELOG.md
ckeshava Nov 18, 2024
b7cd2b0
fix: Update the ledger_entry RPC model
ckeshava Nov 20, 2024
7549e48
refactor: Use LedgerEntryType instead of AccountObjectsType; Rectify …
ckeshava Nov 20, 2024
5bdbf55
fix: Update ledger_entry unit test
ckeshava Nov 20, 2024
12d7075
update CL
ckeshava Nov 20, 2024
fb98fd5
deprecate AccountObjectType enum
ckeshava Jan 8, 2025
f65ba6b
Merge branch 'main' into xls_80d
ckeshava Jan 13, 2025
e00d485
revert the updates to AccountObjects and ledger_entry RPC method; Upd…
ckeshava Jan 15, 2025
19c3caa
update the change-log
ckeshava Jan 15, 2025
d42ebb9
revert the github CI/CD instructions
ckeshava Jan 15, 2025
cb9ec77
revert integration test suite updates to AccountObjects RPC
ckeshava Jan 15, 2025
bc8954b
suggestions from the linter
ckeshava Jan 15, 2025
adf52d3
update the Github CI/CD to use develop branch of rippled
ckeshava Jan 15, 2025
20762f8
address PR comments
ckeshava Feb 3, 2025
c3a15b9
Update tests/unit/models/requests/test_ledger_entry.py
ckeshava Feb 3, 2025
55239ba
no duplicates test: PDSet transaction
ckeshava Feb 3, 2025
39ec51e
address PR comments
ckeshava Feb 6, 2025
abd94f3
Merge branch 'main' into xls_80d
ckeshava Feb 11, 2025
489255e
add unit tests for PDDelete
ckeshava Feb 11, 2025
084d876
reword comment: replace aliter with alternatively
ckeshava Feb 11, 2025
7069c8d
Update xrpl/models/transactions/permissioned_domain_delete.py
ckeshava Feb 11, 2025
b7a185a
Update tests/integration/transactions/test_permissioned_domain.py
ckeshava Feb 12, 2025
3cddf4b
fix linter errors
ckeshava Feb 12, 2025
7e9fdfb
introduce DOMAIN_ID_LENGTH variable
ckeshava Feb 12, 2025
8206d73
update the error message to use f-strings
ckeshava Feb 12, 2025
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 @@ -194,6 +194,7 @@ fixNFTokenPageLinks
fixInnerObjTemplate2
fixEnforceNFTokenTrustline
fixReducedOffersV2
PermissionedDomains

# This section can be used to simulate various FeeSettings scenarios for rippled node in standalone mode
[voting]
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/integration_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: Integration test

env:
POETRY_VERSION: 1.8.3
RIPPLED_DOCKER_IMAGE: rippleci/rippled:2.3.0
RIPPLED_DOCKER_IMAGE: rippleci/rippled:develop

on:
push:
Expand Down Expand Up @@ -32,7 +32,7 @@ jobs:

- name: Run docker in background
run: |
docker run --detach --rm -p 5005:5005 -p 6006:6006 --volume "${{ github.workspace }}/.ci-config/":"/etc/opt/ripple/" --name rippled-service --health-cmd="rippled server_nfo || exit 1" --health-interval=5s --health-retries=10 --health-timeout=2s --env GITHUB_ACTIONS=true --env CI=true --entrypoint bash ${{ env.RIPPLED_DOCKER_IMAGE }} -c "rippled -a"
docker run --detach --rm -p 5005:5005 -p 6006:6006 --volume "${{ github.workspace }}/.ci-config/":"/etc/opt/ripple/" --name rippled-service --health-cmd="rippled server_info || exit 1" --health-interval=5s --health-retries=10 --health-timeout=2s --env GITHUB_ACTIONS=true --env CI=true --entrypoint bash ${{ env.RIPPLED_DOCKER_IMAGE }} -c "rippled -a"

- name: Install poetry
if: steps.cache-poetry.outputs.cache-hit != 'true'
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [[Unreleased]]

### Added
- Add support for the `PermissionedDomains` feature

## [4.0.0] - 2024-12-23

### Added
Expand Down Expand Up @@ -96,6 +99,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [2.0.0] - 2023-07-05
### BREAKING CHANGE
- The default signing algorithm in the `Wallet` was changed from secp256k1 to ed25519

### Added:
- Wallet support for regular key compatibility
- Added new ways of wallet generation: `from_seed`, `from_secret`, `from_entropy`, `from_secret_numbers`
Expand Down
25 changes: 25 additions & 0 deletions tests/integration/reusable_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
XRP,
AMMDeposit,
AMMDepositFlag,
CredentialAccept,
CredentialCreate,
IssuedCurrencyAmount,
OfferCreate,
PaymentChannelCreate,
Expand All @@ -21,6 +23,7 @@
XChainBridge,
XChainCreateBridge,
)
from xrpl.utils import str_to_hex
from xrpl.wallet import Wallet


Expand Down Expand Up @@ -95,6 +98,26 @@ async def _set_up_reusable_values():
amm_asset2 = setup_amm_pool_res["asset2"]
amm_issuer_wallet = setup_amm_pool_res["issuer_wallet"]

# Create and accept one credential; This is used in the PermissionedDomain tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not necessary. You can create a Permissioned Domain without any credentials existing for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that is true, but I'd like to test for a tesSUCCESS code at the end. If the credential-object doesn't exist, I wasn't sure if rippled throws an error.

Besides, this displays the general workflow with PD

Copy link
Collaborator

Choose a reason for hiding this comment

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

The credential object does not need to exist. In fact, there's no way for rippled to check for it, since the subject could be anybody.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nonetheless, a re-usable credential in the integration test cases would be useful for upcoming amendments. I prefer to retain this piece of code.

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 . We should only add necessary things for tests. If I didn't have context on this feature, I would also assume a credential object is required for this to work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are you guys suggesting that I create a PermissionedDomain object which cannot be amenable with any Credential object? That is ill-advised.

Integration tests serve as useful examples for developers. Your suggestion mis-directs the developer and understates the importance of valid Credentials. The education of the end-user should take precedence over DGE team's ease-of-understanding.

@mvadari In a typical scenario, a user interacts with this PermissionedDomain object, apart from the PDSet and PDDelete transactions. Surely, those interactions will fail without a valid credential.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm suggesting not creating a credential in this file, and only creating it as needed. If you need a credential in the PD tests, then create a credential there, not here.

cred_type: str = str_to_hex("IdentityDocument")
await sign_and_reliable_submission_async(
CredentialCreate(
account=wallet.address,
subject=destination.address,
credential_type=cred_type,
),
wallet,
)

credential_accept_txn_response = await sign_and_reliable_submission_async(
CredentialAccept(
issuer=wallet.address,
credential_type=cred_type,
account=destination.address,
),
destination,
)

return (
wallet,
destination,
Expand All @@ -106,6 +129,7 @@ async def _set_up_reusable_values():
amm_asset2,
amm_issuer_wallet,
bridge,
credential_accept_txn_response,
)


Expand Down Expand Up @@ -149,4 +173,5 @@ async def setup_amm_pool(
AMM_ASSET2,
AMM_ISSUER_WALLET,
BRIDGE,
CREDENTIAL_ACCEPT_RESPONSE,
) = asyncio.run(_set_up_reusable_values())
102 changes: 102 additions & 0 deletions tests/integration/transactions/test_permissioned_domain.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
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 CREDENTIAL_ACCEPT_RESPONSE, WALLET
from xrpl.models.requests.account_objects import AccountObjects, AccountObjectType
from xrpl.models.requests.ledger_data import LedgerData
from xrpl.models.requests.ledger_entry import LedgerEntry, PermissionedDomain
from xrpl.models.response import ResponseStatus
from xrpl.models.transactions.permissioned_domain_delete import PermissionedDomainDelete
from xrpl.models.transactions.permissioned_domain_set import (
Credential,
PermissionedDomainSet,
)


class TestPermissionedDomain(IntegrationTestCase):
@test_async_and_sync(globals())
async def test_valid_pd_workflow(self, client):

# Note: In the below test, WALLET is the "Issuer" of the said credential and is
# also the "Owner" of the PermissionedDomain, but these roles need not be
# fulfilled by the same account

# Step-1: Create a PermissionedDomain object
response = await sign_and_reliable_submission_async(
PermissionedDomainSet(
account=WALLET.address,
accepted_credentials=[
Credential(
credential_type=CREDENTIAL_ACCEPT_RESPONSE.result["tx_json"][
"CredentialType"
],
issuer=CREDENTIAL_ACCEPT_RESPONSE.result["tx_json"]["Issuer"],
)
],
),
WALLET,
client,
)

self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")

# Step-2: Verify the existence of the above PD ledger object through the RPC
# calls -- ledger_entry, ledger_data, account_objects

# Use the account_objects RPC to verify the creation of the PermissionedDomain
# ledger object
account_objects_response = await client.request(
AccountObjects(
account=WALLET.address, type=AccountObjectType.PERMISSIONED_DOMAIN
)
)
self.assertEqual(account_objects_response.status, ResponseStatus.SUCCESS)

# Use the LedgerEntry RPC to validate the creation of the credential object
ledger_entry_response = await client.request(
LedgerEntry(
# Use the LedgerIndex from the account_objects RPC response
permissioned_domain=account_objects_response.result["account_objects"][
0
]["index"]
)
)
self.assertEqual(ledger_entry_response.status, ResponseStatus.SUCCESS)

# Aliter: Use the account and sequence-number to retrieve a PermissionedDomain
ledger_entry_response = await client.request(
LedgerEntry(
permissioned_domain=PermissionedDomain(
account=WALLET.address,
seq=account_objects_response.result["account_objects"][0][
"Sequence"
],
)
)
)

self.assertEqual(ledger_entry_response.status, ResponseStatus.SUCCESS)

# Use the ledger_data command to validate the creation of PermissionedDomain
# object
ledger_data_response = await client.request(
LedgerData(ledger_index=response.result["validated_ledger_index"])
)
self.assertEqual(ledger_data_response.status, ResponseStatus.SUCCESS)

# Step-3: Delete the PD object

response = await sign_and_reliable_submission_async(
PermissionedDomainDelete(
account=WALLET.address,
domain_id=ledger_entry_response.result["index"],
),
WALLET,
client,
)

self.assertEqual(response.status, ResponseStatus.SUCCESS)
self.assertEqual(response.result["engine_result"], "tesSUCCESS")
18 changes: 17 additions & 1 deletion tests/unit/models/requests/test_ledger_entry.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

from xrpl.models import XRP, LedgerEntry, XChainBridge
from xrpl.models.exceptions import XRPLModelException
from xrpl.models.requests.ledger_entry import Credential, MPToken, Oracle, RippleState
from xrpl.models.requests.ledger_entry import (
Credential,
MPToken,
Oracle,
PermissionedDomain,
RippleState,
)


class TestLedgerEntry(TestCase):
Expand Down Expand Up @@ -170,6 +176,16 @@ def test_invalid_price_oracle_object(self):
oracle=Oracle(oracle_document_id=1),
)

def test_fetch_permissioned_domain_ledger_index(self):
self.assertTrue(LedgerEntry(permissioned_domain="LEDGEROBJECTHASH"))

def test_fetch_permissioned_domain_ledger_object_params(self):
self.assertTrue(
LedgerEntry(
permissioned_domain=PermissionedDomain(account="rAccount", seq=1234)
)
)

def test_get_mpt_issuance(self):
req = LedgerEntry(
mpt_issuance="rB6XJbxKx2oBSK1E3Hvh7KcZTCCBukWyhv",
Expand Down
56 changes: 56 additions & 0 deletions tests/unit/models/transactions/test_permissioned_domain_set.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
from unittest import TestCase

from xrpl.models.exceptions import XRPLModelException
from xrpl.models.transactions.deposit_preauth import Credential
from xrpl.models.transactions.permissioned_domain_set import PermissionedDomainSet
from xrpl.utils import str_to_hex

_ACCOUNT_1 = "r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ"
_ACCOUNT_SUBJECT = "rNdY9XDnQ4Dr1EgefwU3CBRuAjt3sAutGg"
_VALID_CREDENTIAL_TYPE = str_to_hex("Passport")
_VALID_URI = str_to_hex("www.my-id.com/username")


class TestPermissionedDomainSet(TestCase):
def test_valid(self):
tx = PermissionedDomainSet(
account=_ACCOUNT_1,
accepted_credentials=[
Credential(credential_type=str_to_hex("Passport"), issuer=_ACCOUNT_1)
],
domain_id="BC59567FA7E4078FFC501C5B6D8E4545244146982443E58177"
"BA4DCF1AC99D6C",
)
self.assertTrue(tx.is_valid())

# tests pertaining to the length of AcceptedCredentials list
def test_accepted_credentials_too_long(self):
with self.assertRaises(XRPLModelException) as err:
PermissionedDomainSet(
account=_ACCOUNT_1,
accepted_credentials=[
Credential(
credential_type=str_to_hex("Passport_" + str(i)),
issuer=_ACCOUNT_1,
)
for i in range(11)
],
)

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

def test_accepted_credentials_empty(self):
with self.assertRaises(XRPLModelException) as err:
PermissionedDomainSet(
account=_ACCOUNT_1,
accepted_credentials=[],
)

self.assertEqual(
err.exception.args[0],
"{'PermissionedDomainSet': 'AcceptedCredentials list cannot be empty.'}",
)
Comment on lines +11 to +53
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 15, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing unit tests for PermissionedDomainDelete and domain_id validation

Based on the verification:

  • No unit tests exist for PermissionedDomainDelete (only integration tests)
  • domain_id is defined as an optional string without validation in the model
  • Current test coverage only includes a single valid domain_id case

Required test additions:

  • Create tests/unit/models/transactions/test_permissioned_domain_delete.py for unit testing the delete transaction
  • Add test cases for domain_id validation in both transaction types:
    • Invalid hex string format
    • Incorrect length
    • Missing domain_id when required
🔗 Analysis chain

Enhance test coverage for edge cases and related functionality.

The test suite could benefit from additional test cases:

  1. Invalid domain_id format validation
  2. Missing test coverage for PermissionedDomainDelete transaction as mentioned in the PR objectives

Let's check if there are any tests for PermissionedDomainDelete:

Would you like me to help generate:

  1. Additional test cases for invalid domain_id scenarios?
  2. A new test suite for PermissionedDomainDelete transaction?
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for existing PermissionedDomainDelete tests
# Expected: No matches as PR objectives mention missing tests

rg -l "PermissionedDomainDelete" tests/

Length of output: 98


Script:

#!/bin/bash
# Let's check the content of the integration test file to see if it covers PermissionedDomainDelete
rg "PermissionedDomainDelete" tests/integration/transactions/test_permissioned_domain.py -B 2 -A 5

Length of output: 665


Script:

#!/bin/bash
# Let's also check if there are any unit tests for domain_id validation
rg "domain_id" tests/unit/models/transactions/test_permissioned_domain_set.py -B 2 -A 5

Length of output: 408


Script:

#!/bin/bash
# Check the model implementation to understand domain_id validation
ast-grep --pattern 'class PermissionedDomainSet($_) {
  $$$
}'

Length of output: 64


Script:

#!/bin/bash
# Let's check for unit tests directory for PermissionedDomainDelete
fd "test_permissioned_domain_delete.py" tests/unit/

# Let's also check the model implementation to understand domain_id validation (using Python pattern)
ast-grep --pattern 'class PermissionedDomainSet:
    $$$'

Length of output: 111


Script:

#!/bin/bash
# Let's check the model file directly
rg -l "PermissionedDomainSet" xrpl/models/transactions/

Length of output: 195


Script:

#!/bin/bash
# Let's check the model implementation for domain_id validation
rg "domain_id" xrpl/models/transactions/permissioned_domain_set.py -B 2 -A 5

# Also check if PermissionedDomainDelete model exists and its implementation
rg -l "PermissionedDomainDelete" xrpl/models/transactions/

Length of output: 548

Copy link
Collaborator

Choose a reason for hiding this comment

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

^

Copy link
Contributor

Choose a reason for hiding this comment

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

@mvadari Thanks for confirming! Would you like me to help generate:

  1. The unit test suite for PermissionedDomainDelete?
  2. Additional test cases for domain_id validation in both transaction types?

🐰 thoughtfully nods

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ckeshava bump

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added unit tests in 489255e

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mvadari thanks for the reminder, missed it

27 changes: 25 additions & 2 deletions xrpl/core/binarycodec/definitions/definitions.json
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,16 @@
"type": "Hash256"
}
],
[
"DomainID",
{
"nth": 34,
"isVLEncoded": false,
"isSerialized": true,
"isSigningField": true,
"type": "Hash256"
}
],
[
"hash",
{
Expand Down Expand Up @@ -2530,6 +2540,15 @@
"type": "STArray"
}
],
[
"AcceptedCredentials", {
"nth": 28,
"isVLEncoded": false,
"isSerialized": true,
"isSigningField": true,
"type": "STArray"
}
],
[
"CloseResolution",
{
Expand Down Expand Up @@ -2863,6 +2882,7 @@
"Oracle": 128,
"Credential": 129,
"PayChannel": 120,
"PermissionedDomain": 130,
"RippleState": 114,
"SignerList": 83,
"Ticket": 84,
Expand Down Expand Up @@ -3064,8 +3084,8 @@
"CheckCash": 17,
"CheckCreate": 16,
"Clawback": 30,
"CredentialAccept": 59,
"CredentialCreate": 58,
"CredentialAccept": 59,
"CredentialDelete": 60,
"DIDDelete": 50,
"DIDSet": 49,
Expand All @@ -3085,6 +3105,7 @@
"NFTokenCancelOffer": 28,
"NFTokenCreateOffer": 27,
"NFTokenMint": 25,
"NFTokenModify": 61,
"OfferCancel": 8,
"OfferCreate": 7,
"OracleDelete": 52,
Expand All @@ -3093,6 +3114,8 @@
"PaymentChannelClaim": 15,
"PaymentChannelCreate": 13,
"PaymentChannelFund": 14,
"PermissionedDomainSet": 62,
"PermissionedDomainDelete": 63,
"SetFee": 101,
"SetRegularKey": 5,
"SignerListSet": 12,
Expand Down Expand Up @@ -3138,4 +3161,4 @@
"Vector256": 19,
"XChainBridge": 25
}
}
}
1 change: 1 addition & 0 deletions xrpl/models/requests/account_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class AccountObjectType(str, Enum):
OFFER = "offer"
ORACLE = "oracle"
PAYMENT_CHANNEL = "payment_channel"
PERMISSIONED_DOMAIN = "permissioned_domain"
SIGNER_LIST = "signer_list"
STATE = "state"
TICKET = "ticket"
Expand Down
Loading
Loading