add issuer for changeTrust op #2246

piyalbasu · 2025-09-11T21:09:22Z

Make sure to show asset issuer on changeTrust ops
I also noticed that the Copied tooltip wasn't showing when I clicked on the copy icon. Fixing that while I'm here (this fix may not need to be ported to mobile

piyalbasu · 2025-09-11T21:12:10Z

extension/src/popup/components/signTransaction/Operations/KeyVal/index.tsx

 }: {
  operationKey: string;
  operationValue: string | number | React.ReactNode;
+  isOverflowAllowed?: boolean; // flag is used to show `Copied` tooltip


So the reason the copied tooltip wasn't showing up was because the style Operations__pair--value-text has overflow:hidden. I'm not quite sure what case this solves, so rather than possibly break styling, I'm allowing this component to be configured for both overflow: hidden and overflow:visible.

Since this is really only used when we pass CopyValue, I toyed with the idea of programmatically adding the style based on the component that was being passed. But, ultimately, that felt a little hacky and felt more deterministic to just make this a configurable flag

We could try adding e2e tests that prove that these columns won't overflow their containers without this prop if you want to remove it completely.

It looks like it's a fail safe for the column overflowing but maybe in practice all cases of values handle their own formatting/truncation.

I thought about this a bit more and it looks like overflow:hidden is used to guard against an unexpected long string to prevent the UI from doing this:

So, ultimately, I do think we need to keep overflow:hidden whenever possible to prevent this. I added a more flexible CSS fix that disables overflow:hidden for the brief moment a child is trying to show the Copied tooltip. This should help us make sure we don't break the layout, but also show the Copied tooltip without having to worry about setting a configurable flag

7be46f8

A more complete fix would be to open a Copied tooltip in a React portal to prevent these weird CSS collisions. But that's a bigger architectural change in SDS that we'll need to discuss with that crew

In terms of testing, I'd really like to add tests for every kind of op plus strings of random length to have a more holistic view of how our UI's hold up. I've been playing around trying to get AI to generate more tests for us, but thus far, every time cursor creates a test for me, it fails 😞 I'll keep working on this

ok sounds good, I think also we should maybe sync with @sdfcharles about the differences in copy implementations from mobile <-> extension. Maybe this problem goes away if we decide to move to a notification style copy feedback like in mobile.

piyalbasu · 2025-09-11T21:13:13Z

extension/src/popup/components/signTransaction/Operations/KeyVal/index.tsx

                      displayValue={truncateString(salt, 8)}
                    />
                  }
+                  isOverflowAllowed


adding this flag every time we pass CopyValue as the operationValue

piyalbasu · 2025-09-11T21:13:46Z

extension/src/popup/components/signTransaction/Operations/KeyVal/index.tsx

-    <KeyValueList operationKey={t("Asset Code")} operationValue={line.code} />
+    <>
+      <KeyValueList operationKey={t("Asset Code")} operationValue={line.code} />
+      <KeyValueList


adding the issuer here

piyalbasu · 2025-09-12T17:25:44Z

extension/src/popup/components/signTransaction/Operations/styles.scss

+
+          &:has(.Floater.trigger.trigger--active) {
+            overflow: visible;
+          }


temporarily disable overflow:hidden while showing the Copied tooltip

This reverts commit 19c8a68.

* ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op (#2246) * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * Revert "add issuer for changeTrust op (#2246)" (#2247) This reverts commit 19c8a68.

* upgrade to yarn 2 and use resolutions to block vulnerable package versions (#2239) * upgrade to yarn 2 and use resolutions to block vulnerable package versions * rm deprecated .yarnrc * rm yarnpath * try committing yarn binary to repo * try corepack enable for gha * update run tests cmd * rm yarnpath * rm npm i yarn * update all pipelines * rm superfluous history types * ensure invoke host function tx shows contract parameters (#2243) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * Bugfix/rm auth param names (#2244) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op (#2246) * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * Revert "add issuer for changeTrust op (#2246)" (#2247) This reverts commit 19c8a68. * Bugfix/add issuer for changetrust (#2249) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * cache account balances and poll for updates (#2252) * cache account balances and poll for updates * fix CI tests * rm `force:true` which was causing action to happen too fast * do a fresh balance fetch on account/network change * pr comments * add more sentry tracking for Account and Wallets views (#2268) * add more sentry tracking for Account and Wallets views * adding more sentry reporting * gracefully degrade on errors from Blockaid (#2269) * gracefully degrade on errors from Blockaid * should not be necessary to skip dapp scanning on custom network * rm extra dep * add a test for persisting configurations in the send flow (#2271) * add a test for persisting configurations in the send flow * rm logs and update muxed acct; lower xlm payment * handle missing scan-tx result; add disabled state for Confirm Anyway (#2272) * handle missing scan-tx result; add disabled state for Confirm Anyway * assertions to show correct confirm button on Blockaid error * add cache for balances to ensure we do a fresh lookup when needed (#2275) * add cache for balances to ensure we do a fresh lookup when needed * add try...catch to token-prices polling * rm log * only dispatch saveBalancesForAccount when fresh data has been fetched * adjust test to wait for UI change * replace yarn setup with just yarn * after adding/removing asset: update status (#2285) * Bugfix/update custom token balance (#2286) * after adding/removing asset: update status * update token balances after adding/removing token * update comment * test for showing e2e token in account balances view * remove event properties from blockaid metric event properties (#2288) * Bugfix/fix avail balance (#2291) * better logic for preventing negative numbers in available balance * simplify minimum balance calc by using BE's value * update mock data

* upgrade to yarn 2 and use resolutions to block vulnerable package versions (#2239) * upgrade to yarn 2 and use resolutions to block vulnerable package versions * rm deprecated .yarnrc * rm yarnpath * try committing yarn binary to repo * try corepack enable for gha * update run tests cmd * rm yarnpath * rm npm i yarn * update all pipelines * rm superfluous history types * ensure invoke host function tx shows contract parameters (#2243) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * Bugfix/rm auth param names (#2244) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op (#2246) * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * Revert "add issuer for changeTrust op (#2246)" (#2247) This reverts commit 19c8a68. * Bugfix/add issuer for changetrust (#2249) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * cache account balances and poll for updates * fix CI tests * rm `force:true` which was causing action to happen too fast * do a fresh balance fetch on account/network change * first pass at async history * pr comments * allow for history caching * add more sentry tracking for Account and Wallets views (#2268) * add more sentry tracking for Account and Wallets views * adding more sentry reporting * gracefully degrade on errors from Blockaid (#2269) * gracefully degrade on errors from Blockaid * should not be necessary to skip dapp scanning on custom network * rm extra dep * add a test for persisting configurations in the send flow (#2271) * add a test for persisting configurations in the send flow * rm logs and update muxed acct; lower xlm payment * rm slow loading simulation * handle missing scan-tx result; add disabled state for Confirm Anyway (#2272) * handle missing scan-tx result; add disabled state for Confirm Anyway * assertions to show correct confirm button on Blockaid error * add cache for balances to ensure we do a fresh lookup when needed (#2275) * add cache for balances to ensure we do a fresh lookup when needed * add try...catch to token-prices polling * rm log * only dispatch saveBalancesForAccount when fresh data has been fetched * adjust test to wait for UI change * replace yarn setup with just yarn * rm unnecessary return * clear token details on redux clear action * make history row construction async and check for redux state for updates * add tests for assetdetails * increase timeout for flakey test * pr comments * refresh account history every time account balances refresh

* Feature/move history fetch to bg (#2273) * upgrade to yarn 2 and use resolutions to block vulnerable package versions (#2239) * upgrade to yarn 2 and use resolutions to block vulnerable package versions * rm deprecated .yarnrc * rm yarnpath * try committing yarn binary to repo * try corepack enable for gha * update run tests cmd * rm yarnpath * rm npm i yarn * update all pipelines * rm superfluous history types * ensure invoke host function tx shows contract parameters (#2243) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * Bugfix/rm auth param names (#2244) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op (#2246) * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * Revert "add issuer for changeTrust op (#2246)" (#2247) This reverts commit 19c8a68. * Bugfix/add issuer for changetrust (#2249) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * cache account balances and poll for updates * fix CI tests * rm `force:true` which was causing action to happen too fast * do a fresh balance fetch on account/network change * first pass at async history * pr comments * allow for history caching * add more sentry tracking for Account and Wallets views (#2268) * add more sentry tracking for Account and Wallets views * adding more sentry reporting * gracefully degrade on errors from Blockaid (#2269) * gracefully degrade on errors from Blockaid * should not be necessary to skip dapp scanning on custom network * rm extra dep * add a test for persisting configurations in the send flow (#2271) * add a test for persisting configurations in the send flow * rm logs and update muxed acct; lower xlm payment * rm slow loading simulation * handle missing scan-tx result; add disabled state for Confirm Anyway (#2272) * handle missing scan-tx result; add disabled state for Confirm Anyway * assertions to show correct confirm button on Blockaid error * add cache for balances to ensure we do a fresh lookup when needed (#2275) * add cache for balances to ensure we do a fresh lookup when needed * add try...catch to token-prices polling * rm log * only dispatch saveBalancesForAccount when fresh data has been fetched * adjust test to wait for UI change * replace yarn setup with just yarn * rm unnecessary return * clear token details on redux clear action * make history row construction async and check for redux state for updates * add tests for assetdetails * increase timeout for flakey test * pr comments * refresh account history every time account balances refresh * check for updated appdata before showing password modal (#2300) * check for updated appdata before showing password modal * update error msg * rm unused redirect logic * stringify errors rather than using `cause` (#2302) * Feature/move icons to own hook (#2308) * move get icons out of critical path; rely on background's cache * add tests and comments * add comment * add comment * only dispatch if we have cached icons * PR comments * skip blockaid scan on first fetch of account-balances (#2310) * skip blockaid scan on first fetch of account-balances * rm stub change * rm more stubs * rm log * add comments and update boolean naming * Dropdown menu option to copy wallet address (#2316) * add button to copy address from dropdown * Added translations * revert translation file changes * revert translation file changes * Added translations * revert changes to translation files * move copy address button to first dropdown position * scroll on long strings; pretty print json (#2320) * scroll on long strings; pretty print json * rm log * add correct snapshot for json message * rm log * finish comment * add error case for JSON * don't use carat for lib * update yarn.lock * move scrollbar to btm of container; reduce json font size * update snapshot * re-searching so should abort any in flight API requests (#2323) * re-searching so should abort any in flight API requests * add comment * fix test name * make test more reliable * add check for correct search results * fix jest locator * [FEATURE] new send/swap navigation flow (#2353) * adds SelectionTile and AddressTile, updates nav flows to match updates. Adds query parameter for default values in send flow * Added translations * adds address tile and uses it in swap flow, tweaks selection tile styles * adds unit tests for new tile components * Added translations * updates swap navigation flow to match updates, updates tests flows to match * updates back icon for send and swap steps, fixes bad test references * tweaks locator in address tile tests * adds store state to asset tile tests, removes asset icon mock * updates SelectionTile prop name, adds isSuspicious prop for AssetTile * adds placeholder value in TokenList for missing token USD value * uses real IdenticonImg in address tile unit tests * adds query param validation for send and swap flow * Update extension/src/popup/views/SendPayment/index.tsx Co-authored-by: Cássio Marcos Goulart <[email protected]> * adds missing import * adds class for tile icon --------- Co-authored-by: Cássio Marcos Goulart <[email protected]> * [FEATURE] adds send and swap buttons to asset detail view (#2351) * adds send and swap buttons to asset detail view * uses secondary button styles * removes run snapshots job (#2355) * release/5.35.4 (#2354) * upgrade to ledger-hq/hw-transport-webhid (#2350) * upgrade to ledger-hq/hw-transport-webhid * add tests * add ledger support for new trustline flow (#2352) * upgrade to ledger-hq/hw-transport-webhid * add ledger support for new trustline flow * only re-fetch balances if we were successful * test for fetching balances on success * add reset spys * adjust spacing at top of hw wallet modal * Now that `Done` button properly shows, click it in tests (#2356) * skip flakey test * skip flakey test * renames local vars to follow convention * adds tests for LP share and tweaks LP title * adds links with query params for asset detail CTAs --------- Co-authored-by: Piyal Basu <[email protected]> * only fetch asset list data if needed (#2369) * only fetch asset list data if needed * correctly show icon loading state * [BUG] SAC token management improvements (#2374) * adds SAC detection when changing trust in the add and remove token flows * updates arg signature for isAssetSac * Feature/cache token prices (#2373) * cache token prices and batch loading wallets * use similar methodology for token price and account balance caching * fix loading state trigger * fix tests * use helper for cache clearing * set isFetchingTokenPrices to false in catch handler * rollback error change * load backend settings async on Account view (#2381) * load backend settings async on Account view * rm console.logs * Feature/use ledger key for home domains (#2363) * use ledger-key/accounts endpoint for home domains * fix tests * create generic ledger key account helper; add tests * rm unneeded data-test prop * rm unused import * [CHORE] git process updates (#2361) * moves the add translations hook to the pre commit stage, removes standalone translations commit * adds script to update app version, removes version update from submit production action, adds update version step to test run action for release branches * removes version input, now uses package version * fetch asset domains in one calls * fix tests * cache home domains while iterating over account history rows * fitler non-G keys * rm .only * fix test param * PR comments --------- Co-authored-by: aristides <[email protected]> * update version numbers for release * rm unnecessary calls to make flows even faster (#2391) * makes send swap buttons stay in the container in full screen mode (#2392) * makes send swap buttons stay in the container in full screen mode * add a pause to make sure flakey e2e test has time to save changes * use redux selector for allAccounts to properly update rename (#2403) * use redux selector for allAccounts to properly update rename * add longer timeout for flakey btn * Add docs for 5.36.0 (#2408) --------- Co-authored-by: leofelix077 <[email protected]> Co-authored-by: aristides <[email protected]> Co-authored-by: Cássio Marcos Goulart <[email protected]>

add issuer for changeTrust op

6900852

piyalbasu commented Sep 11, 2025

View reviewed changes

piyalbasu mentioned this pull request Sep 11, 2025

Show asset issuer in changeTrust ops in Sign Transaction stellar/freighter-mobile#363

Open

aristidesstaffieri approved these changes Sep 12, 2025

View reviewed changes

programmatically disable overflow:hidden when copying a value

7be46f8

piyalbasu commented Sep 12, 2025

View reviewed changes

piyalbasu merged commit 19c8a68 into bugfix/rm-auth-param-names Sep 12, 2025
4 checks passed

piyalbasu deleted the bugfix/add-issuer-for-changetrust branch September 12, 2025 19:07

piyalbasu added a commit that referenced this pull request Sep 12, 2025

Revert "add issuer for changeTrust op (#2246)"

084d2b6

This reverts commit 19c8a68.

piyalbasu added a commit that referenced this pull request Sep 12, 2025

Revert "add issuer for changeTrust op (#2246)" (#2247)

f927cda

This reverts commit 19c8a68.

piyalbasu restored the bugfix/add-issuer-for-changetrust branch September 12, 2025 19:11

piyalbasu mentioned this pull request Sep 12, 2025

Bugfix/add issuer for changetrust #2249

Merged

Keeeeeeeks mentioned this pull request Sep 22, 2025

Can't identify asset being trusted in change trust operation #2194

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

add issuer for changeTrust op #2246

add issuer for changeTrust op #2246

Uh oh!

piyalbasu commented Sep 11, 2025

Uh oh!

piyalbasu Sep 11, 2025

Uh oh!

aristidesstaffieri Sep 12, 2025

Uh oh!

piyalbasu Sep 12, 2025

Uh oh!

piyalbasu Sep 12, 2025

Uh oh!

aristidesstaffieri Sep 12, 2025

Uh oh!

piyalbasu Sep 11, 2025

Uh oh!

piyalbasu Sep 11, 2025

Uh oh!

piyalbasu Sep 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

add issuer for changeTrust op #2246

add issuer for changeTrust op #2246

Uh oh!

Conversation

piyalbasu commented Sep 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants