Skip to content

feat(history): show muxed contract (W) destinations - #3049

Merged
JiahuiWho merged 2 commits into
feature/104-send-w-destinationsfrom
feature/106-history-w-muxed-id
Oct 30, 2026
Merged

JiahuiWho merged 2 commits into
feature/104-send-w-destinationsfrom
feature/106-history-w-muxed-id

Conversation

@JiahuiWho

Copy link
Copy Markdown
Contributor

What

History shows a SAC transfer to a muxed contract (W…) address as sent to that W address. Before this, the row and the transaction detail showed the bare C.

  • AssetBalanceChange gains the optional destination_muxed_id field, matching the SDK's Horizon BalanceChange type.
  • processAssetBalanceChanges rebuilds the W with encodeMuxedContractAddress for the displayed counterparty. Matching the user's own balance changes stays on the base C, so a smart wallet still sees its credits.
  • The transaction detail shows To WA7Q…AWWC and no memo row, the same way an M destination is shown today.
  • getAttrsFromSorobanHorizonOp catches decode errors and returns null. One undecodable envelope no longer breaks the whole history load.

Why

Closes stellar/wallet-eng-monorepo#106.

Screenshots / video

These are generated from e2e stub, since testnet is on protocol 29 and no W transfer exists on chain, I can't take screenshots on a real network.

w-history-detail w-history-row

Checklist

  • I have read CONTRIBUTING.md and this PR meets its requirements.
  • This PR has narrow scope and does not mix refactoring with feature changes.
  • This PR adds tests for the new functionality or fixes.
  • These changes have been tested in both Chrome and Firefox.
  • This PR includes relevant before and after screenshots for UI changes.
  • I took the time to review my own PR.

@JiahuiWho
JiahuiWho marked this pull request as ready for review October 28, 2026 20:42
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-470f9a1bc2e02a4eb057
Backend: V1 prod + V2 beta (no sandbox configured for @JiahuiWho). SDF collaborators only — install instructions in the release description.

@JiahuiWho
JiahuiWho added this pull request to stack #3054 October 29, 2026 13:45
@JiahuiWho
JiahuiWho force-pushed the feature/106-history-w-muxed-id branch from ad4774f to f3cfc7c Compare October 29, 2026 13:46
@JiahuiWho
JiahuiWho force-pushed the feature/106-history-w-muxed-id branch from a9ee912 to 6d03ec6 Compare October 30, 2026 14:34
@piyalbasu

Copy link
Copy Markdown
Contributor

The mux-id rebuild is keyed on W only, and a test pins that (pre-existing for M)

Low — not a regression and not a merge blocker. A question about scope, plus one test I'd change.

TL;DR: Horizon reports a SAC transfer to any muxed destination the same way: the bare base address in to and the id in a separate field. That is true whether the sender used a W or an M. This PR is the first code to read that field, and it rebuilds the displayed address only when the base is a contract, so a SAC transfer to an M keeps rendering as the bare G with a memo row. That was already the case before this PR, so nothing regresses. But the new unit test asserts that a G plus an id stays a bare G, which turns the gap into pinned behaviour. Since the general version branches on base type and is the same size, is contract-only intentional? If so, I'd drop or invert that test rather than lock the M case in.


Detailed explanation (for agents)

Horizon shape: createSACBalanceChangeEntry in stellar-horizon sets to from the event's to topic and destination_muxed_id from the event's destination memo id, with no distinction between muxed accounts and muxed contracts. The SDK's HorizonApi.BalanceChange already declares destination_muxed_id?: string.

The guard:

// Determine if this is a credit (receiving) or debit (sending)
const isCredit = isSameAccount(change.to, publicKey);
// Destination is the counterparty (from for credits, to for debits)
let destination = isCredit ? change.from : change.to;
// A muxed contract destination arrives as the base contract plus a mux
// id. Rebuild the muxed address for display; matching above stays on
// the base address.
if (!isCredit && change.destination_muxed_id && isContractId(change.to)) {
destination = encodeMuxedContractAddress(
change.to,
change.destination_muxed_id,
);
}

The test that pins it:

it("only rebuilds a muxed address when the destination is a contract", async () => {
jest
.spyOn(sorobanHelpers, "getAttrsFromSorobanHorizonOp")
.mockReturnValue(null as any);
const operation = buildInvokeHostFnOperation({
asset_balance_changes: [
{
asset_type: "native",
from: PUBLIC_KEY,
to: COUNTERPARTY,
amount: "5",
destination_muxed_id: "1",
},
],
});
const row = await callGetRowData(operation);
expect(row.metadata.to).toBe(COUNTERPARTY);
});
});
const NON_NATIVE_CONTRACT =

Where M-via-SAC comes from: not Freighter's own send flow (XLM or a classic asset to an M goes classic). It comes from a dApp or another wallet invoking the SAC transfer with an M as to (CAP-67), with this user as sender.

Suggested fix: branch on the base type. StrKey.isValidContract(to) → encodeMuxedContractAddress; StrKey.isValidEd25519PublicKey(to) → createMuxedAccount from helpers/stellar.ts (already used by the send flow). Then getMemoDisabledState hides the memo row for both, and the test flips to expect the M strkey.

@piyalbasu

Copy link
Copy Markdown
Contributor

One malformed mux id takes down the whole history load

Medium — the failure mode is "no history at all", the same class the soroban.ts change in this PR is guarding against. Merge blocker only if the backend-v2 history path can feed this code; otherwise a cheap guard.

TL;DR: The SDK's muxed-contract encoder throws on any id that is not a decimal string of at most 22 digits, and the call is not wrapped. If a balance change ever carries an id as a JSON number, an empty string, or anything non-numeric, the throw escapes section building and the whole fetch fails. One bad record should degrade to showing the bare address, not blank the tab.

Steps to reproduce:

  1. Stub a history record whose asset_balance_changes[0].destination_muxed_id is 1 (number) or "abc".
  2. Open the History tab.
  3. The fetch lands in its error state and no rows render.

Detailed explanation (for agents)

The unguarded call:

if (!isCredit && change.destination_muxed_id && isContractId(change.to)) {
destination = encodeMuxedContractAddress(
change.to,
change.destination_muxed_id,
);
}

What the SDK enforces (parseUint64Id in decode_encode_muxed_account.js, stellar-sdk 17.4.0): throws unless typeof id === "string", id.length <= 22, /^\d+$/ matches, and the value is at most MAX_UINT64.

Suggested fixes (in increasing order of depth):

  1. Cheap guard: wrap the encode in try/catch and fall back to change.to, with a captureException carrying the record id as extra.
  2. Validate first: /^\d{1,22}$/.test(change.destination_muxed_id) before encoding, which also documents the contract with the backend.

Whichever guard lands should also cover the account-base branch if the previous comment's fix is taken.

@piyalbasu

Copy link
Copy Markdown
Contributor

The new catch hides fee-bumped Soroban ops instead of decoding them (pre-existing break)

Medium — before this PR a fee-bumped Soroban transfer broke the whole history load; after it, the same record is silently misrendered and reported to Sentry on every load. Not a merge blocker, but the real fix is a known three-line unwrap, so I'd take it here.

TL;DR: The envelope decoder returns a fee-bump transaction for fee-bump envelopes, and that object has no operations list. The code indexes the first operation directly, so it throws. That throw used to fail the entire history fetch. With the new catch it returns nothing, so a fee-bumped token transfer shows as a generic contract interaction instead of Sent or Received, drops out of the asset's history, and fires a Sentry event. The background already has the correct unwrap for this exact case.

Steps to reproduce:

  1. Submit a custom-token transfer wrapped in a fee bump (any sponsor flow), or stub a history record whose envelope_xdr is a fee-bump envelope.
  2. Open the History tab.
  3. The row renders as "Interacted / Contract Function" and a Sentry event fires.

Detailed explanation (for agents)

The decode and index:

// One undecodable envelope must not break the whole history load.
try {
const txEnvelope = TransactionBuilder.fromXdr(
operation.transaction_attr.envelope_xdr as string,
networkDetails.networkPassphrase,
) as Transaction;
// only one op per tx in Soroban right now; isSorobanOp above guarantees it
// is an invokeHostFunction operation
const invokeHostFn = txEnvelope
.operations[0] as Operation.InvokeHostFunction;
return getInvocationArgsFromInvokeHostFn(invokeHostFn);
} catch (error) {
captureException(`Failed to decode Soroban operation: ${error}`);
return null;
}

TransactionBuilder.fromXdr returns FeeBumpTransaction for envelopeTypeTxFeeBump, so txEnvelope.operations is undefined and [0] throws a TypeError, now swallowed by the catch.

The existing unwrap to reuse:

const getOperations = (
transaction: StellarSdk.Transaction | StellarSdk.FeeBumpTransaction,
) => {
if ("innerTransaction" in transaction) {
return transaction.innerTransaction.operations;
}
if ("operations" in transaction) {
return transaction.operations;
}
return [];
};

Two related things worth tightening while here:

  • captureException is passed a template string, so Sentry gets no stack and cannot group. The sibling at line 1159 passes the error object; pass error here with the context as extra.
  • getAttrsFromSorobanHorizonOp is called several times per operation across the history hook and account helpers, so one undecodable op reports more than once per load. Decoding once per operation would fix that, but it is a follow-up, not a blocker.

Suggested fix: unwrap with "innerTransaction" in tx ? tx.innerTransaction.operations : tx.operations before indexing, keep the catch for genuinely malformed XDR, and pass the error object to Sentry.

@JiahuiWho

JiahuiWho commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed all three.

M rebuild: good catch. Now branches on the base: contract → W, account → M via createMuxedAccount. Flipped the test to expect the M.

Bad mux id: fixed. The encode is wrapped, falls back to the base address and reports to Sentry with the op id as extra.

Fee bump: @piyalbasu I checked this one. FeeBumpTransaction.operations reads through to the inner tx in stellar-base, so the old code already decoded fee bumps. Also added a per-envelope cache so the five call sites decode each op once and a bad record reports once per load.

@JiahuiWho
JiahuiWho force-pushed the feature/106-history-w-muxed-id branch from dec3f32 to 14d176a Compare October 30, 2026 19:15
@JiahuiWho
JiahuiWho merged commit 5c6956b into master Oct 30, 2026
11 checks passed
@JiahuiWho
JiahuiWho deleted the feature/106-history-w-muxed-id branch October 30, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants