Port to master: V2-precondition signing fix + Amplitude remote-code stripping#2862
Conversation
The freighter-api signTransaction flow embedded the live stellar-sdk Transaction object in the signing-popup URL blob, then JSON.stringify'd it in encodeObject. On stellar-sdk v16 (Protocol 27), a parsed tx carries a native BigInt for the V2 precondition minSeqAge (transaction ._minAccountSequenceAge), which JSON.stringify cannot serialize. The throw was swallowed as FreighterApiInternalError, so the popup never opened and signing silently failed for any V2-precondition transaction (common for Soroban dApp txs assembled via soroban-rpc). Serialize only the fields the popup actually reads (_fee, _networkPassphrase, operation types) via a new getSerializableTransaction helper. The popup already rebuilds the full transaction from transactionXdr, so dropping the live object is safe and sidesteps this and any future BigInt-bearing field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-applies the webpack alias + stub from #2822 (which was merged into the v5.41.2 release line but never forward-ported to master, so 5.42.0 and this emergency release regressed). Aliases @amplitude/plugin-autocapture-browser to a no-op stub so the autocapture "visual tagging" / "background capture" remote-script code can never be bundled — a deterministic guard against Chrome Web Store / Firefox AMO "remotely hosted code" rejection. We run with autocapture: false, so this code is dead anyway. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit f50afc8)
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-add0cded6bfa6cd13670 (SDF collaborators only — install instructions in the release description) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f6c3d788c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Ports two production fixes from the v5.42.1 emergency release line back onto master to prevent regressions in the next regular release: (1) restore signing for Soroban / V2-precondition transactions impacted by BigInt JSON serialization, and (2) deterministically strip Amplitude autocapture’s remotely-hosted-script code paths from the extension bundle to satisfy store policies.
Changes:
- Avoid serializing live stellar-sdk
Transactionobjects into the signing popup URL blob by replacing them with a minimal JSON-serializable shape (_networkPassphrase,_fee, operationtypes). - Add a regression test suite covering V2-precondition Soroban XDR serialization (including fee-bump op-type preservation).
- Add a webpack alias that replaces
@amplitude/plugin-autocapture-browserwith a no-op stub to prevent remote-code strings from appearing in the built artifacts.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
extension/webpack/amplitude-autocapture-stub.js |
Adds a stub module used to replace Amplitude autocapture plugin at build time. |
extension/webpack.common.js |
Adds webpack resolve.alias to force the autocapture plugin to resolve to the stub. |
extension/src/background/messageListener/freighterApiMessageListener.ts |
Uses a minimal serializable transaction representation when encoding the signing popup URL blob. |
extension/src/background/helpers/transactionInfo.ts |
Introduces getSerializableTransaction() helper to safely serialize only popup-consumed fields. |
extension/src/background/helpers/__tests__/transactionInfo.test.ts |
Adds regression tests ensuring V2-precondition transactions can be encoded/decoded without BigInt serialization failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ix fragment) The applied auto-fix left a flagged 'background-capture' token and a dangling sentence fragment. Rephrase without the host/lib-name/identifier literals so no remote-code string can ride into shipped source maps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5b9bf623b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export const frustrationPlugin = noopPlugin( | ||
| "@amplitude/plugin-frustration-browser", | ||
| ); |
There was a problem hiding this comment.
The current lockfile resolves @amplitude/analytics-browser to 2.44.1, whose browser client imports performancePlugin from @amplitude/plugin-autocapture-browser along with the two factories stubbed here. Because this alias does not export performancePlugin, any path that enables Amplitude's performance autocapture resolves to a missing/undefined binding instead of the intended no-op, so initialization can break despite the stub; add a no-op performancePlugin export next to these plugin stubs.
Useful? React with 👍 / 👎.
TL;DR
Ports two fixes from the
5.42.1emergency release intomasterso the next regular release doesn't regress them:Both shipped in the
5.42.1emergency release but were never onmaster, somastercurrently has both regressions.Implementation details (for agents)
Why master is missing these: the emergency-release fix (PR #2860) merged into the
v5.42.1branch, notmaster. The Amplitude stripping originally landed via #2822 into thev5.41.2release line and was likewise never forward-ported. Somasterhas neither.Change 1 — V2-precondition signing fix (
b04acf1e)freighterApiMessageListener.tsembedded the live stellar-sdkTransactionin the signing-popup URL blob andJSON.stringify'd it inencodeObject. On stellar-sdk v16, a V2-precondition tx carries a nativeBigInt(_minAccountSequenceAge), whichJSON.stringifythrows on (Do not know how to serialize a BigInt). The throw was swallowed asFreighterApiInternalError, so the popup never opened.extension/src/background/helpers/transactionInfo.ts→getSerializableTransaction()serializes only the fields the popup reads (_networkPassphrase,_fee, operationtypes). The popup already rebuilds the full tx fromtransactionXdr. No BigInt path, future-proof against other BigInt fields.extension/src/background/helpers/__tests__/transactionInfo.test.ts(4 cases) against a real V2-precondition Soroban XDR.Change 2 — Amplitude autocapture stripping (
6f6c3d78, re-applies #2822's mechanism)webpack.common.js:resolve.aliasmaps@amplitude/plugin-autocapture-browser→ a no-op stub.extension/webpack/amplitude-autocapture-stub.js: the stub.loadScriptOnce("https://cdn.amplitude.com/libs/visual-tagging-selector-1.0.0-alpha.js.gz")) still exists in the installed dependency (@amplitude/plugin-autocapture-browser). It currently tree-shakes out of the bundle, but that's an incidental side effect of the resolved dep versions — master's amplitude deps have already drifted (analytics-browser2.44.1 here vs 2.37.0 in 5.42.1), and the next bump could silently reintroduce it. We runautocapture: false, so the alias is behavior-neutral; it just makes exclusion deterministic and version-independent.Verification (on master's dependency set):
yarn jest .../transactionInfo.test.ts→ 4/4 pass (stellar-sdk v16).yarn build:extension:production→ succeeds; recursive grep of the build dir incl..mapfiles →0occurrences ofcdn.amplitude.com/visual-tagging-selector.npx tsc --noEmit -p extension/tsconfig.jsonclean;eslintclean on changed source.Permalinks (head
6f6c3d788c454b27cdc500e87ec9cf2af10783f2):Follow-up (out of scope): the emergency-release tooling (
newRelease/submitBeta/submitProduction) has atarget_commitishvscommitishtypo that mis-tags releases, andnewReleasebranched the hotfix frommasterinstead of the release tag — both tracked separately.