Open
Conversation
Contributor
WalkthroughThis PR implements XLS-0068 sponsorship functionality across the XRPL-py library, adding transaction types (SponsorshipSet, SponsorshipTransfer), sponsor signature models, ledger entry objects, RPC methods, base transaction validation, and comprehensive test coverage. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (21)
tests/unit/models/ledger_objects/__init__.py-1-2 (1)
1-2:⚠️ Potential issue | 🟡 MinorFix trailing blank line to pass CI.
The pipeline is failing due to a trailing blank line (W391) and Black formatting issues.
Proposed fix
"""Unit tests for ledger objects.""" -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/ledger_objects/__init__.py` around lines 1 - 2, Remove the trailing blank line after the module docstring ("""Unit tests for ledger objects.""") in the tests/unit/models/ledger_objects/__init__.py module so the file ends immediately after the closing triple quotes (single newline at EOF only) to satisfy W391/Black formatting.tests/unit/models/transactions/types/__init__.py-1-2 (1)
1-2:⚠️ Potential issue | 🟡 MinorFix trailing blank line to pass CI.
The pipeline is failing due to a trailing blank line (W391) and Black formatting issues.
Proposed fix
"""Unit tests for transaction types.""" -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/transactions/types/__init__.py` around lines 1 - 2, The file __init__.py currently has a trailing blank line causing W391 and Black failure; open tests/unit/models/transactions/types/__init__.py and remove the extra blank line so the file ends immediately after the module docstring (ensure exactly one terminating newline, no extra empty line), then reformat with Black if needed.xrpl/models/transactions/delegate_set.py-74-75 (1)
74-75:⚠️ Potential issue | 🟡 MinorFix line length to pass CI.
Line 75 exceeds the 88-character limit (89 > 88). Shorten the docstring.
Proposed fix
SPONSOR_RESERVE = "SponsorReserve" - """Delegate the ability to sponsor reserve requirements on behalf of this account.""" + """Delegate the ability to sponsor reserves on behalf of this account."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/transactions/delegate_set.py` around lines 74 - 75, Docstring for the SPONSOR_RESERVE constant exceeds the 88-char line limit; shorten it to <=88 chars. Locate the SPONSOR_RESERVE constant in delegate_set.py and replace the current docstring text with a more concise description (e.g., "Delegate ability to sponsor reserve requirements for this account.") ensuring the resulting line length is <=88 characters while preserving meaning; keep the constant name SPONSOR_RESERVE unchanged.xrpl/models/ledger_objects/__init__.py-10-11 (1)
10-11:⚠️ Potential issue | 🟡 MinorFix trailing blank line to pass CI.
The pipeline is failing due to a trailing blank line (W391) and Black formatting issues.
Proposed fix
__all__ = [ "LedgerEntryType", "Sponsorship", "SponsorshipFlag", ] -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/ledger_objects/__init__.py` around lines 10 - 11, The file ends with a trailing blank line after the closing bracket ']' which triggers W391 and Black failures; open xrpl/models/ledger_objects/__init__.py, remove the extra blank line so the file ends immediately after the ']' with a single newline (no extra empty lines), save and reformat with Black to confirm CI passes.tests/unit/models/transactions/test_granular_permission_sponsorship.py-1-124 (1)
1-124:⚠️ Potential issue | 🟡 MinorThis test file still needs formatting cleanup.
isort/Black/W391 are already failing here, so please normalize the import/layout formatting and remove the trailing blank line before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/transactions/test_granular_permission_sponsorship.py` around lines 1 - 124, Reformat the test file imports and layout to satisfy isort/Black and remove the trailing blank line at EOF: run isort on the top imports (ensure unittest.TestCase and xrpl.models.transactions imports are grouped and ordered), run Black to normalize spacing and line lengths, and delete the extra empty line at the end of the file so there is no trailing blank line; verify tests like TestGranularPermissionSponsorship and functions such as test_sponsor_fee_exists still pass after formatting.test_xls0068_combined.py-19-37 (1)
19-37:⚠️ Potential issue | 🟡 MinorTrim the unused imports or add the missing coverage.
This module currently trips F401 for most of the sponsorship-related imports, which means the "combined" file is not yet exercising most of what it pulls in. Either add the corresponding tests here or keep the imports local to the cases that actually use them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test_xls0068_combined.py` around lines 19 - 37, The file imports many sponsorship-related symbols (LedgerEntryType, Sponsorship, SponsorshipFlag, AccountSponsoring, DelegateSet, Payment, SponsorshipSet, SponsorshipTransfer, GranularPermission, Permission, SponsorSignature, SponsorSigner, SponsorshipSetFlag, SponsorshipSetFlagInterface, SponsorshipType) but most are unused and causing F401; either remove the unused imports or add minimal test/exercise code that references them (for example create/validate simple instances or call constructors for Sponsorship, SponsorshipSet, SponsorshipTransfer, SponsorSignature/SponsorSigner, DelegateSet with GranularPermission/Permission, and reference SponsorshipSetFlag/Interface and SponsorshipType) so lint sees them used—update the file by keeping only the imports actually exercised or by adding tiny usage snippets that import-related classes (e.g., instantiate or type-check) to satisfy coverage.tests/unit/models/requests/test_account_sponsoring.py-22-27 (1)
22-27:⚠️ Potential issue | 🟡 MinorFix the lint blockers in this new test file.
CI is already red here: Line 26 exceeds the repo's 88-character limit, Black wants to reflow the constructor block around Line 70, and there's a trailing blank line at EOF.
Also applies to: 68-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/requests/test_account_sponsoring.py` around lines 22 - 27, The test file has lint issues: in test_account_sponsoring_with_ledger_hash and the other block around the AccountSponsoring constructor (lines ~68-77) a single line exceeds 88 chars and Black wants the constructor reflowed, and there's a trailing blank line at EOF; fix by reformatting the AccountSponsoring instantiation so each argument is on its own line (e.g. account="...", ledger_hash="...") to keep lines <=88, apply the same change to the other constructor block, remove the extra trailing newline at end-of-file, and run Black/flake8 to verify formatting; look for symbols test_account_sponsoring_with_ledger_hash and AccountSponsoring to locate the places to change.xrpl/models/transactions/types/sponsorship_type.py-19-23 (1)
19-23:⚠️ Potential issue | 🟡 MinorReformat this enum file before merge.
This file still fails E501/BLK100/W391, so the new type will not clear the current lint gate as written.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/transactions/types/sponsorship_type.py` around lines 19 - 23, The enum file violates line-length and whitespace lint rules around the RESERVE member and its docstring; open sponsorship_type.py, ensure the enum class (and the RESERVE constant) use a properly formatted docstring or inline comment that stays under the E501 line-length limit, remove any trailing whitespace/newline to fix W391, and adjust blank lines to satisfy BLK100 (e.g., place the triple-quoted description either as the class-level docstring directly under the class definition or convert it to a short comment on the RESERVE line) so the file passes flake8/black checks.tests/unit/models/transactions/test_sponsorship_set.py-5-8 (1)
5-8:⚠️ Potential issue | 🟡 MinorDrop the unused
SponsorshipSetFlagInterfaceimport.It's tripping F401 in CI and isn't referenced anywhere in this module.
Suggested change
from xrpl.models.transactions import SponsorshipSet from xrpl.models.transactions.sponsorship_set import ( SponsorshipSetFlag, - SponsorshipSetFlagInterface, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/transactions/test_sponsorship_set.py` around lines 5 - 8, Remove the unused import symbol SponsorshipSetFlagInterface from the import statement that currently brings in SponsorshipSetFlag and SponsorshipSetFlagInterface (leave SponsorshipSetFlag only); ensure there are no remaining references to SponsorshipSetFlagInterface in the test module and run linters to confirm F401 is resolved.xrpl/models/transactions/sponsorship_transfer.py-111-112 (1)
111-112:⚠️ Potential issue | 🟡 MinorRemove trailing blank line.
File ends with a blank line (W391).
🔧 Proposed fix
return errors -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/transactions/sponsorship_transfer.py` around lines 111 - 112, The file ends with an extra trailing blank line after the final statement "return errors" in sponsorship_transfer.py; remove the blank line so the file ends immediately after the return to satisfy W391 (no trailing blank line) — locate the end of the function or module where "return errors" appears and delete the empty final line.xrpl/models/transactions/transaction.py-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorRemove unused
TYPE_CHECKINGimport.Static analysis indicates
TYPE_CHECKINGis imported but never used in this file.🔧 Proposed fix
-from typing import TYPE_CHECKING, Any, Dict, List, Optional, Type, Union, cast +from typing import Any, Dict, List, Optional, Type, Union, cast🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/transactions/transaction.py` at line 8, Remove the unused TYPE_CHECKING import from the module-level import list in transaction.py: update the typing import line that currently includes "TYPE_CHECKING" so it only imports the actually used symbols (Any, Dict, List, Optional, Type, Union, cast); ensure there are no other references to TYPE_CHECKING elsewhere in the file (e.g., in any conditional imports or type-only blocks) before committing the change.tests/unit/models/transactions/test_sponsorship_transfer.py-164-174 (1)
164-174:⚠️ Potential issue | 🟡 MinorFix formatting issues and remove trailing blank line.
- Line 165: Docstring is 90 characters (E501)
- Line 174: Trailing blank line (W391)
🔧 Proposed fix
def test_sponsorship_transfer_minimal(self): - """Test minimal SponsorshipTransfer (no sponsor, transferring back to sponsee).""" + """Test minimal SponsorshipTransfer (transferring back to sponsee).""" tx = SponsorshipTransfer( account=_ACCOUNT, object_id=_OBJECT_ID, ) self.assertTrue(tx.is_valid()) self.assertIsNone(tx.sponsor) self.assertIsNone(tx.sponsor_flags) self.assertIsNone(tx.sponsor_signature) -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/transactions/test_sponsorship_transfer.py` around lines 164 - 174, Shorten the docstring in test_sponsorship_transfer_minimal to be under the line length limit (wrap or reword the description of the minimal SponsorshipTransfer case) and remove the trailing blank line at the end of the test; update only the docstring text and delete the extra newline so the function (test_sponsorship_transfer_minimal) conform to E501 and W391 while keeping the rest of the test (SponsorshipTransfer instantiation and assertions) unchanged.tests/unit/models/transactions/test_sponsorship_transfer.py-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorSplit long constant to fix line length error.
Line 10 exceeds 88 characters (161 chars). Split the signature string.
🔧 Proposed fix
-_TXN_SIGNATURE = "3045022100D184EB4AE5956FF600E7536EE459345C7BBCF097A84CC61A93B9AF7197EDB98702201CEA8009B7BEEBAA2AACC0359B41C427C1C5B550A4CA4B80CF2174AF2D6D5DCE" +_TXN_SIGNATURE = ( + "3045022100D184EB4AE5956FF600E7536EE459345C7BBCF097A84CC61A93B9AF7197EDB987" + "02201CEA8009B7BEEBAA2AACC0359B41C427C1C5B550A4CA4B80CF2174AF2D6D5DCE" +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/transactions/test_sponsorship_transfer.py` at line 10, The _TXN_SIGNATURE constant value is too long for the line-length limit; split the string literal into multiple shorter adjacent string literals (e.g., "_TXN_SIGNATURE = "part1" "part2"") so the final concatenated value remains identical but no single source line exceeds 88 characters; update the declaration of _TXN_SIGNATURE accordingly while keeping the same symbol name and exact signature content.xrpl/models/requests/account_sponsoring.py-49-50 (1)
49-50:⚠️ Potential issue | 🟡 MinorRemove trailing blank line to fix pipeline failure.
The file has a blank line at the end (W391) and Black formatting issues (BLK100).
🔧 Proposed fix
marker: Optional[Any] = None """ Value from a previous paginated response. Resume retrieving data where that response left off. """ -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/requests/account_sponsoring.py` around lines 49 - 50, Remove the trailing blank line at the end of the module xrpl.models.requests.account_sponsoring so the file ends immediately after the closing docstring (no extra empty line), then re-run formatting/lint (Black/flake8) to ensure W391/BLK100 are resolved; locate the file by the module name and trim the final blank line so EOF is the closing triple-quote line.xrpl/models/transactions/sponsorship_transfer.py-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorRemove unused
REQUIREDimport.Static analysis indicates
REQUIREDis imported but never used in this file.🔧 Proposed fix
-from xrpl.models.required import REQUIRED🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/transactions/sponsorship_transfer.py` at line 10, Remove the unused import of REQUIRED from the top of sponsorship_transfer.py: delete the line "from xrpl.models.required import REQUIRED" (the symbol REQUIRED is not referenced anywhere in this file, e.g., in the SponsorshipTransfer model or related functions), leaving only the required/used imports.tests/unit/models/transactions/test_transaction.py-271-272 (1)
271-272:⚠️ Potential issue | 🟡 MinorFix formatting issues causing pipeline failures.
The pipeline is failing due to formatting issues:
- Line 272-273: Two blank lines instead of one (E303)
- Multiple lines exceed 88 characters (E501) - the long signature/key strings need to be wrapped or extracted to constants
Consider extracting the repeated test data to module-level constants to reduce line lengths and improve maintainability.
🔧 Suggested approach
+# Test constants for sponsor-related tests +_SPONSOR = "rsA2LpzuawewSBQXkiju3YQTMzW13pAAdW" +_SPONSOR_SIGNING_PUB_KEY = ( + "0330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD020" +) +_SPONSOR_TXN_SIGNATURE = ( + "3045022100D184EB4AE5956FF600E7536EE459345C7BBCF097A84CC61A93B9AF7197EDB987" + "02201CEA8009B7BEEBAA2AACC0359B41C427C1C5B550A4CA4B80CF2174AF2D6D5DCE" +) - - def test_transaction_with_sponsor(self):Then use these constants in all sponsor-related tests to keep lines under 88 characters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/transactions/test_transaction.py` around lines 271 - 272, Fix the formatting errors by removing the extra blank line that causes E303 and by extracting long literal strings (signature/keys used in sponsor-related tests) into module-level constants (e.g., SPONSOR_SIGNATURE, SPONSOR_PUBLIC_KEY) declared at the top of tests/unit/models/transactions/test_transaction.py, then replace the in-test long literals with those constants so no line exceeds 88 characters (E501) and all sponsor tests reference the new constants.xrpl/models/transactions/transaction.py-329-329 (1)
329-329:⚠️ Potential issue | 🟡 MinorFix line length to pass linter.
Line 329 is 89 characters (1 over the 88-character limit), causing pipeline failure.
🔧 Proposed fix
- # Validate SponsorFlags values (only tfSponsorFee and tfSponsorReserve are valid) + # Validate SponsorFlags values (tfSponsorFee and tfSponsorReserve only)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/transactions/transaction.py` at line 329, The comment containing "Validate SponsorFlags values (only tfSponsorFee and tfSponsorReserve are valid)" exceeds the 88-char line limit; shorten or split it so each line is <=88 chars. Locate the comment in transaction.py near the SponsorFlags validation (search for "SponsorFlags" or the current comment text inside the Transaction class or its validation method) and either rephrase it to a shorter single-line comment or break it into two shorter comment lines to satisfy the linter.xrpl/models/transactions/sponsorship_transfer.py-74-75 (1)
74-75:⚠️ Potential issue | 🟡 MinorFix line length issues causing pipeline failures.
Lines 75 and 98 exceed 88 characters.
🔧 Proposed fix
# Validate sponsor_flags and sponsor_signature consistency - # If sponsor field is present, sponsor_flags and sponsor_signature should be present + # If sponsor is present, sponsor_flags and sponsor_signature are required if self.sponsor is not None:And for line 98:
- # Validate sponsor_flags values (only tfSponsorFee and tfSponsorReserve are valid) + # Validate sponsor_flags values (tfSponsorFee and tfSponsorReserve only) if self.sponsor_flags is not None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/transactions/sponsorship_transfer.py` around lines 74 - 75, The two long lines in sponsorship_transfer.py exceed the 88-character limit; shorten them by wrapping or rephrasing the long comment/code so each line is <=88 chars. Locate the lines around the validation logic for sponsor_flags and sponsor_signature (e.g., in the SponsorshipTransfer class or its validate/validation block where sponsor, sponsor_flags, and sponsor_signature are referenced) and split long comment text or long expressions into multiple shorter lines or variables, ensuring identifiers like sponsor_flags and sponsor_signature remain unchanged.tests/unit/models/ledger_objects/test_sponsorship.py-112-112 (1)
112-112:⚠️ Potential issue | 🟡 MinorFix line length and trailing issues.
- Line 112: Comment is 89 characters
- Line 261: Black formatting issue
- Line 310: Trailing blank line
🔧 Proposed fix for line 112
- # Missing both fee_amount and reserve_count (reserve_count defaults to 0) + # Missing fee_amount and reserve_count (reserve_count defaults to 0)Also remove the trailing blank line at the end of the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/ledger_objects/test_sponsorship.py` at line 112, The inline comment "# Missing both fee_amount and reserve_count (reserve_count defaults to 0)" at test_sponsorship.py should be wrapped to respect the project's line length (Black default 88) — split it into two shorter comment lines (e.g., "Missing both fee_amount and reserve_count" and "reserve_count defaults to 0") near the same location; run Black/flake formatting to fix the formatting issue identified around the test at the region flagged (line ~261) and remove the trailing blank line at the end of the file so the file ends with a single newline.XLS-0068-IMPLEMENTATION-SUMMARY.md-7-8 (1)
7-8:⚠️ Potential issue | 🟡 MinorAvoid hard-coding a green status in checked-in docs.
The branch is still red in the current test job, so
COMPLETE/All 91 unit tests passingis already out of sync with the PR state and will drift again whenever the suite changes. Please either update this once CI is green or remove the live status/count from the document.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@XLS-0068-IMPLEMENTATION-SUMMARY.md` around lines 7 - 8, The checked-in document contains a hard-coded status line (`**Status**: ✅ **COMPLETE** - All 91 unit tests passing`) that can become stale; remove or replace that literal status/count with either a placeholder (e.g., remove the emoji and test count) or a note indicating the current CI status is tracked by the job, or update it only after CI is green. Locate the exact markdown line `**Status**: ✅ **COMPLETE** - All 91 unit tests passing` in XLS-0068-IMPLEMENTATION-SUMMARY.md and either delete the status/count portion or change it to a non-live statement (e.g., "See CI for current test status") so the checked-in doc won't drift.xrpl/models/transactions/sponsorship_set.py-125-153 (1)
125-153:⚠️ Potential issue | 🟡 MinorGate the self-sponsorship check behind the role-field precondition.
When both
sponsorandsponseeare omitted, the earlier branch already records the real error. The fallback derivation here then comparesself.accountto itself and adds a secondself_sponsorshiperror, which is misleading for callers.🔧 Tighten the role derivation
- sponsor_account = self.sponsee if self.sponsee is not None else self.account - sponsee_account = self.sponsor if self.sponsor is not None else self.account - - if sponsor_account == sponsee_account: - errors["self_sponsorship"] = ( - "Cannot create self-sponsorship. " - "Sponsor and Sponsee must be different accounts." - ) + if (self.sponsor is None) != (self.sponsee is None): + sponsor_account = ( + self.account if self.sponsee is not None else self.sponsor + ) + sponsee_account = ( + self.sponsee if self.sponsee is not None else self.account + ) + + if sponsor_account == sponsee_account: + errors["self_sponsorship"] = ( + "Cannot create self-sponsorship. " + "Sponsor and Sponsee must be different accounts." + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xrpl/models/transactions/sponsorship_set.py` around lines 125 - 153, The self-sponsorship comparison currently runs even when both sponsor and sponsee are omitted, producing a misleading "self_sponsorship" error; change the logic in the SponsorshipSet validation so the sponsor_account/sponsee_account derivation and the if sponsor_account == sponsee_account check only execute when at least one role field is present (i.e., only if not (self.sponsor is None and self.sponsee is None)). Update the block that computes sponsor_account and sponsee_account and the subsequent errors["self_sponsorship"] assignment to be guarded by that condition, referencing the existing symbols self.sponsor, self.sponsee, self.account and errors to locate and adjust the code.
🧹 Nitpick comments (3)
tests/unit/models/transactions/test_sponsorship_set.py (1)
80-156: These negative-path tests look too strict for this layer.Most of the cases from Line 80 onward are rippled-side flag/business-rule validation, and the substring assertions also couple the suite to internal error text. I would keep the model tests here focused on structural invariants and serialization, and let rippled enforce the rest.
Based on learnings, validation logic for transaction fields is primarily handled by rippled rather than duplicated in the Python models, flag validation checks are not performed in the models, and invalid-input tests should avoid relying on specific error messages when that adds maintenance overhead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/transactions/test_sponsorship_set.py` around lines 80 - 156, The tests from test_sponsor_field_without_delete_flag through test_invalid_max_fee_format are asserting rippled-level business rules and matching internal error substrings; update them to only test model-layer structural/serialization invariants: for SponsorshipSet and flags (SponsorshipSet, SponsorshipSetFlag) remove the substring assertions and either (a) keep only assertRaises(XRPLModelException) where the model truly should reject structurally invalid input (e.g., clearly malformed types/negative numeric types), or (b) convert rippled-rule cases (sponsor with no tfDeleteObject, delete-flag combinations, signature-flag combos, fee/max_fee/reserve_count interlocks) into positive serialization tests or drop them; specifically edit test_sponsor_field_without_delete_flag, test_delete_flag_with_fee_amount, test_delete_flag_with_max_fee, test_delete_flag_with_reserve_count, test_delete_flag_with_signature_requirement_flag, test_negative_max_fee, and test_invalid_max_fee_format to stop asserting on error message substrings and only assert exception type or remove the test if it validates rippled business logic.test_xls0068_all.py (1)
1-34: Consider relocating this file to the tests directory.The file is at the repository root (
test_xls0068_all.py) but should likely be intests/to follow project conventions and be discovered by test runners.Also, the hardcoded test counts in the docstring and
print_summary()will become stale if tests are added or removed. Consider computing counts dynamically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test_xls0068_all.py` around lines 1 - 34, Move the test_xls0068_all.py file into the tests/ directory to follow project conventions and ensure test discovery, and remove or generalize the hardcoded test counts in the module docstring; update the test runner code (e.g., the print_summary() function or the test invocation that currently prints counts) to compute counts dynamically by using unittest.TestLoader.discover or by inspecting the TestSuite/TestResult (e.g., call loader.discover('tests') or analyze the returned TestSuite to sum test.countTestCases() and derive per-phase totals from the actual loaded test cases) and then print those computed values instead of the static numbers.tests/unit/models/transactions/test_sponsor_signature.py (1)
93-95: Avoid asserting on exception wording in the invalid-path cases.These tests only need to prove construction fails. Checking substrings from
str(errors)couples them to wording and dict formatting changes without adding much signal. Based on learnings, when testing invalid inputs, avoid adding assertions on specific error messages if it increases the complexity and maintenance overhead of the tests.Also applies to: 112-145, 162-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/models/transactions/test_sponsor_signature.py` around lines 93 - 95, Remove the brittle substring assertions that inspect error.exception.args[0] and instead only assert that constructing the SponsorSignature model fails; for each failing case (the block that instantiates SponsorSignature and currently captures the context/error), keep the with self.assertRaises(ValidationError): (or switch to assertRaises if using a different exception type) around the SponsorSignature(...) call and delete the subsequent self.assertIn(...) checks — apply this change to the three failing blocks that reference the assertIn check (the current block with "must contain either txn_signature" and the other blocks in the ranges you noted).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demo_xls0068.py`:
- Around line 27-37: Replace the placeholder XRPL addresses and incomplete
sponsorship examples with valid canonical test addresses and include the missing
ledger-node fields: update the Payment(...) and SponsorSignature(...)
instantiations to use canonical test accounts (e.g., "rPT1S..."/other known test
addresses or addresses that pass xrpl.account.is_valid()) and ensure amounts and
flags are realistic; for the Sponsorship(...) examples add the required
owner_node and sponsee_node fields (use "0"/"0" for node hints in ledger-entry
demos) so the Sponsorship constructor succeeds; apply the same fixes to the
other occurrences referenced (around lines 64-70, 90-94, 112-119, 140-144,
169-173) so all samples are valid happy-path payloads.
In `@xrpl/models/__init__.py`:
- Line 8: The root package is re-exporting ledger_objects symbols via "from
xrpl.models.ledger_objects import *", which causes LedgerEntryType to collide
with the enum in xrpl.models.requests; change the import to keep the module
namespaced (e.g., replace the star-import with a module import such as "import
xrpl.models.ledger_objects as ledger_objects" or "from . import ledger_objects")
so callers must reference xrpl.models.ledger_objects.LedgerEntryType and the
package root no longer exposes ledger_objects.LedgerEntryType; apply the same
change to the other star-imports noted (lines 24-25) to avoid similar name
shadowing.
In `@xrpl/models/ledger_objects/ledger_entry_type.py`:
- Around line 6-100: The package exports list includes LedgerEntryType but it is
not imported into the package namespace; add an import for the new enum (e.g.
add "from .ledger_entry_type import LedgerEntryType") to the package __init__
where Sponsorship and SponsorshipFlag are imported so that "from
xrpl.models.ledger_objects import LedgerEntryType" works (ensure the existing
__all__ already contains "LedgerEntryType" or add it if missing).
In `@xrpl/models/transactions/sponsorship_set.py`:
- Around line 48-58: The validation in _get_errors() assumes self.flags is an
int and performs bitwise operations, which will raise TypeError when callers
pass a dict matching SponsorshipSetFlagInterface; update _get_errors() (and the
similar logic around the 158-193 block) to normalize flags up front: if
self.flags is a dict, convert the named-boolean keys
(TF_SPONSORSHIP_SET_REQUIRE_SIGN_FOR_FEE,
TF_SPONSORSHIP_CLEAR_REQUIRE_SIGN_FOR_FEE,
TF_SPONSORSHIP_SET_REQUIRE_SIGN_FOR_RESERVE,
TF_SPONSORSHIP_CLEAR_REQUIRE_SIGN_FOR_RESERVE, TF_DELETE_OBJECT) into the
equivalent integer bitmask before doing any & checks, otherwise leave ints
unchanged; ensure normalization handles missing keys as False and is applied
once at the start of the method so all subsequent bitwise checks use an int.
In `@xrpl/models/transactions/types/transaction_type.py`:
- Around line 64-65: You added SPONSORSHIP_SET and SPONSORSHIP_TRANSFER enum
values but didn’t register them in the binary codec mapping used at runtime;
update the _DEFINITIONS["TRANSACTION_TYPES"] mapping to include entries for
"SponsorshipSet" and "SponsorshipTransfer" (matching the SPONSORSHIP_SET and
SPONSORSHIP_TRANSFER symbols) and provide their binary codec definitions/schema
names so the codec can resolve them during serialization/signing; also ensure
any referenced field schemas (transaction field definitions) used by those
entries are present in the definitions collection so no KeyError occurs.
---
Minor comments:
In `@test_xls0068_combined.py`:
- Around line 19-37: The file imports many sponsorship-related symbols
(LedgerEntryType, Sponsorship, SponsorshipFlag, AccountSponsoring, DelegateSet,
Payment, SponsorshipSet, SponsorshipTransfer, GranularPermission, Permission,
SponsorSignature, SponsorSigner, SponsorshipSetFlag,
SponsorshipSetFlagInterface, SponsorshipType) but most are unused and causing
F401; either remove the unused imports or add minimal test/exercise code that
references them (for example create/validate simple instances or call
constructors for Sponsorship, SponsorshipSet, SponsorshipTransfer,
SponsorSignature/SponsorSigner, DelegateSet with GranularPermission/Permission,
and reference SponsorshipSetFlag/Interface and SponsorshipType) so lint sees
them used—update the file by keeping only the imports actually exercised or by
adding tiny usage snippets that import-related classes (e.g., instantiate or
type-check) to satisfy coverage.
In `@tests/unit/models/ledger_objects/__init__.py`:
- Around line 1-2: Remove the trailing blank line after the module docstring
("""Unit tests for ledger objects.""") in the
tests/unit/models/ledger_objects/__init__.py module so the file ends immediately
after the closing triple quotes (single newline at EOF only) to satisfy
W391/Black formatting.
In `@tests/unit/models/ledger_objects/test_sponsorship.py`:
- Line 112: The inline comment "# Missing both fee_amount and reserve_count
(reserve_count defaults to 0)" at test_sponsorship.py should be wrapped to
respect the project's line length (Black default 88) — split it into two shorter
comment lines (e.g., "Missing both fee_amount and reserve_count" and
"reserve_count defaults to 0") near the same location; run Black/flake
formatting to fix the formatting issue identified around the test at the region
flagged (line ~261) and remove the trailing blank line at the end of the file so
the file ends with a single newline.
In `@tests/unit/models/requests/test_account_sponsoring.py`:
- Around line 22-27: The test file has lint issues: in
test_account_sponsoring_with_ledger_hash and the other block around the
AccountSponsoring constructor (lines ~68-77) a single line exceeds 88 chars and
Black wants the constructor reflowed, and there's a trailing blank line at EOF;
fix by reformatting the AccountSponsoring instantiation so each argument is on
its own line (e.g. account="...", ledger_hash="...") to keep lines <=88, apply
the same change to the other constructor block, remove the extra trailing
newline at end-of-file, and run Black/flake8 to verify formatting; look for
symbols test_account_sponsoring_with_ledger_hash and AccountSponsoring to locate
the places to change.
In `@tests/unit/models/transactions/test_granular_permission_sponsorship.py`:
- Around line 1-124: Reformat the test file imports and layout to satisfy
isort/Black and remove the trailing blank line at EOF: run isort on the top
imports (ensure unittest.TestCase and xrpl.models.transactions imports are
grouped and ordered), run Black to normalize spacing and line lengths, and
delete the extra empty line at the end of the file so there is no trailing blank
line; verify tests like TestGranularPermissionSponsorship and functions such as
test_sponsor_fee_exists still pass after formatting.
In `@tests/unit/models/transactions/test_sponsorship_set.py`:
- Around line 5-8: Remove the unused import symbol SponsorshipSetFlagInterface
from the import statement that currently brings in SponsorshipSetFlag and
SponsorshipSetFlagInterface (leave SponsorshipSetFlag only); ensure there are no
remaining references to SponsorshipSetFlagInterface in the test module and run
linters to confirm F401 is resolved.
In `@tests/unit/models/transactions/test_sponsorship_transfer.py`:
- Around line 164-174: Shorten the docstring in
test_sponsorship_transfer_minimal to be under the line length limit (wrap or
reword the description of the minimal SponsorshipTransfer case) and remove the
trailing blank line at the end of the test; update only the docstring text and
delete the extra newline so the function (test_sponsorship_transfer_minimal)
conform to E501 and W391 while keeping the rest of the test (SponsorshipTransfer
instantiation and assertions) unchanged.
- Line 10: The _TXN_SIGNATURE constant value is too long for the line-length
limit; split the string literal into multiple shorter adjacent string literals
(e.g., "_TXN_SIGNATURE = "part1" "part2"") so the final concatenated value
remains identical but no single source line exceeds 88 characters; update the
declaration of _TXN_SIGNATURE accordingly while keeping the same symbol name and
exact signature content.
In `@tests/unit/models/transactions/test_transaction.py`:
- Around line 271-272: Fix the formatting errors by removing the extra blank
line that causes E303 and by extracting long literal strings (signature/keys
used in sponsor-related tests) into module-level constants (e.g.,
SPONSOR_SIGNATURE, SPONSOR_PUBLIC_KEY) declared at the top of
tests/unit/models/transactions/test_transaction.py, then replace the in-test
long literals with those constants so no line exceeds 88 characters (E501) and
all sponsor tests reference the new constants.
In `@tests/unit/models/transactions/types/__init__.py`:
- Around line 1-2: The file __init__.py currently has a trailing blank line
causing W391 and Black failure; open
tests/unit/models/transactions/types/__init__.py and remove the extra blank line
so the file ends immediately after the module docstring (ensure exactly one
terminating newline, no extra empty line), then reformat with Black if needed.
In `@XLS-0068-IMPLEMENTATION-SUMMARY.md`:
- Around line 7-8: The checked-in document contains a hard-coded status line
(`**Status**: ✅ **COMPLETE** - All 91 unit tests passing`) that can become
stale; remove or replace that literal status/count with either a placeholder
(e.g., remove the emoji and test count) or a note indicating the current CI
status is tracked by the job, or update it only after CI is green. Locate the
exact markdown line `**Status**: ✅ **COMPLETE** - All 91 unit tests passing` in
XLS-0068-IMPLEMENTATION-SUMMARY.md and either delete the status/count portion or
change it to a non-live statement (e.g., "See CI for current test status") so
the checked-in doc won't drift.
In `@xrpl/models/ledger_objects/__init__.py`:
- Around line 10-11: The file ends with a trailing blank line after the closing
bracket ']' which triggers W391 and Black failures; open
xrpl/models/ledger_objects/__init__.py, remove the extra blank line so the file
ends immediately after the ']' with a single newline (no extra empty lines),
save and reformat with Black to confirm CI passes.
In `@xrpl/models/requests/account_sponsoring.py`:
- Around line 49-50: Remove the trailing blank line at the end of the module
xrpl.models.requests.account_sponsoring so the file ends immediately after the
closing docstring (no extra empty line), then re-run formatting/lint
(Black/flake8) to ensure W391/BLK100 are resolved; locate the file by the module
name and trim the final blank line so EOF is the closing triple-quote line.
In `@xrpl/models/transactions/delegate_set.py`:
- Around line 74-75: Docstring for the SPONSOR_RESERVE constant exceeds the
88-char line limit; shorten it to <=88 chars. Locate the SPONSOR_RESERVE
constant in delegate_set.py and replace the current docstring text with a more
concise description (e.g., "Delegate ability to sponsor reserve requirements for
this account.") ensuring the resulting line length is <=88 characters while
preserving meaning; keep the constant name SPONSOR_RESERVE unchanged.
In `@xrpl/models/transactions/sponsorship_set.py`:
- Around line 125-153: The self-sponsorship comparison currently runs even when
both sponsor and sponsee are omitted, producing a misleading "self_sponsorship"
error; change the logic in the SponsorshipSet validation so the
sponsor_account/sponsee_account derivation and the if sponsor_account ==
sponsee_account check only execute when at least one role field is present
(i.e., only if not (self.sponsor is None and self.sponsee is None)). Update the
block that computes sponsor_account and sponsee_account and the subsequent
errors["self_sponsorship"] assignment to be guarded by that condition,
referencing the existing symbols self.sponsor, self.sponsee, self.account and
errors to locate and adjust the code.
In `@xrpl/models/transactions/sponsorship_transfer.py`:
- Around line 111-112: The file ends with an extra trailing blank line after the
final statement "return errors" in sponsorship_transfer.py; remove the blank
line so the file ends immediately after the return to satisfy W391 (no trailing
blank line) — locate the end of the function or module where "return errors"
appears and delete the empty final line.
- Line 10: Remove the unused import of REQUIRED from the top of
sponsorship_transfer.py: delete the line "from xrpl.models.required import
REQUIRED" (the symbol REQUIRED is not referenced anywhere in this file, e.g., in
the SponsorshipTransfer model or related functions), leaving only the
required/used imports.
- Around line 74-75: The two long lines in sponsorship_transfer.py exceed the
88-character limit; shorten them by wrapping or rephrasing the long comment/code
so each line is <=88 chars. Locate the lines around the validation logic for
sponsor_flags and sponsor_signature (e.g., in the SponsorshipTransfer class or
its validate/validation block where sponsor, sponsor_flags, and
sponsor_signature are referenced) and split long comment text or long
expressions into multiple shorter lines or variables, ensuring identifiers like
sponsor_flags and sponsor_signature remain unchanged.
In `@xrpl/models/transactions/transaction.py`:
- Line 8: Remove the unused TYPE_CHECKING import from the module-level import
list in transaction.py: update the typing import line that currently includes
"TYPE_CHECKING" so it only imports the actually used symbols (Any, Dict, List,
Optional, Type, Union, cast); ensure there are no other references to
TYPE_CHECKING elsewhere in the file (e.g., in any conditional imports or
type-only blocks) before committing the change.
- Line 329: The comment containing "Validate SponsorFlags values (only
tfSponsorFee and tfSponsorReserve are valid)" exceeds the 88-char line limit;
shorten or split it so each line is <=88 chars. Locate the comment in
transaction.py near the SponsorFlags validation (search for "SponsorFlags" or
the current comment text inside the Transaction class or its validation method)
and either rephrase it to a shorter single-line comment or break it into two
shorter comment lines to satisfy the linter.
In `@xrpl/models/transactions/types/sponsorship_type.py`:
- Around line 19-23: The enum file violates line-length and whitespace lint
rules around the RESERVE member and its docstring; open sponsorship_type.py,
ensure the enum class (and the RESERVE constant) use a properly formatted
docstring or inline comment that stays under the E501 line-length limit, remove
any trailing whitespace/newline to fix W391, and adjust blank lines to satisfy
BLK100 (e.g., place the triple-quoted description either as the class-level
docstring directly under the class definition or convert it to a short comment
on the RESERVE line) so the file passes flake8/black checks.
---
Nitpick comments:
In `@test_xls0068_all.py`:
- Around line 1-34: Move the test_xls0068_all.py file into the tests/ directory
to follow project conventions and ensure test discovery, and remove or
generalize the hardcoded test counts in the module docstring; update the test
runner code (e.g., the print_summary() function or the test invocation that
currently prints counts) to compute counts dynamically by using
unittest.TestLoader.discover or by inspecting the TestSuite/TestResult (e.g.,
call loader.discover('tests') or analyze the returned TestSuite to sum
test.countTestCases() and derive per-phase totals from the actual loaded test
cases) and then print those computed values instead of the static numbers.
In `@tests/unit/models/transactions/test_sponsor_signature.py`:
- Around line 93-95: Remove the brittle substring assertions that inspect
error.exception.args[0] and instead only assert that constructing the
SponsorSignature model fails; for each failing case (the block that instantiates
SponsorSignature and currently captures the context/error), keep the with
self.assertRaises(ValidationError): (or switch to assertRaises if using a
different exception type) around the SponsorSignature(...) call and delete the
subsequent self.assertIn(...) checks — apply this change to the three failing
blocks that reference the assertIn check (the current block with "must contain
either txn_signature" and the other blocks in the ranges you noted).
In `@tests/unit/models/transactions/test_sponsorship_set.py`:
- Around line 80-156: The tests from test_sponsor_field_without_delete_flag
through test_invalid_max_fee_format are asserting rippled-level business rules
and matching internal error substrings; update them to only test model-layer
structural/serialization invariants: for SponsorshipSet and flags
(SponsorshipSet, SponsorshipSetFlag) remove the substring assertions and either
(a) keep only assertRaises(XRPLModelException) where the model truly should
reject structurally invalid input (e.g., clearly malformed types/negative
numeric types), or (b) convert rippled-rule cases (sponsor with no
tfDeleteObject, delete-flag combinations, signature-flag combos,
fee/max_fee/reserve_count interlocks) into positive serialization tests or drop
them; specifically edit test_sponsor_field_without_delete_flag,
test_delete_flag_with_fee_amount, test_delete_flag_with_max_fee,
test_delete_flag_with_reserve_count,
test_delete_flag_with_signature_requirement_flag, test_negative_max_fee, and
test_invalid_max_fee_format to stop asserting on error message substrings and
only assert exception type or remove the test if it validates rippled business
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fbd18388-6a18-4514-a5e1-dc543babbd3c
📒 Files selected for processing (31)
XLS-0068-IMPLEMENTATION-SUMMARY.mddemo_xls0068.pytest_xls0068_all.pytest_xls0068_combined.pytests/unit/models/ledger_objects/__init__.pytests/unit/models/ledger_objects/test_sponsorship.pytests/unit/models/requests/test_account_sponsoring.pytests/unit/models/transactions/test_granular_permission_sponsorship.pytests/unit/models/transactions/test_sponsor_signature.pytests/unit/models/transactions/test_sponsorship_set.pytests/unit/models/transactions/test_sponsorship_transfer.pytests/unit/models/transactions/test_transaction.pytests/unit/models/transactions/types/__init__.pytests/unit/models/transactions/types/test_sponsorship_type.pyxrpl/models/__init__.pyxrpl/models/ledger_objects/__init__.pyxrpl/models/ledger_objects/ledger_entry_type.pyxrpl/models/ledger_objects/sponsorship.pyxrpl/models/requests/__init__.pyxrpl/models/requests/account_objects.pyxrpl/models/requests/account_sponsoring.pyxrpl/models/requests/request.pyxrpl/models/transactions/__init__.pyxrpl/models/transactions/delegate_set.pyxrpl/models/transactions/sponsor_signature.pyxrpl/models/transactions/sponsorship_set.pyxrpl/models/transactions/sponsorship_transfer.pyxrpl/models/transactions/transaction.pyxrpl/models/transactions/types/__init__.pyxrpl/models/transactions/types/sponsorship_type.pyxrpl/models/transactions/types/transaction_type.py
Comment on lines
+27
to
+37
| payment = Payment( | ||
| account="rSender1234567890123456789012345", | ||
| destination="rReceiver123456789012345678901", | ||
| amount="1000000", # 1 XRP | ||
| sponsor="rSponsor123456789012345678901234", | ||
| sponsor_flags=0x00000001, # tfSponsorFee | ||
| sponsor_signature=SponsorSignature( | ||
| signing_pub_key="0330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD020", | ||
| txn_signature="3045022100D184EB4AE5956FF600E7536EE459345C7BBCF097A84CC61A93B9AF7197EDB98702201CEA8009B7BEEBAA2AACC0359B41C427C1C5B550A4CA4B80CF2174AF2D6D5DCE", | ||
| ), | ||
| ) |
Contributor
There was a problem hiding this comment.
Use valid sample payloads here so the demo actually shows happy-path usage.
Most of these placeholder account strings contain 0, so they are not valid XRPL classic addresses and cannot serve as successful is_valid() examples. The Sponsorship(...) sample also omits the required owner_node and sponsee_node fields from xrpl/models/ledger_objects/sponsorship.py, so that constructor is guaranteed to fail. Please switch these examples to canonical test addresses and provide node hints such as "0"/"0" for the ledger-entry demo.
Also applies to: 64-70, 90-94, 112-119, 140-144, 169-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@demo_xls0068.py` around lines 27 - 37, Replace the placeholder XRPL addresses
and incomplete sponsorship examples with valid canonical test addresses and
include the missing ledger-node fields: update the Payment(...) and
SponsorSignature(...) instantiations to use canonical test accounts (e.g.,
"rPT1S..."/other known test addresses or addresses that pass
xrpl.account.is_valid()) and ensure amounts and flags are realistic; for the
Sponsorship(...) examples add the required owner_node and sponsee_node fields
(use "0"/"0" for node hints in ledger-entry demos) so the Sponsorship
constructor succeeds; apply the same fixes to the other occurrences referenced
(around lines 64-70, 90-94, 112-119, 140-144, 169-173) so all samples are valid
happy-path payloads.
| from xrpl.models.auth_account import AuthAccount | ||
| from xrpl.models.currencies import * # noqa: F401, F403 | ||
| from xrpl.models.exceptions import XRPLModelException | ||
| from xrpl.models.ledger_objects import * # noqa: F401, F403 |
Contributor
There was a problem hiding this comment.
Avoid re-exporting ledger_objects.LedgerEntryType from the package root.
xrpl.models.requests already has a different LedgerEntryType, so star-importing ledger_objects makes from xrpl.models import LedgerEntryType ambiguous and one enum ends up shadowing the other. I would expose the ledger_objects module itself here, but keep the colliding enum names namespaced under xrpl.models.ledger_objects.
Suggested change
from xrpl.models.exceptions import XRPLModelException
-from xrpl.models.ledger_objects import * # noqa: F401, F403
+from xrpl.models.ledger_objects import Sponsorship, SponsorshipFlag # noqa: F401
from xrpl.models.mptoken_metadata import MPTokenMetadata, MPTokenMetadataUri
from xrpl.models.path import Path, PathStep
from xrpl.models.requests import * # noqa: F401, F403
@@
*currencies.__all__,
"ledger_objects",
- *ledger_objects.__all__,
+ "Sponsorship",
+ "SponsorshipFlag",
"MPTokenMetadata",Also applies to: 24-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/models/__init__.py` at line 8, The root package is re-exporting
ledger_objects symbols via "from xrpl.models.ledger_objects import *", which
causes LedgerEntryType to collide with the enum in xrpl.models.requests; change
the import to keep the module namespaced (e.g., replace the star-import with a
module import such as "import xrpl.models.ledger_objects as ledger_objects" or
"from . import ledger_objects") so callers must reference
xrpl.models.ledger_objects.LedgerEntryType and the package root no longer
exposes ledger_objects.LedgerEntryType; apply the same change to the other
star-imports noted (lines 24-25) to avoid similar name shadowing.
Comment on lines
+6
to
+100
| class LedgerEntryType(str, Enum): | ||
| """Enum representing ledger entry types on the XRP Ledger.""" | ||
|
|
||
| ACCOUNT_ROOT = "AccountRoot" | ||
| """An AccountRoot ledger entry describes a single account.""" | ||
|
|
||
| AMENDMENTS = "Amendments" | ||
| """The Amendments ledger entry tracks the status of amendments.""" | ||
|
|
||
| AMM = "AMM" | ||
| """An AMM ledger entry describes an Automated Market Maker instance.""" | ||
|
|
||
| BRIDGE = "Bridge" | ||
| """A Bridge ledger entry describes a cross-chain bridge.""" | ||
|
|
||
| CHECK = "Check" | ||
| """A Check ledger entry describes a check.""" | ||
|
|
||
| CREDENTIAL = "Credential" | ||
| """A Credential ledger entry describes a credential.""" | ||
|
|
||
| DELEGATE = "Delegate" | ||
| """A Delegate ledger entry describes delegation permissions.""" | ||
|
|
||
| DEPOSIT_PREAUTH = "DepositPreauth" | ||
| """A DepositPreauth ledger entry tracks preauthorization for payments.""" | ||
|
|
||
| DIRECTORY_NODE = "DirectoryNode" | ||
| """A DirectoryNode ledger entry represents a page in a directory.""" | ||
|
|
||
| DID = "DID" | ||
| """A DID ledger entry describes a decentralized identifier.""" | ||
|
|
||
| ESCROW = "Escrow" | ||
| """An Escrow ledger entry describes an escrow.""" | ||
|
|
||
| FEE_SETTINGS = "FeeSettings" | ||
| """The FeeSettings ledger entry contains the current base transaction cost and reserve amounts.""" | ||
|
|
||
| LEDGER_HASHES = "LedgerHashes" | ||
| """A LedgerHashes ledger entry contains hashes of previous ledgers.""" | ||
|
|
||
| LOAN = "Loan" | ||
| """A Loan ledger entry describes a loan.""" | ||
|
|
||
| LOAN_BROKER = "LoanBroker" | ||
| """A LoanBroker ledger entry describes a loan broker.""" | ||
|
|
||
| MPTOKEN = "MPToken" | ||
| """An MPToken ledger entry describes a multi-purpose token.""" | ||
|
|
||
| MPTOKEN_ISSUANCE = "MPTokenIssuance" | ||
| """An MPTokenIssuance ledger entry describes a multi-purpose token issuance.""" | ||
|
|
||
| NEGATIVE_UNL = "NegativeUNL" | ||
| """The NegativeUNL ledger entry contains the current negative UNL.""" | ||
|
|
||
| NFTOKEN_OFFER = "NFTokenOffer" | ||
| """An NFTokenOffer ledger entry describes an offer to buy or sell an NFToken.""" | ||
|
|
||
| NFTOKEN_PAGE = "NFTokenPage" | ||
| """An NFTokenPage ledger entry contains a collection of NFTokens owned by the same account.""" | ||
|
|
||
| OFFER = "Offer" | ||
| """An Offer ledger entry describes an offer to exchange currencies.""" | ||
|
|
||
| ORACLE = "Oracle" | ||
| """An Oracle ledger entry describes a price oracle.""" | ||
|
|
||
| PAY_CHANNEL = "PayChannel" | ||
| """A PayChannel ledger entry describes a payment channel.""" | ||
|
|
||
| PERMISSIONED_DOMAIN = "PermissionedDomain" | ||
| """A PermissionedDomain ledger entry describes a permissioned domain.""" | ||
|
|
||
| RIPPLE_STATE = "RippleState" | ||
| """A RippleState ledger entry describes a trust line between two accounts.""" | ||
|
|
||
| SIGNER_LIST = "SignerList" | ||
| """A SignerList ledger entry describes a list of signers for multi-signing.""" | ||
|
|
||
| SPONSORSHIP = "Sponsorship" | ||
| """A Sponsorship ledger entry describes a sponsorship relationship between two accounts.""" | ||
|
|
||
| TICKET = "Ticket" | ||
| """A Ticket ledger entry represents a sequence number set aside for future use.""" | ||
|
|
||
| VAULT = "Vault" | ||
| """A Vault ledger entry describes a vault.""" | ||
|
|
||
| XCHAIN_OWNED_CLAIM_ID = "XChainOwnedClaimID" | ||
| """An XChainOwnedClaimID ledger entry represents a cross-chain claim ID.""" | ||
|
|
||
| XCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID = "XChainOwnedCreateAccountClaimID" | ||
| """An XChainOwnedCreateAccountClaimID ledger entry represents a cross-chain create account claim ID.""" |
Contributor
There was a problem hiding this comment.
Re-export the new enum from xrpl.models.ledger_objects.
The package snippet for xrpl/models/ledger_objects/__init__.py already lists LedgerEntryType in __all__, but it only imports Sponsorship and SponsorshipFlag. That means from xrpl.models.ledger_objects import LedgerEntryType still fails even though this PR presents it as public API.
🔧 Follow-up needed in xrpl/models/ledger_objects/__init__.py
+from xrpl.models.ledger_objects.ledger_entry_type import LedgerEntryType
from xrpl.models.ledger_objects.sponsorship import Sponsorship, SponsorshipFlag🧰 Tools
🪛 GitHub Actions: Unit test
[error] 43-43: E501 line too long (102 > 88 characters)
[error] 67-67: E501 line too long (98 > 88 characters)
[error] 88-88: E501 line too long (95 > 88 characters)
[error] 100-100: E501 line too long (107 > 88 characters)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/models/ledger_objects/ledger_entry_type.py` around lines 6 - 100, The
package exports list includes LedgerEntryType but it is not imported into the
package namespace; add an import for the new enum (e.g. add "from
.ledger_entry_type import LedgerEntryType") to the package __init__ where
Sponsorship and SponsorshipFlag are imported so that "from
xrpl.models.ledger_objects import LedgerEntryType" works (ensure the existing
__all__ already contains "LedgerEntryType" or add it if missing).
Comment on lines
+48
to
+58
| class SponsorshipSetFlagInterface(TransactionFlagInterface): | ||
| """ | ||
| Transactions of the SponsorshipSet type support additional values in the Flags field. | ||
| This TypedDict represents those options. | ||
| """ | ||
|
|
||
| TF_SPONSORSHIP_SET_REQUIRE_SIGN_FOR_FEE: bool | ||
| TF_SPONSORSHIP_CLEAR_REQUIRE_SIGN_FOR_FEE: bool | ||
| TF_SPONSORSHIP_SET_REQUIRE_SIGN_FOR_RESERVE: bool | ||
| TF_SPONSORSHIP_CLEAR_REQUIRE_SIGN_FOR_RESERVE: bool | ||
| TF_DELETE_OBJECT: bool |
Contributor
There was a problem hiding this comment.
Normalize flags before the bitwise checks.
SponsorshipSetFlagInterface advertises the named-bool form, but _get_errors() assumes self.flags is always an int and uses & on it. Callers who pass the supported dict-shaped flags will hit a TypeError during validation.
🔧 One way to support both flag representations
`@require_kwargs_on_init`
`@dataclass`(frozen=True, **KW_ONLY_DATACLASS)
class SponsorshipSet(Transaction):
@@
+ def _has_flag(self, flag: SponsorshipSetFlag) -> bool:
+ if self.flags is None:
+ return False
+ if isinstance(self.flags, int):
+ return bool(self.flags & flag)
+ return bool(self.flags.get(flag.name, False))
+
def _get_errors(self: Self) -> Dict[str, str]:
@@
- if self.sponsor is not None:
- if self.flags is None or not (self.flags & SponsorshipSetFlag.TF_DELETE_OBJECT):
+ if self.sponsor is not None:
+ if not self._has_flag(SponsorshipSetFlag.TF_DELETE_OBJECT):
errors["sponsor_field"] = (
@@
- if self.flags is not None and (self.flags & SponsorshipSetFlag.TF_DELETE_OBJECT):
+ if self._has_flag(SponsorshipSetFlag.TF_DELETE_OBJECT):
@@
for flag in conflicting_flags:
- if self.flags & flag:
+ if self._has_flag(flag):
errors["invalid_flag_with_delete"] = (Also applies to: 158-193
🧰 Tools
🪛 GitHub Actions: Unit test
[error] 50-50: E501 line too long (89 > 88 characters)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/models/transactions/sponsorship_set.py` around lines 48 - 58, The
validation in _get_errors() assumes self.flags is an int and performs bitwise
operations, which will raise TypeError when callers pass a dict matching
SponsorshipSetFlagInterface; update _get_errors() (and the similar logic around
the 158-193 block) to normalize flags up front: if self.flags is a dict, convert
the named-boolean keys (TF_SPONSORSHIP_SET_REQUIRE_SIGN_FOR_FEE,
TF_SPONSORSHIP_CLEAR_REQUIRE_SIGN_FOR_FEE,
TF_SPONSORSHIP_SET_REQUIRE_SIGN_FOR_RESERVE,
TF_SPONSORSHIP_CLEAR_REQUIRE_SIGN_FOR_RESERVE, TF_DELETE_OBJECT) into the
equivalent integer bitmask before doing any & checks, otherwise leave ints
unchanged; ensure normalization handles missing keys as False and is applied
once at the start of the method so all subsequent bitwise checks use an int.
Comment on lines
+64
to
+65
| SPONSORSHIP_SET = "SponsorshipSet" | ||
| SPONSORSHIP_TRANSFER = "SponsorshipTransfer" |
Contributor
There was a problem hiding this comment.
Register these transaction types in the binary codec definitions too.
Adding the enum values here is not sufficient. xrpl/core/binarycodec/definitions/definitions.py:149-164 resolves transaction names through _DEFINITIONS["TRANSACTION_TYPES"]; without matching definitions entries, serializing or signing either transaction will raise KeyError.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/models/transactions/types/transaction_type.py` around lines 64 - 65, You
added SPONSORSHIP_SET and SPONSORSHIP_TRANSFER enum values but didn’t register
them in the binary codec mapping used at runtime; update the
_DEFINITIONS["TRANSACTION_TYPES"] mapping to include entries for
"SponsorshipSet" and "SponsorshipTransfer" (matching the SPONSORSHIP_SET and
SPONSORSHIP_TRANSFER symbols) and provide their binary codec definitions/schema
names so the codec can resolve them during serialization/signing; also ensure
any referenced field schemas (transaction field definitions) used by those
entries are present in the definitions collection so no KeyError occurs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
Context of Change
Type of Change
Did you update CHANGELOG.md?
Test Plan