Sponsorship xls 68#3238
Open
cybele-ripple wants to merge 13 commits intomainfrom
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces comprehensive XLS-68 sponsorship support across the xrpl.js library. Changes include new binary codec field definitions, transaction types ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/xrpl/src/sugar/getOrderbook.ts (1)
15-22:⚠️ Potential issue | 🟡 Minor
BigNumber.comparedTo()returnsnullfor non-numeric values without explicit handling.While lines 17–18 default nullish
qualityto0, a non-numeric string (e.g., from a malformed server response) producesNaNinBigNumber, causingcomparedTo()to returnnull. JavaScript coercesnullto0inArray.sort(), so sorting behavior is functionally unchanged. However, explicitly handling this edge case with?? 0makes the code more defensive and clearer about intent. If the API contract guarantees valid numeric strings, consider adding that assumption as a comment; otherwise, restore the fallback or validate quality at the response parsing level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/xrpl/src/sugar/getOrderbook.ts` around lines 15 - 22, sortOffers currently constructs BigNumber from possibly non-numeric quality values so comparedTo can return null; update sortOffers to defensively coerce/validate qualities before comparing: inside sortOffers (function name) convert qualityA/qualityB to BigNumber instances, check .isNaN() on each and replace NaN instances with BigNumber(0), then call comparedTo on the validated BigNumber instances and return that result; reference the variables qualityA, qualityB and the function sortOffers when making the change.
🧹 Nitpick comments (3)
packages/xrpl/src/sugar/validateSponsorship.ts (1)
88-110: Consider requiring FeeAmount when validating fee sponsorship.When
tfSponsorFeeis set (line 85), the validation checkssponsorship.FeeAmountonly if it exists (line 98). However, ifFeeAmountis undefined or missing on a sponsorship that's supposed to cover fees, this could silently pass validation and fail at submission time.Consider adding explicit validation:
💡 Suggested enhancement
if (!isSponsoringFee) { // Only reserve sponsorship, no fee validation needed return { valid: true, sponsorship, estimatedFee: fee, } } + // Fee sponsorship requires FeeAmount to be set + if (!sponsorship.FeeAmount) { + return { + valid: false, + error: 'Sponsorship does not have FeeAmount set for fee sponsorship', + sponsorship, + estimatedFee: fee, + } + } + // Validate FeeAmount has sufficient balance - if (sponsorship.FeeAmount) { const feeAmount = new BigNumber(sponsorship.FeeAmount)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/xrpl/src/sugar/validateSponsorship.ts` around lines 88 - 110, When tfSponsorFee/isSponsoringFee is true, ensure FeeAmount is required: update validateSponsorship (around isSponsoringFee and the sponsorship.FeeAmount check) to return valid:false with a clear error if sponsorship.FeeAmount is undefined or null (instead of skipping validation), and keep returning sponsorship and estimatedFee in that error response; otherwise continue to parse FeeAmount and compare it to fee as currently implemented.packages/xrpl/test/integration/transactions/sponsorship.test.ts (1)
776-781: Consider using enum values for combined flags.Similar to the unit test, the magic number
3could be replaced with explicit enum combination for better readability.♻️ Suggested improvement
Sponsor: sponsorWallet.classicAddress, - // SponsorFlags: fee (1) + reserve (2) = 3 - SponsorFlags: 3, + /* eslint-disable no-bitwise -- combining flags */ + SponsorFlags: SponsorFlags.tfSponsorFee | SponsorFlags.tfSponsorReserve, + /* eslint-enable no-bitwise */ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/xrpl/test/integration/transactions/sponsorship.test.ts` around lines 776 - 781, Replace the magic literal 3 assigned to SponsorFlags with the explicit enum combination of the two sponsorship flag values; update the object so SponsorFlags is set using the bitwise OR of the enum members (e.g., SponsorFlag.FEE | SponsorFlag.RESERVE) instead of the number 3 so the intent is clear when creating the transaction that includes sponsorWallet.classicAddress.packages/xrpl/test/wallet/sponsorSigner.test.ts (1)
316-334: Consider using enum values instead of magic number for combined flags.The combined flags value
3is correct but less readable than using the enum values explicitly.♻️ Suggested improvement for readability
const sponsorAddress = 'rBJMcbqnAaxcUeEPF7WiaoHCtFiTmga7un' - // SponsorFlags.tfSponsorFee (1) + SponsorFlags.tfSponsorReserve (2) = 3 - const sponsorFlags = 3 + /* eslint-disable no-bitwise -- combining flags */ + const sponsorFlags = + SponsorFlags.tfSponsorFee | SponsorFlags.tfSponsorReserve + /* eslint-enable no-bitwise */ const result = addPreFundedSponsor(payment, sponsorAddress, sponsorFlags) assert.equal(result.SponsorFlags, sponsorFlags) - assert.equal(result.SponsorFlags, 3) + assert.equal( + result.SponsorFlags, + SponsorFlags.tfSponsorFee | SponsorFlags.tfSponsorReserve, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/xrpl/test/wallet/sponsorSigner.test.ts` around lines 316 - 334, Replace the magic number 3 with the explicit enum combination when testing addPreFundedSponsor: compute sponsorFlags using the SponsorFlags enum (e.g., SponsorFlags.tfSponsorFee | SponsorFlags.tfSponsorReserve) and use that variable in assertions instead of the literal 3; update the test in sponsorSigner.test.ts to reference the SponsorFlags symbol so it’s clearer which flags are combined when calling addPreFundedSponsor and asserting result.SponsorFlags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ripple-keypairs/src/signing-schemes/secp256k1/index.ts`:
- Around line 45-57: The call to nobleSecp256k1.sign(...).toBytes('der') is
invalid because Signature objects use toDERRawBytes() for DER encoding; update
the expression in the signing function to call
nobleSecp256k1.sign(Sha512.half(message), hexToBytes(normedPrivateKey),
{...}).toDERRawBytes(), then pass that result into bytesToHex(...).toUpperCase()
so the function continues returning the same hex DER signature format.
In `@packages/xrpl/src/models/methods/index.ts`:
- Line 211: RequestResponseMap is missing a specific mapping for
AccountSponsoringRequest, so calls to client.request({ command:
'account_sponsoring', ... }) resolve to the generic fallback; update the
RequestResponseMap type (the conditional chain in index.ts) to add a branch
mapping "T extends AccountSponsoringRequest ? AccountSponsoringResponse"
immediately before the final ": Response<Version>" fallback so the compiler
resolves the concrete AccountSponsoringResponse for that request type.
In `@packages/xrpl/src/models/transactions/common.ts`:
- Around line 736-742: The self-sponsorship check using string equality (sponsor
=== tx.Account) can miss equivalence between X-address and classic address
formats; update the check in the function containing hasSponsor, sponsor and
tx.Account to normalize both addresses to their classic form using the existing
xAddressToClassicAddress (or a helper that falls back to the original if
conversion fails) and then compare the resulting classic addresses, catching any
conversion errors and still throwing the same ValidationError ('Transaction:
Sponsor and Account cannot be the same (self-sponsorship not allowed)') when
they match; apply the same normalization approach to other address-equality
spots noted (e.g., NFTokenCreateOffer, depositPreauth, sponsorshipTransfer) to
avoid cross-format false negatives.
In `@packages/xrpl/src/models/transactions/sponsorshipSet.ts`:
- Around line 181-207: The FeeAmount and MaxFee validation in the SponsorshipSet
transaction currently uses Number() coercion which accepts non-canonical drop
strings; update the validation in the SponsorshipSet validator to reject
anything that is not a canonical unsigned integer string (no whitespace, no
scientific notation, no decimals, not "Infinity" or empty) by using the existing
isXRPLNumber helper or the same drops / unsigned integer regex used elsewhere in
the codebase instead of Number(), and keep the same error messages thrown from
the SponsorshipSet validation for tx.FeeAmount and tx.MaxFee when the new strict
check fails.
In `@packages/xrpl/src/sugar/autofill.ts`:
- Around line 322-373: The code currently treats any transaction with tx.Sponsor
!= null as requiring sponsor-signature fees; change calculateSponsorFee to only
add sponsor signer fees when a SponsorSignature is present (tx.SponsorSignature
!= null) or when an explicit opt-in is provided (e.g., a new flag/field such as
tx.EstimateSponsorSigners or a function parameter), and remove the automatic fee
scaling for the plain tx.Sponsor case; update any other places that mirror this
behavior (the similar branch referenced around the later lines) to use the same
explicit check so pre-funded Sponsor/SponsorFlags flows without SponsorSignature
are not overcharged.
In `@packages/xrpl/src/Wallet/sponsorSigner.ts`:
- Around line 218-229: getTransactionWithAllSponsorSigners currently
concatenates and sorts all SponsorSignature.Signers but doesn't remove
duplicates, so identical Signer.Account entries can appear twice; change the
function to deduplicate the sortedSigners list by Signer.Account (e.g., iterate
sortedSigners and keep a single Signer per unique Signer.Account, preserving the
ordering established by compareSigners) before returning the combined
Transaction with SponsorSignature.Signers; ensure you still call
compareSigners(signer.Signer, ...) to build the initial sort and then collapse
duplicates (keeping either the first occurrence) so the returned blob won’t
contain duplicate signer entries.
- Around line 91-124: Update the self-sponsorship guard and validation order in
sponsorSigner: check self-sponsorship using tx.Sponsor (i.e., throw if
tx.Account === tx.Sponsor) instead of comparing to wallet.classicAddress, then
perform validate(...) after you attach SponsorSignature so the final signed
payload is validated; ensure both the multisign branch (where you set
tx.SponsorSignature with Signers/TxnSignature via computeSignature(tx,
wallet.privateKey, multisignAddress)) and the single-signer branch (Setting
SigningPubKey/TxnSignature via computeSignature(tx, wallet.privateKey)) are
followed by a call to validate(tx) before calling encode(tx).
- Around line 272-291: The sponsorFlags validation currently uses typeof
sponsorFlags !== 'number', which permits NaN/Infinity/floats; change the check
to use Number.isInteger(sponsorFlags) (e.g., if
(!Number.isInteger(sponsorFlags)) throw ValidationError) inside the
addPreFundedSponsor validation block that references sponsorFlags and
SponsorFlags.tfSponsorFee|tfSponsorReserve so non-integer and non-number values
are rejected before the bitwise flag checks.
In `@packages/xrpl/test/client/autofill.test.ts`:
- Around line 669-769: Rename and split the existing sponsorship tests to
separate "sponsor-signed" from "pre-funded" scenarios: ensure the sponsor-signed
tests supply a SponsorSignature (or signer list for multisig sponsor) in the tx
object so autofill() will detect sponsor signatures and add the correct extra
signature fees (e.g., modify the single-sponsor test to include SponsorSignature
and the multisig test to mock signer_lists for the Sponsor account), and add a
new pre-funded test that only sets Sponsor (no SponsorSignature and no
signer_lists) and asserts the base fee (12) remains unchanged; update test names
(e.g., tests around Payment tx, txResult from client.autofill) and expectations
accordingly so fees reflect the correct path.
---
Outside diff comments:
In `@packages/xrpl/src/sugar/getOrderbook.ts`:
- Around line 15-22: sortOffers currently constructs BigNumber from possibly
non-numeric quality values so comparedTo can return null; update sortOffers to
defensively coerce/validate qualities before comparing: inside sortOffers
(function name) convert qualityA/qualityB to BigNumber instances, check .isNaN()
on each and replace NaN instances with BigNumber(0), then call comparedTo on the
validated BigNumber instances and return that result; reference the variables
qualityA, qualityB and the function sortOffers when making the change.
---
Nitpick comments:
In `@packages/xrpl/src/sugar/validateSponsorship.ts`:
- Around line 88-110: When tfSponsorFee/isSponsoringFee is true, ensure
FeeAmount is required: update validateSponsorship (around isSponsoringFee and
the sponsorship.FeeAmount check) to return valid:false with a clear error if
sponsorship.FeeAmount is undefined or null (instead of skipping validation), and
keep returning sponsorship and estimatedFee in that error response; otherwise
continue to parse FeeAmount and compare it to fee as currently implemented.
In `@packages/xrpl/test/integration/transactions/sponsorship.test.ts`:
- Around line 776-781: Replace the magic literal 3 assigned to SponsorFlags with
the explicit enum combination of the two sponsorship flag values; update the
object so SponsorFlags is set using the bitwise OR of the enum members (e.g.,
SponsorFlag.FEE | SponsorFlag.RESERVE) instead of the number 3 so the intent is
clear when creating the transaction that includes sponsorWallet.classicAddress.
In `@packages/xrpl/test/wallet/sponsorSigner.test.ts`:
- Around line 316-334: Replace the magic number 3 with the explicit enum
combination when testing addPreFundedSponsor: compute sponsorFlags using the
SponsorFlags enum (e.g., SponsorFlags.tfSponsorFee |
SponsorFlags.tfSponsorReserve) and use that variable in assertions instead of
the literal 3; update the test in sponsorSigner.test.ts to reference the
SponsorFlags symbol so it’s clearer which flags are combined when calling
addPreFundedSponsor and asserting result.SponsorFlags.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20082e83-297d-4ea2-a1ae-f2f99a55ff62
📒 Files selected for processing (36)
packages/ripple-binary-codec/src/enums/definitions.jsonpackages/ripple-keypairs/src/signing-schemes/secp256k1/index.tspackages/xrpl/src/Wallet/index.tspackages/xrpl/src/Wallet/sponsorSigner.tspackages/xrpl/src/Wallet/utils.tspackages/xrpl/src/index.tspackages/xrpl/src/models/common/index.tspackages/xrpl/src/models/ledger/AccountRoot.tspackages/xrpl/src/models/ledger/LedgerEntry.tspackages/xrpl/src/models/ledger/RippleState.tspackages/xrpl/src/models/ledger/Sponsorship.tspackages/xrpl/src/models/ledger/index.tspackages/xrpl/src/models/methods/accountObjects.tspackages/xrpl/src/models/methods/accountSponsoring.tspackages/xrpl/src/models/methods/index.tspackages/xrpl/src/models/methods/ledger.tspackages/xrpl/src/models/methods/ledgerEntry.tspackages/xrpl/src/models/methods/simulate.tspackages/xrpl/src/models/methods/tx.tspackages/xrpl/src/models/transactions/common.tspackages/xrpl/src/models/transactions/index.tspackages/xrpl/src/models/transactions/payment.tspackages/xrpl/src/models/transactions/sponsorshipSet.tspackages/xrpl/src/models/transactions/sponsorshipTransfer.tspackages/xrpl/src/models/transactions/transaction.tspackages/xrpl/src/models/utils/flags.tspackages/xrpl/src/sugar/autofill.tspackages/xrpl/src/sugar/getOrderbook.tspackages/xrpl/src/sugar/index.tspackages/xrpl/src/sugar/validateSponsorship.tspackages/xrpl/test/client/autofill.test.tspackages/xrpl/test/integration/transactions/sponsorship.test.tspackages/xrpl/test/models/sponsorshipSet.test.tspackages/xrpl/test/models/sponsorshipTransfer.test.tspackages/xrpl/test/sugar/validateSponsorship.test.tspackages/xrpl/test/wallet/sponsorSigner.test.ts
Comment on lines
45
to
57
| return bytesToHex( | ||
| nobleSecp256k1.sign(Sha512.half(message), hexToBytes(normedPrivateKey), { | ||
| // "Canonical" signatures | ||
| lowS: true, | ||
| // Would fail tests if signatures aren't deterministic | ||
| extraEntropy: undefined, | ||
| format: 'der', | ||
| // We pass a pre-hashed message (Sha512Half), so disable secp256k1's | ||
| // default SHA-256 prehashing (added as default in @noble/curves 2.0.0) | ||
| prehash: false, | ||
| }), | ||
| nobleSecp256k1 | ||
| .sign(Sha512.half(message), hexToBytes(normedPrivateKey), { | ||
| // "Canonical" signatures | ||
| lowS: true, | ||
| // Would fail tests if signatures aren't deterministic | ||
| extraEntropy: undefined, | ||
| // We pass a pre-hashed message (Sha512Half), so disable secp256k1's | ||
| // default SHA-256 prehashing (added as default in @noble/curves 2.0.0) | ||
| prehash: false, | ||
| }) | ||
| .toBytes('der'), | ||
| ).toUpperCase() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the return type of secp256k1.sign() in @noble/curves when options are passed but no format is specified? Does the Signature object have toBytes or toDERRawBytes method?
💡 Result:
The return type of secp256k1.sign in @noble/curves, when options are passed but no format is specified, is a Signature object. This is evidenced by documentation examples where secp256k1.sign(msg, secretKey) without format returns a Signature instance, as shown in code snippets like const sigInstance = secp256k1.Signature.fromBytes(sigRec, 'recovered'); and historical notes that sign returns Signature with { r, s, recovery }. The default format when unspecified is 'compact' (64-byte RS), but the primary return is the Signature class instance. The Signature object has toCompactRawBytes and toDERRawBytes methods (or equivalent toBytes('compact')/'der' in some versions), as explicitly stated in interface definitions: interface ECDSASignature { r: bigint; s: bigint; recovery?: number; toCompactRawBytes: Uint8Array; toDERRawBytes: Uint8Array; }. Older versions used toBytes, but raw bytes methods are confirmed. It does not have a plain toBytes method without arguments based on available source snippets, but the queried methods exist.
Citations:
- 1: https://www.npmjs.com/package/@noble/secp256k1
- 2: https://www.npmjs.com/package/@noble/curves
- 3: https://github.com/paulmillr/noble-curves
- 4: https://tessl.io/registry/tessl/npm-noble--curves/1.9.0
- 5: https://raw.githubusercontent.com/paulmillr/noble-secp256k1/main/index.ts
Build failure: toBytes is not the correct method for DER encoding.
In @noble/curves, secp256k1.sign() returns a Signature object. To export it as DER-encoded bytes, use .toDERRawBytes() instead of .toBytes('der'), which does not exist on the Signature class.
Fix
return bytesToHex(
nobleSecp256k1
.sign(Sha512.half(message), hexToBytes(normedPrivateKey), {
lowS: true,
extraEntropy: undefined,
prehash: false,
})
- .toBytes('der'),
+ .toDERRawBytes(),
).toUpperCase()🧰 Tools
🪛 GitHub Actions: Node.js CI
[error] 56-56: TypeScript build failed in ripple-keypairs. TS2339: Property 'toBytes' does not exist on type 'Uint8Array'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ripple-keypairs/src/signing-schemes/secp256k1/index.ts` around lines
45 - 57, The call to nobleSecp256k1.sign(...).toBytes('der') is invalid because
Signature objects use toDERRawBytes() for DER encoding; update the expression in
the signing function to call nobleSecp256k1.sign(Sha512.half(message),
hexToBytes(normedPrivateKey), {...}).toDERRawBytes(), then pass that result into
bytesToHex(...).toUpperCase() so the function continues returning the same hex
DER signature format.
Collaborator
Author
There was a problem hiding this comment.
This finding is based on an outdated version of the @noble/curves api. We are using version 2.x which includes a change from toDERRawBytes to toBytes('der').
Contributor
There was a problem hiding this comment.
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!
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/xrpl/src/models/transactions/depositPreauth.ts (1)
56-72: Consider usingisStringhelper for consistency with NFTokenCreateOffer.The X-address equality checks are correct. However, NFTokenCreateOffer imports and uses the
isStringhelper, while this file uses inlinetypeof ... === 'string'. Consider importingisStringfrom./commonfor consistency across transaction validators.♻️ Suggested refactor for consistency
import { BaseTransaction, validateBaseTransaction, validateCredentialsList, MAX_AUTHORIZED_CREDENTIALS, areAddressesEqual, + isString, } from './common'Then update the checks:
if ( - typeof tx.Account === 'string' && + isString(tx.Account) && areAddressesEqual(tx.Account, tx.Authorize) ) {if ( - typeof tx.Account === 'string' && + isString(tx.Account) && areAddressesEqual(tx.Account, tx.Unauthorize) ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/xrpl/src/models/transactions/depositPreauth.ts` around lines 56 - 72, Replace inline typeof string checks in the DepositPreauth validator with the shared isString helper for consistency: import isString from ./common at the top, then use isString(tx.Account), isString(tx.Unauthorize) (and any other typeof ... === 'string' checks in this file) instead of typeof ... === 'string'; keep the existing areAddressesEqual logic and ValidationError messages unchanged. This mirrors how NFTokenCreateOffer uses isString and ensures consistent validation helpers across validators.packages/xrpl/src/sugar/autofill.ts (1)
331-337: Consider using|| 1for defensive handling of edge cases.Using the nullish coalescing operator (
?? 1) won't catch an emptySignersarray because0is not nullish. IfSignersis[],sponsorSignersCountwould be0, leading to no additional fee. While the protocol likely rejects transactions with emptySignersarrays, using|| 1would be more defensive:- const sponsorSignersCount = tx.SponsorSignature.Signers?.length ?? 1 + const sponsorSignersCount = tx.SponsorSignature.Signers?.length || 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/xrpl/src/sugar/autofill.ts` around lines 331 - 337, The code currently sets sponsorSignersCount with "tx.SponsorSignature.Signers?.length ?? 1", which treats an empty Signers array as 0; change this defensive handling to use logical OR so an empty array yields at least 1 signer: compute sponsorSignersCount from tx.SponsorSignature.Signers?.length || 1 (referencing the SponsorSignature.Signers access and the sponsorSignersCount variable) and keep returning new BigNumber(scaleValue(netFeeDrops, sponsorSignersCount)) so fee calculation still uses scaleValue and netFeeDrops.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/xrpl/src/models/transactions/depositPreauth.ts`:
- Around line 56-72: Replace inline typeof string checks in the DepositPreauth
validator with the shared isString helper for consistency: import isString from
./common at the top, then use isString(tx.Account), isString(tx.Unauthorize)
(and any other typeof ... === 'string' checks in this file) instead of typeof
... === 'string'; keep the existing areAddressesEqual logic and ValidationError
messages unchanged. This mirrors how NFTokenCreateOffer uses isString and
ensures consistent validation helpers across validators.
In `@packages/xrpl/src/sugar/autofill.ts`:
- Around line 331-337: The code currently sets sponsorSignersCount with
"tx.SponsorSignature.Signers?.length ?? 1", which treats an empty Signers array
as 0; change this defensive handling to use logical OR so an empty array yields
at least 1 signer: compute sponsorSignersCount from
tx.SponsorSignature.Signers?.length || 1 (referencing the
SponsorSignature.Signers access and the sponsorSignersCount variable) and keep
returning new BigNumber(scaleValue(netFeeDrops, sponsorSignersCount)) so fee
calculation still uses scaleValue and netFeeDrops.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 227becb1-42ac-495b-9b1e-3bfce350d6f7
📒 Files selected for processing (9)
packages/xrpl/src/Wallet/sponsorSigner.tspackages/xrpl/src/models/methods/index.tspackages/xrpl/src/models/transactions/NFTokenCreateOffer.tspackages/xrpl/src/models/transactions/common.tspackages/xrpl/src/models/transactions/depositPreauth.tspackages/xrpl/src/models/transactions/sponsorshipSet.tspackages/xrpl/src/models/transactions/sponsorshipTransfer.tspackages/xrpl/src/sugar/autofill.tspackages/xrpl/test/client/autofill.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/xrpl/src/models/methods/index.ts
- packages/xrpl/test/client/autofill.test.ts
- packages/xrpl/src/models/transactions/sponsorshipTransfer.ts
- packages/xrpl/src/models/transactions/sponsorshipSet.ts
- packages/xrpl/src/models/transactions/common.ts
- packages/xrpl/src/Wallet/sponsorSigner.ts
… xrpl4j - Add optional Sponsor field to Check, DepositPreauth, Escrow, NFTokenOffer, Offer, PayChannel, and Ticket ledger entry types so consumers can read the sponsor address from returned ledger objects. - Add deletion_blockers_only and type params to AccountSponsoringRequest to match the account_sponsoring RPC shape supported by xrpl4j.
…onsor rippled - Sponsorship LedgerEntryType was 135; rippled's XLS-68 branch defines ltSPONSORSHIP = 0x0090 = 144. Update definitions.json so parsed Sponsorship ledger entries decode correctly. - Switch CI rippled image from rippleci/rippled:develop (no XLS-68) to legleux/xrpld:sponsor so integration tests can actually exercise SponsorshipSet / SponsorshipTransfer instead of being rejected with "Unknown field". Matches the image xrpl4j uses.
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
Support for XLS-68 (Sponsored Fees and Reserves)
Context of Change
Type of Change
Did you update HISTORY.md?
Test Plan
Added/updated unit tests, integration tests, and relevant autofill tests