Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Simulation metrics not captured in the wallet initiated Redesigned Confirmation Send Flow #28369

Open
sleepytanya opened this issue Nov 7, 2024 · 7 comments · May be fixed by #28427
Open
Assignees
Labels
regression-RC-12.7.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team type-bug

Comments

@sleepytanya
Copy link
Contributor

sleepytanya commented Nov 7, 2024

Describe the bug

The properties simulations_sending_assets_total_value, simulations_receiving_assets_total_value and transaction_contract_method are intended to be present in both Anonymous and non-Anonymous events. However, these simulation metrics are not presented for wallet initiated redesigned send flow.

Expected behavior

For a given transaction, we should see the same simulations metrics when doing it with the improved transactions toggle on and when doing it with the improved transactions toggle off.
Simulations metrics should be capture even when the simulation UI is not visible (only exception is if the user had disabled it themselves).

Screenshots/Recordings

Experimental and Developer toggles are ON - simulations metrics are not present:

redesignOn.mov

Experimental and Developer toggles are OFF - simulation metrics are in Anon- and non-Anon events:

redesignOff.mov

Steps to reproduce

  1. Send a transaction with simulation
  2. Send a tx with Redesign Experimental and Developer toggles ON
  3. Observe Transaction Approved and Transaction Approved Anon events in the Segment
  4. Repeat step 2 with toggles OFF
  5. Repeat step 3

Error messages or log output

No response

Detection stage

During release testing

Version

12.7.0

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response

@digiwand
Copy link
Contributor

digiwand commented Nov 8, 2024

hello @sleepytanya! I think the bug is that the simulation is expected to work in the redesign as it is shown in the old confirmation pages using the same flow.

we can observe no "Estimated Changes" section in the redesign here:
Image

For now, I will unassign myself and put this back into refine status to re-refine next week

& updated the title from:
[Bug]: Simulation metrics become visible exclusively when both the Experimental and Developer redesign toggles are turned off
→ [Bug]: Simulation metrics not captured in the wallet initiated Redesigned Confirmation Send Flow

@digiwand digiwand removed their assignment Nov 8, 2024
@digiwand digiwand changed the title [Bug]: Simulation metrics become visible exclusively when both the Experimental and Developer redesign toggles are turned off [Bug]: Simulation is not shown in Redesigned Confirmation Send Flow Nov 8, 2024
@bschorchit
Copy link

bschorchit commented Nov 8, 2024

@digiwand could we still capture the simulation data even if we don't display it to the user? Alternatively, we can display it under advanced view only, but capture the simulation data for both users with advanced view on and users that have it off.

Asking as the send flow is also important for the same leadership ask around metrics.

@digiwand
Copy link
Contributor

digiwand commented Nov 8, 2024

hello @bschorchit 👋🏼 we could. What I would suggest or question is how we'd distinguish between no results returned vs. not showing the simulation. (I'm not exactly sure of the UI/UX behavior here as I'm writing this.)

Updating metrics to indicate whether the user has seen or has not seen the simulation rendered in the advanced view could be a different feature request

@bschorchit
Copy link

That's a good point.

I think we could infer if the user has seen simulations or not from other properties like:
(1) the transaction type and referrer (these will tell us it's a send started within MM) and
(2)the advanced_view property (this will tell whether the user was seeing advanced view or not - this is not working on prod yet, but I believe it's on develop).

With that we can build a dashboard or even custom property directly on Mixpanel to indicate to us whether the simulations UI was visible to users at transaction time.

@bschorchit bschorchit changed the title [Bug]: Simulation is not shown in Redesigned Confirmation Send Flow [Bug]: Simulation metrics not captured in the wallet initiated Redesigned Confirmation Send Flow Nov 11, 2024
@bschorchit bschorchit added the release-blocker This bug is blocking the next release label Nov 11, 2024
@digiwand
Copy link
Contributor

digiwand commented Nov 11, 2024

Hi team,

I believe I have good news. I tested on 12.7.0 using ETH mainnet, with both toggles on, and through a dapp-initiated send to confirm that the simulation metrics work as expected

CleanShot 2024-11-11 at 23 36 22@2x

note:

  • testing on the Sepholia chain did not add the simulation metrics. This may be expected for other chains
  • (example flow in the description) wallet-initiated send transactions do not display simulations on the redesign version. Since simulations are not shown on this flow, it is expected not to see simulation metrics

cc: @bschorchit @pedronfigueiredo @sleepytanya

@digiwand
Copy link
Contributor

As discussed in an internal slack thread, we will capture simulation metrics even when no estimated changes are shown e.g. when we initiate send through the wallet. We will mirror the same metrics that we use when we display "Estimated Changes" "No changes..."
CleanShot 2024-11-11 at 23 52 06@2x

@bschorchit
Copy link

I've updated the issue post discussion on slack. This continues to be a release blocker for the reasons mentioned earlier above.

For a given transaction, we should see the same simulations metrics when doing it with the improved transactions toggle on and when doing it with the improved transactions toggle off.
Simulations metrics should be capture even when the simulation UI is not visible (only exception is if the user had disabled it themselves).

@digiwand digiwand linked a pull request Nov 12, 2024 that will close this issue
7 tasks
@digiwand digiwand self-assigned this Nov 13, 2024
@benjisclowder benjisclowder added the Sev2-normal Normal severity; minor loss of service or inconvenience. label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-RC-12.7.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team type-bug
Projects
Status: To be fixed
Status: To be fixed
Development

Successfully merging a pull request may close this issue.

5 participants