feat: add AMMClawback support #2893
Conversation
|
Warning Rate limit exceeded@khancode has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThis pull request introduces comprehensive support for the AMMClawback amendment across several components. The changes update the server configuration, release history, and transaction type definitions, add new TypeScript models and validation logic for AMMClawback transactions, and adjust existing XRPL exports and utility mappings. Additionally, new integration and unit tests have been added to validate the AMMClawback functionality, and a function signature is updated to enable conditional clawback in pool creation. Changes
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/xrpl/src/models/transactions/AMMClawback.ts (1)
42-42: Typo in documentation comment: Missing "be" in "it cannot be reverted."In the documentation comment for the
AMMClawbackinterface, there's a grammatical error in the sentence "After you enable clawback, it cannot reverted." The word "be" is missing.Apply this diff to correct the typo:
- * After you enable clawback, it cannot reverted: the account permanently gains the ability to claw back + * After you enable clawback, it cannot be reverted: the account permanently gains the ability to claw backpackages/xrpl/test/models/AMMClawback.test.ts (3)
145-155: Fix incorrect test title.The test title suggests checking Account/Asset.issuer match, but the test actually verifies currency match between Amount and Asset.
Apply this diff to fix the test title:
- it(`throws w/ Account must be the same as Asset.issuer`, function () { + it(`throws w/ currency must be the same for Amount and Asset`, function () {
41-41: Optimize performance by avoiding the delete operator.The
deleteoperator can impact performance. Consider using undefined assignments instead.Apply this diff to optimize the code:
- delete ammClawback.Amount + ammClawback.Amount = undefined - delete ammClawback.Holder + ammClawback.Holder = undefined - delete ammClawback.Asset + ammClawback.Asset = undefined - delete ammClawback.Asset2 + ammClawback.Asset2 = undefinedAlso applies to: 47-47, 80-80, 113-113
🧰 Tools
🪛 Biome (1.9.4)
[error] 41-41: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
32-32: Improve type safety by using a more specific type assertion.The
as anytype assertion is too broad and bypasses type checking. Consider using a more specific type that matches the expected structure.Apply this diff to improve type safety:
- } as any + } as Partial<AMMClawback>packages/xrpl/HISTORY.md (1)
11-12: Deprecation Notice and New Utility Function Update
The section clearly notes the deprecation ofsetTransactionFlagsToNumberand recommends usingconvertTxFlagsToNumberinstead. Ensure that all internal documentation, API references, and migration guides are updated accordingly to avoid any confusion among users.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/ripple-binary-codec/package-lock.jsonis excluded by!**/package-lock.jsonpackages/xrpl/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.ci-config/rippled.cfg(1 hunks)packages/ripple-binary-codec/HISTORY.md(1 hunks)packages/ripple-binary-codec/src/enums/definitions.json(1 hunks)packages/xrpl/HISTORY.md(1 hunks)packages/xrpl/src/models/transactions/AMMClawback.ts(1 hunks)packages/xrpl/src/models/transactions/index.ts(1 hunks)packages/xrpl/src/models/transactions/transaction.ts(3 hunks)packages/xrpl/src/models/utils/flags.ts(2 hunks)packages/xrpl/test/integration/transactions/ammClawback.test.ts(1 hunks)packages/xrpl/test/integration/utils.ts(2 hunks)packages/xrpl/test/models/AMMClawback.test.ts(1 hunks)
🧰 Additional context used
📓 Learnings (2)
packages/xrpl/test/models/AMMClawback.test.ts (1)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
packages/xrpl/test/integration/transactions/ammClawback.test.ts (1)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
🪛 Biome (1.9.4)
packages/xrpl/test/models/AMMClawback.test.ts
[error] 41-41: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 47-47: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 80-80: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 113-113: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 GitHub Actions: Node.js CI
packages/xrpl/test/integration/transactions/ammClawback.test.ts
[error] 206-206: ValidationError: AccountSet: invalid SetFlag.
🔇 Additional comments (10)
packages/xrpl/test/integration/transactions/ammClawback.test.ts (1)
20-21:⚠️ Potential issueInvestigate potential invalid SetFlag in
createAMMPoolcausing pipeline failureThe pipeline failure indicates a
ValidationError: AccountSet: invalid SetFlag, which may originate from thecreateAMMPoolfunction when enabling clawback. Since you're passingtruetocreateAMMPoolto enable clawback, ensure that the AccountSet transaction within this function uses the correctSetFlagvalue.Run the following script to verify the
SetFlagused in theAccountSettransaction:✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Search for AccountSet transactions and display their SetFlag values. # Find all AccountSet transactions in the test directory rg -A 5 "TransactionType:\s*'AccountSet'" packages/xrpl/test/integration/ -t yaml # Alternatively, look specifically in the utils file if that's where `createAMMPool` is defined rg -A 5 "TransactionType:\s*'AccountSet'" packages/xrpl/test/integration/utils.ts # Review the SetFlag values to ensure they are correct for enabling clawbackLength of output: 351
I'll run a script to search for the definition of the
accountSetFlagsvariable so we can verify whether it holds the correct flag value when clawback is enabled.
🏁 Script executed:
#!/bin/bash # Search for the definition and usage of `accountSetFlags` in the repository rg -n "const\s\+accountSetFlags" .Length of output: 36
I'll run another script to search for the definition of
createAMMPoolin the repository (specifically in theutils.tsfile) so we can inspect how theSetFlagis assigned and whether it uses the correct value for enabling clawback.
🏁 Script executed:
#!/bin/bash # Search for the definition of createAMMPool in the utils file and display surrounding lines for context. rg -A 15 "function createAMMPool" packages/xrpl/test/integration/utils.tsLength of output: 534
I'll run a script to extract more lines from the
createAMMPoolfunction to see howaccountSetFlagsis modified in the clawback branch.
🏁 Script executed:
#!/bin/bash # Get more context from the createAMMPool function implementation rg -A 50 "export async function createAMMPool" packages/xrpl/test/integration/utils.tsLength of output: 1422
Clawback flag usage appears correct in
createAMMPoolThe inspected code shows that when
enableClawbackis true, the function setsaccountSetFlags = AccountSetAsfFlags.asfDefaultRipple | AccountSetAsfFlags.asfAllowTrustLineClawbackThis indicates that the correct flag (as defined by
AccountSetAsfFlags) is ORed in when clawback is enabled.Given that our search for the flag definition didn’t yield any conflicting value and the AccountSet transaction is constructed using these values, the evidence suggests that the
SetFlagvalue withincreateAMMPoolis implemented as expected.Actionable Item:
• Verify locally or in staging that the underlyingAccountSetAsfFlagsconstants hold the proper values per XRPL protocol specifications.packages/xrpl/src/models/transactions/index.ts (1)
17-24: LGTM!The exports for
AMMClawbackand related flags are correctly added, enhancing the transaction model's capabilities. No issues found..ci-config/rippled.cfg (1)
183-183: LGTM!The
AMMClawbackamendment is correctly added to the[features]section with other amendments from version 2.3.0.packages/xrpl/src/models/utils/flags.ts (1)
8-8: LGTM!The
AMMClawbackflags are correctly integrated into the flags handling system, following the established pattern for other transaction types.Also applies to: 51-51
packages/xrpl/src/models/transactions/transaction.ts (1)
12-12: LGTM!The
AMMClawbacktransaction type is correctly integrated into the transaction system, following the established pattern for other transaction types:
- Import of the transaction type and its validation function
- Addition to the
SubmittableTransactiontype- Addition of the validation case in the switch statement
Also applies to: 119-119, 268-270
packages/xrpl/test/integration/utils.ts (2)
376-393: LGTM! Clean implementation of clawback flag support.The addition of the
enableClawbackparameter with a default value offalseis a clean way to maintain backward compatibility while adding new functionality. The bitwise operation to combine flags is implemented correctly.
395-399: LGTM! Proper flag setting in AccountSet transaction.The
accountSetFlagsis correctly assigned to theSetFlagfield in the AccountSet transaction.packages/ripple-binary-codec/HISTORY.md (1)
5-6: LGTM! Clear and well-formatted changelog entry.The changelog entry follows the established format and properly references the XLS-73 specification.
packages/ripple-binary-codec/src/enums/definitions.json (1)
3056-3056: LGTM! Proper addition of AMMClawback transaction type.The new transaction type is correctly defined with a unique code and follows the established format.
packages/xrpl/HISTORY.md (1)
7-10: AMMClawback Amendment Entry – Verify Specification Number
The release notes now include a new entry:
* Support for the AMMClawback amendment (XLS-73)
Please confirm that the amendment number is correct. The PR objectives mention XLS-74, so this discrepancy should be resolved to ensure consistency across documentation and the implementation.
packages/xrpl/test/integration/transactions/ammClawback.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/xrpl/src/models/transactions/AMMClawback.ts (3)
35-71: Consider enhancing documentation with examples.The interface is well-defined with clear documentation. Consider adding code examples to demonstrate valid transaction structures, especially for the optional Amount field.
Example addition to the documentation:
/** * Example: * { * TransactionType: 'AMMClawback', * Account: 'rIssuerAccount...', * Holder: 'rHolderAccount...', * Asset: { * currency: 'USD', * issuer: 'rIssuerAccount...' * }, * Asset2: { * currency: 'XRP' * }, * Amount: { * currency: 'USD', * issuer: 'rIssuerAccount...', * value: '100' * } * } */
82-83: Consider using type guard for safer type casting.While the current type casting is better than using
@ts-expect-error, consider implementing a type guard function for more robust type safety.function isAMMClawback(tx: Record<string, unknown>): tx is AMMClawback { return ( tx.TransactionType === 'AMMClawback' && typeof tx.Holder === 'string' && typeof tx.Asset === 'object' ) } // Then use it like: if (!isAMMClawback(tx)) { throw new ValidationError('Invalid AMMClawback transaction') } const txAMMClawback = tx // TypeScript now knows this is AMMClawback
87-102: Consider validating Asset2 before Asset issuer checks.The current validation order might lead to runtime errors if Asset is invalid but we try to access its issuer. Consider validating Asset2 right after Asset to ensure both currency objects are valid before performing issuer comparisons.
validateRequiredField(tx, 'Asset', isCurrency) + validateRequiredField(tx, 'Asset2', isCurrency) if (txAMMClawback.Holder === txAMMClawback.Asset.issuer) { throw new ValidationError( 'AMMClawback: Holder and Asset.issuer must be distinct', ) } if (txAMMClawback.Account !== txAMMClawback.Asset.issuer) { throw new ValidationError( 'AMMClawback: Account must be the same as Asset.issuer', ) } - validateRequiredField(tx, 'Asset2', isCurrency)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/xrpl/src/models/transactions/AMMClawback.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: snippets (22.x)
- GitHub Check: snippets (20.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: integration (18.x)
- GitHub Check: browser (18.x)
🔇 Additional comments (1)
packages/xrpl/src/models/transactions/AMMClawback.ts (1)
1-34: LGTM! Well-structured imports and flag definitions.The imports are properly organized, and the flag definitions follow best practices with clear documentation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/xrpl/test/integration/utils.ts (1)
397-405: Add documentation for the clawback functionality.While the implementation is correct, please add JSDoc comments explaining:
- The purpose of the
enableAMMClawbackparameter- The impact of enabling clawback on the AMM pool
- Any security considerations when using clawback
Add this documentation above the function:
+/** + * Creates an AMM pool for testing purposes. + * + * @param client - The XRPL client + * @param enableAMMClawback - When true, enables trustline clawback for the issuer wallet. + * This allows the issuer to reclaim issued tokens through AMMClawback transactions. + * Use with caution as it grants significant control to the issuer. + * @returns An object containing the issuer wallet, LP wallet, and asset information + */ export async function createAMMPool(
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/xrpl/test/integration/transactions/ammClawback.test.ts(1 hunks)packages/xrpl/test/integration/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/xrpl/test/integration/transactions/ammClawback.test.ts
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: snippets (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: integration (22.x)
- GitHub Check: snippets (20.x)
- GitHub Check: unit (18.x)
- GitHub Check: integration (20.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (18.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: browser (18.x)
🔇 Additional comments (2)
packages/xrpl/test/integration/utils.ts (2)
376-379: LGTM! Clear and backward-compatible signature change.The optional parameter
enableAMMClawbackwith a default value offalseis well-named and maintains backward compatibility.
376-459: LGTM! Well-structured implementation with correct ordering of operations.The function maintains a clear sequence of operations and properly integrates the new clawback functionality without disrupting the existing flow.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/xrpl/test/models/AMMClawback.test.ts (2)
18-36: Consider using proper typing for the test object.Instead of using
as any, consider properly typing the test object using the AMMClawback interface to catch type-related issues early.- ammClawback = { + ammClawback: AMMClawback = { TransactionType: 'AMMClawback', Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm', Holder: 'rPyfep3gcLzkosKC9XiE77Y8DZWG6iWDT9', Asset: { currency: 'USD', issuer: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm', }, Asset2: { currency: 'XRP', }, Amount: { currency: 'USD', issuer: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm', value: '1000', }, Sequence: 1337, - } as any + }
44-44: Consider using undefined assignment instead of delete operator.Using the delete operator can impact performance. Consider using undefined assignment instead.
- delete ammClawback.Amount + ammClawback.Amount = undefined - delete ammClawback.Holder + ammClawback.Holder = undefined - delete ammClawback.Asset + ammClawback.Asset = undefined - delete ammClawback.Asset2 + ammClawback.Asset2 = undefinedAlso applies to: 56-56, 89-89, 122-122
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/xrpl/test/integration/transactions/ammClawback.test.ts(1 hunks)packages/xrpl/test/models/AMMClawback.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/xrpl/test/integration/transactions/ammClawback.test.ts
🧰 Additional context used
🧠 Learnings (1)
packages/xrpl/test/models/AMMClawback.test.ts (1)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
🪛 Biome (1.9.4)
packages/xrpl/test/models/AMMClawback.test.ts
[error] 44-44: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 56-56: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 89-89: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 122-122: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: snippets (22.x)
- GitHub Check: snippets (20.x)
- GitHub Check: integration (22.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (20.x)
- GitHub Check: unit (22.x)
- GitHub Check: integration (18.x)
- GitHub Check: unit (20.x)
- GitHub Check: unit (18.x)
- GitHub Check: browser (18.x)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/xrpl/test/models/AMMClawback.test.ts (1)
38-53: LGTM! Comprehensive positive test cases.The test cases effectively validate:
- Basic AMMClawback transaction
- Optional Amount field
- tfClawTwoAssets flag
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/xrpl/src/models/transactions/AMMClawback.ts (1)
79-90: 🛠️ Refactor suggestionMove type assertions after all validations.
The type assertions are performed before all validations are complete, which could lead to runtime errors if the fields don't exist or are of the wrong type.
export function validateAMMClawback(tx: Record<string, unknown>): void { validateBaseTransaction(tx) validateRequiredField(tx, 'Holder', isAccount) validateRequiredField(tx, 'Asset', isCurrency) + validateRequiredField(tx, 'Asset2', isCurrency) + validateOptionalField(tx, 'Amount', isAmount) // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- required const asset = tx.Asset as IssuedCurrency // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- required const amount = tx.Amount as IssuedCurrencyAmount
🧹 Nitpick comments (4)
packages/xrpl/src/models/transactions/AMMClawback.ts (2)
35-71: Consider enhancing the JSDoc comment.The main JSDoc comment could be improved by adding
@remarkstag to separate the implementation details from the main description, and by adding@throwstag to document potential validation errors./** * Claw back tokens from a holder that has deposited your issued tokens into an AMM pool. * + * @remarks * Clawback is disabled by default. To use clawback, you must send an AccountSet transaction to enable the * Allow Trust Line Clawback setting. An issuer with any existing tokens cannot enable clawback. You can * only enable Allow Trust Line Clawback if you have a completely empty owner directory, meaning you must * do so before you set up any trust lines, offers, escrows, payment channels, checks, or signer lists. * After you enable clawback, it cannot reverted: the account permanently gains the ability to claw back * issued assets on trust lines. + * + * @throws {ValidationError} When the transaction is malformed. */
107-119: Make error messages consistent.The error messages for Amount validation use slightly different wording than other messages.
if (amount.currency !== asset.currency) { throw new ValidationError( - 'AMMClawback: currency must be identical for Amount and Asset', + 'AMMClawback: Amount.currency must match Asset.currency', ) } if (amount.issuer !== asset.issuer) { throw new ValidationError( - 'AMMClawback: issuer must be identical for Amount and Asset', + 'AMMClawback: Amount.issuer must match Asset.issuer', ) }packages/xrpl/test/models/AMMClawback.test.ts (2)
18-36: Improve type safety in test setup.The test setup uses
as anytype assertion, which bypasses type checking. Consider using proper types.- ammClawback = { + ammClawback: AMMClawback = { TransactionType: 'AMMClawback', Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm', Holder: 'rPyfep3gcLzkosKC9XiE77Y8DZWG6iWDT9', Asset: { currency: 'USD', issuer: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm', }, Asset2: { currency: 'XRP', }, Amount: { currency: 'USD', issuer: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm', value: '1000', }, Sequence: 1337, - } as any + }
44-44: Improve performance by avoiding delete operator.Static analysis suggests avoiding the delete operator for better performance.
- delete ammClawback.Amount + ammClawback.Amount = undefined - delete ammClawback.Holder + ammClawback.Holder = undefined - delete ammClawback.Asset + ammClawback.Asset = undefined - delete ammClawback.Asset2 + ammClawback.Asset2 = undefinedAlso applies to: 56-56, 89-89, 122-122
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/xrpl/src/models/transactions/AMMClawback.ts(1 hunks)packages/xrpl/test/models/AMMClawback.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/xrpl/test/models/AMMClawback.test.ts (1)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
🪛 Biome (1.9.4)
packages/xrpl/test/models/AMMClawback.test.ts
[error] 44-44: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 56-56: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 89-89: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 122-122: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: unit (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: snippets (22.x)
- GitHub Check: unit (18.x)
- GitHub Check: integration (22.x)
- GitHub Check: snippets (20.x)
- GitHub Check: integration (20.x)
- GitHub Check: build-and-lint (18.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (18.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: browser (18.x)
🔇 Additional comments (2)
packages/xrpl/src/models/transactions/AMMClawback.ts (2)
1-33: LGTM!The imports are well-organized, and the flag definition follows best practices.
91-101: LGTM!The validation checks for distinct Holder and matching Account are correct and well-implemented.
High Level Overview of Change
Adds
AMMClawbacktransaction.Resources:
Fixes #2850
Type of Change
Did you update HISTORY.md?
Test Plan
Adds unit and integ tests for
AMMClawbacktransaction.