[prajwalu142]

Description

explainTransaction for XLM reported only setFlags, clearFlags and lowThreshold for a setOptions operation, and silently left out any operation type it does not model. A reviewer reading the explanation (for example in offline vault console) could sign a transaction that adds a signer, sets thresholds, a master weight, a home domain or an inflation destination, that runs an operation against another account, or that carries an extra operation, without seeing it.

Changes in modules/sdk-coin-xlm:

• accountConfig operations also carry masterWeight, medThreshold, highThreshold, homeDomain, inflationDest and signer. Each is undefined when the operation does not set it. A masterWeight or signer weight of 0 is kept (weight 0 means signer removal). A signer is reported as { type, key, weight } for ed25519 public key, sha256 hash, pre-auth transaction and signed payload signers.
• authorizeTrustline operations carry authorized, authorizedToMaintainLiabilities and clawbackEnabled. Each is true when the operation sets the flag, false when it clears it and undefined when it does not touch it.
• clawback operations carry assetCode and assetIssuer, so consumers do not parse the coin name to label the amount.
• Every operations entry and every outputs entry carries source, set only when the operation names its own source account, so an operation that runs against another account is visible.
• An operation type that explain does not model now produces an entry of type unsupported with the stellar operationType, instead of being dropped. Payments and account creation are still reported through outputs, and changeTrust through operations.

All additions are optional fields. A consumer that assumes every operations entry is one of the known types will now also see unsupported entries, and strict deep-equality on explain output will see the new optional keys.

Issue Number

CSHLD-2079

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Unit tests added in modules/sdk-coin-xlm/test/unit/xlm.ts for each new setOptions field, unset fields, signer variants and removal, the three trustline flags, the clawback asset fields, operation source on every operation kind and its absence, the unsupported entry (including that payment and changeTrust do not produce one), and operation order for a mixed transaction. The explain tests pass. The rest of the module suite and tsc give the same results with and without the change; prettier and eslint are clean.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• My code compiles correctly for both Node and Browser environments

[linear-code]

CSHLD-2079

[github-actions]

⚠️ Compatibility checks are failing on Node.js 27-nightly (experimental, non-blocking). This does not block merge, but it signals an incompatibility to fix before this release becomes supported.

View run

[prajwalu142]

@claude review this pr for regression, backwards compatibility and edge cases

[github-actions]

Claude finished @prajwalu142's task in 30s —— View job

---

Review: regression, backwards compatibility, edge cases

I read the diff in modules/sdk-coin-xlm/src/xlm.ts and the surrounding explainTransaction code. I did not run the tests. The change looks safe to merge, and I found nothing blocking.

Regression

• No change to existing paths. The payment, createAccount, changeTrust, authorizeTrustline and clawback branches are unchanged apart from the extra fields. The outputs, spendAmount and spendAmounts calculations aren't touched.
• No in-repo consumers affected. accountConfig and authorizeTrustline appear only in sdk-coin-xlm and in sdk-core BuildParams, which is the build-parameter type and doesn't read explain output. The new else branch only adds entries to operations. It can't throw, and it doesn't touch the verify paths (verifyTransaction works on raw tx.operations).

Backwards compatibility

• Additive fields. All new fields are optional. Anything that serializes the result to JSON drops undefined fields, so existing consumers see the same shape for those entries.
• unsupported entries are the one behavior change. Any transaction that contains an operation type explain doesn't model (manageData, bumpSequence, accountMerge, manageSellOffer, setTrustLineFlags variants and so on) now gets an extra operations entry. Previously it was dropped.
  • A consumer that does operations.length > 0 or operations[0].type === ... may behave differently on such transactions. Examples are a UI that treats any non-empty operations as "token enablement" or "account config".
  • The PR description already notes this. A release note or changelog entry would help.
• Typing. type is still string, so there's no compile-time break. A string-literal union would make the new 'unsupported' value easier to discover, but it would be a TypeScript-visible change, so leaving it as string is reasonable.

Edge cases

1. Zero values. masterWeight: 0 passes through as 0, which is correct, and the PR tests it. Consumers must check !== undefined, not truthiness. Worth a line in the docs.
2. Signer changes are invisible. The PR says this is intentional. A setOptions operation that only adds or removes a signer, with no other field set, still yields an accountConfig entry with every field undefined. That looks like a no-op to a reviewer but changes who controls the account, which is the risk this PR is meant to close. Consider adding a signer field, or a hasSignerChange: true marker. At the least, mention it in the docs.
3. setOptions with source set. Operation-level source accounts aren't reported for any operation type, including payment. A multi-source transaction is therefore only partly described. This is pre-existing but relevant to the goal of the PR. You could add source to unsupported entries and accountConfig entries at minimum.
4. unsupported entries carry no detail. Only operationType is reported, so a reviewer can't see an accountMerge destination or a manageData key. That's fine as a safety flag. Consumers should treat it as "block or escalate", not "ignore".
5. liquidity_pool_shares in clawback. changeTrust guards against liquidity_pool_shares, but clawback doesn't. clawbackOp.asset.code is fine because clawback takes an Asset, but the new fields assume that. This is pre-existing and not a concern.
6. Native asset. assetCode and assetIssuer would be 'XLM' and undefined for a native asset. Clawback of native is invalid on Stellar, so this is moot.

Tests

The PR covers each new field, the unset case, 0, the three authorized states, and that payment and changeTrust don't produce unsupported. One extra case would help: a mixed transaction (payment + manageData) should produce outputs plus a single unsupported entry with the right ordering.

Summary: safe and additive. The only behavior change is the new unsupported entries, which is intended. The main suggestion is to consider surfacing signer changes, or at least flagging them, because as written they are indistinguishable from an empty accountConfig.
· Branch: prajwalu/cshld-2079-xlm-explain-account-config

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 modules/sdk-coin-xlm/src/xlm.ts — pre-existing, security-relevant: Reviewers still cannot see two of the three trustline flags a setTrustLineFlags op changes, so a full deauthorization can show as "not touched". The authorizeTrustline entry at xlm.ts:1107-1112 copies only flags?.authorized and drops authorizedToMaintainLiabilities and clawbackEnabled. Fix: report every flag the op sets or clears (true, false or undefined each), and update the new comment, which claims undefined authorized means no revoke. Clearing maintain-liabilities on a trustline in that state deauthorizes it while authorized stays undefined. [also at: modules/sdk-coin-xlm/src/xlm.ts:151 - Pre-existing, security-relevant: an issuer reviewing an authorizeTrustline explanation still cannot see a setTrustLineFlags op that clears clawbackEnabled or changes authorizedToMaintainLiabilities.]

  Why this was flagged

  Trigger: a txBase64 holding stellar.Operation.setTrustLineFlags({ trustor, asset, flags: { authorizedToMaintainLiabilities: false } }) is passed to explainTransaction. When the trustline is in the authorized-to-maintain-liabilities state, that op fully deauthorizes it. The explain loop at xlm.ts:1107-1112 sets only authorized: setTrustLineFlagsOp.flags?.authorized. The entry therefore shows authorized undefined, and the new interface comment at xlm.ts:150-151 tells consumers that undefined "must not be read as a revoke". A second case: flags { authorized: false, authorizedToMaintainLiabilities: true } shows as authorized: false, a full revoke, when the real effect is a downgrade to maintain-liabilities. Clearing clawbackEnabled is likewise never shown. The base branch has the same omission. verifyAuthorizeTrustlineOperations at xlm.ts:1328-1333 also checks only authorized, so verification does not catch these flags either.

  Verification: The explain loop at modules/sdk-coin-xlm/src/xlm.ts:1107-1112 pushes only type, coin, trustor and authorized: setTrustLineFlagsOp.flags?.authorized. The other two flags are never copied, and the TransactionOperation interface (xlm.ts:147-152) has no field for them. The base has the same mapping (base xlm.ts:1089-1094, same authorized: setTrustLineFlagsOp.flags?.authorized).

[claude]

🟣 pre-existing, not blocking: pre-existing, security-relevant: A reviewer still sees no sign of a setOptions that adds a signer, which is the most dangerous account change. The accountConfig entry at xlm.ts:1093-1104 omits setOptionsOp.signer. A signer-only op shows up as accountConfig with every field undefined, which looks like a no-op. Fix: report signer (key and weight, with weight 0 meaning removal) on accountConfig entries. The PR says this omission is intentional, but the stated goal is that no signed account change is hidden.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Trigger: a txBase64 holding stellar.Operation.setOptions({ signer: { ed25519PublicKey: attacker, weight: 3 } }) is passed to explainTransaction. The setOptions branch at xlm.ts:1091-1104 copies setFlags, clearFlags, masterWeight, the three thresholds, homeDomain and inflationDest, but never signer. The result is { type: 'accountConfig', coin } with every other field undefined. verifyAccountConfigOperations (xlm.ts:1269-1281) lists 'signer' among the fields that matter, citing signer:{attacker,weight:3}, but that check only runs on the verify path, not for explain consumers such as offline vault review. The base behaves the same way. Still, this PR extends the setOptions output so that a reviewer can see every account change, and it leaves out the field that hands over control of the account. The PR description calls the omission intentional but gives no mitigation for explain readers.

Verification: Trigger: a setOptions op that sets only signer is passed to explainTransaction. The setOptions branch at modules/sdk-coin-xlm/src/xlm.ts:1091-1104 never reads setOptionsOp.signer. So a signer-only op comes out as { type: 'accountConfig', coin }, which looks like a no-op. The 'signer' guard in verifyAccountConfigOperations only runs in verifyTransaction. The base had the same omission.

[prajwalu142]

Addressed in 416b1f5. accountConfig entries now carry signer as { type, key, weight } (ed25519 public key, sha256 hash, pre-auth tx and signed payload signers). Weight 0 is kept and means removal, so a signer-only setOptions no longer reads as an empty entry. Tests cover add, removal, hex-encoded hash and pre-auth signers, and the unset case.

[claude]

🟣 pre-existing, not blocking: pre-existing, security-relevant: Reviewers cannot tell when any operation runs against a different account, because no explained operation reports its op-level source. The explain loop at xlm.ts:1047-1124 ignores op.source for setOptions, setTrustLineFlags, clawback, changeTrust, payment and createAccount. A setOptions with source set to another wallet looks the same as one for the tx source. Fix: add op.source (undefined when unset) to every operations entry and output the loop builds, including the new unsupported entries. The verify path already rejects source on setOptions.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Trigger: a transaction whose setOptions or clawback op carries source: otherAccount, for example stellar.Operation.setOptions({ source: victim, masterWeight: 0 }), is passed to explainTransaction. Each branch of the forEach at xlm.ts:1047-1125 builds its entry only from op-specific fields, and none reads op.source. The explanation therefore shows masterWeight: 0 as if it applied to the transaction's own account. verifyAccountConfigOperations (xlm.ts:1263-1281) names 'source' as dangerous, saying an op redirected to another account runs under that account's thresholds. That check only runs on the verify path and only for setOptions. The base omits source in the same way. This PR's stated aim is that a reviewer sees everything a transaction does, and it adds fields to these same entries without this one.

Verification: The trigger is a txBase64 whose op carries an op-level source for another account, passed to explainTransaction. No branch of the loop in modules/sdk-coin-xlm/src/xlm.ts (lines 1047-1126) reads op.source. The base explain loop also never reported source. The source check in verifyAccountConfigOperations (1269-1277) covers only setOptions on the verify path, not explain.

[prajwalu142]

Addressed in 416b1f5. Every operations entry (including unsupported) and every outputs entry now carries source, set only when the operation names its own source account and undefined otherwise. Tests cover an op-level source on each operation kind and its absence.

[prajwalu142]

Addressed in 416b1f5:

• Trustline flags: authorizeTrustline entries now report authorized, authorizedToMaintainLiabilities and clawbackEnabled. Each is true when the operation sets the flag, false when it clears it and undefined when it does not touch it. The interface comment now says undefined is never a revoke and that consumers must read all three flags. A test covers a downgrade to maintain-liabilities and a clear of maintain-liabilities with authorized left undefined.
• Signer changes: reported on accountConfig (see the inline thread).
• Op-level source: reported on every operation and output (see the inline thread).
• Mixed transaction: added a test where a payment, an unmodelled op and a clawback give one outputs entry plus unsupported then clawback, in order.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}