feat(ui): #2053: implement SwapClaim action view
#2061
Conversation
|
Visit the preview URL for this PR (updated for commit 59a15d9): https://penumbra-ui-preview--pr2061-feat-2053-action-vi-khqajj28.web.app (expires Wed, 26 Feb 2025 15:22:35 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 709d729610ef7a6369b23f1cb2b820a60cc685b1 |
There was a problem hiding this comment.
requested some changes – the data we current display in our deprecated UI library for visible and opaque swap claims is correct, and the new actions should be modeled accordingly, rather than following the figma design. In this instance, we should only perform a cosmetic change rather than any kind of data modeling for swap claims.
| outputData: { | ||
| tradingPair: { | ||
| asset1: USDC_METADATA.penumbraAssetId, | ||
| asset2: PENUMBRA_METADATA.penumbraAssetId, | ||
| }, | ||
| lambda1: AMOUNT_123_456_789, | ||
| lambda2: AMOUNT_999, |
There was a problem hiding this comment.
comment: privacy leak here I think? referencing the fourth bullet point in #889, we should only display the swap claim fee for SwapClaim actions in opaque views. We can also extend this to display in the visible view for consistency, since the swap claim already links to the associated swap in the user's visible view — see the attached example
Screen.Recording.2025-02-18.at.9.02.53.AM.mov
There was a problem hiding this comment.
@TalDerei thanks for the reference to another issue – i understand the context now.
Question: how not displaying the input/output amounts would leak the information having that this data is still in the protobuf message?
There was a problem hiding this comment.
I guess I don't fully understand the idea after re-reading the comment for a 10th time. When talking about cosmetic changes, do you mean we shouldn't display the input/output amounts, or is it only about the fees?
There was a problem hiding this comment.
In this context, cosmetic changes mean that we should display the same data as the current deprecated UI, with only styling, typography, etc. updated according to the figma design. Like the data modeling shouldn't change, for instance we shouldn't be adding new opaque (public view) fields that weren't there before.
to clarify, the opaque swap claim view should display only the swap claim fee and I don't think we should display the input / output amounts.
There was a problem hiding this comment.
Yeah, but what's the reason for this?
Like the data modeling shouldn't change, for instance we shouldn't be adding new opaque (public view) fields that weren't there before.
I'm only changing the rendering without touching the fields. If data exists, why can't we display it?
There was a problem hiding this comment.
well okay, the BSOD is part of the opaque swap claim view, however I initally thought displaying that information might be confusing as apposed to just the claimed amount since we also already link to the swap. It's not actually a privacy leak because I had forgotten the BSOD field was part of the swap claim body.
There was a problem hiding this comment.
Yeah there's no privacy issue in the UI here — if the data is there, better to display it (as long as that makes sense in the context of the designs)
| <> | ||
| {!!fee && <ActionRow label='Swap Claim Fee' info={fee} />} | ||
| {!!txId && ( | ||
| <ActionRow label='Swap Claim Transaction' info={shorten(txId, 8)} copyText={txId} /> |
There was a problem hiding this comment.
comment: this should link to the associated swap (rather than swap claim) transaction.
the visible (your view) swap claim views should display the relevant swap transaction, swap claim fee (I like this verbage better than "prepaid claim fee"), and optionally the claimed amount.
and the opaque swap claim view should display only the swap claim fee.
|
@TalDerei Updated the PR, seems to be correct now |
@erwanor @hdevalence can you sanity check #2061 (comment)? |
Relates to #2053
Implements SwapClaim action view. The old version didn't display the opaque view, this one does.
Live: https://penumbra-ui-preview--pr2061-feat-2053-action-vi-khqajj28.web.app/?path=/docs/ui-library-actionview--docs
Before
Now