Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Incoming transactions #164

Closed
lukasschor opened this issue Sep 10, 2019 · 16 comments
Closed

Incoming transactions #164

lukasschor opened this issue Sep 10, 2019 · 16 comments
Assignees
Labels
Feature 👑 New functionality Major Needs to be fixed for immediate next public release.

Comments

@lukasschor
Copy link
Member

lukasschor commented Sep 10, 2019

What is this feature about? (1 sentence)

Display incoming transactions and notify users.

Why is it needed? What is the value? For whom do we build it?

Currently, incoming transactions are not shown, this is a major UX flaw as incoming transactions are expected from users.

High-level overview of the feature

  • Show incoming transactions in the transaction list
  • Notification when there is an incoming transaction "Incoming transfer: XXX ETH" (The design does not show the ":" but it should be included")
  • Icons in the Tx List to differentiate transaction types

Screenshot 2019-09-26 at 12.04.14.png
Screenshot 2019-09-26 at 12.06.36.png

Backend Endpoint

safe-global/safe-transaction-service#22

@lukasschor lukasschor added this to the Safe for Teams v2.0.0 (Public Beta) milestone Sep 10, 2019
@lukasschor lukasschor self-assigned this Sep 10, 2019
@lukasschor lukasschor added the Critical Only for bugs in released apps, needs to be fixed asap and hotfix needs to be shipped. label Sep 10, 2019
@posthnikova
Copy link

Incoming tx in the list and notification: https://invis.io/C8TFLRFU4ZV#/384309988_28_Transactions_-incoming-_

Expanded view: https://invis.io/C8TFLRFU4ZV#/384309989_32_Transactions_-_Expand_-incoming-_

Expanded view with known address: https://invis.io/C8TFLRFU4ZV#/384309991_33_Transactions_-_Expand_-incoming_Known_Address-_

image.png

In the initial design there was a pending label, I'm not sure if we display pending incoming transactions.

@KristinaMayman
Copy link
Member

I know this is not the same as the mobile app, but in the mobile app I like that we have an easy visual way to scan the tx list and immediately tell which txs were incoming and which were outgoing (with the coloured arrows). We already have the Success/Pending/Failed part as the colour focus but I wonder if there was a way to indicate the difference between outgoing and incoming in any other way than text... Would icons clutter the list too much?

@lukasschor
Copy link
Member Author

Agree. It would be good to differentiate transactions visually. Maybe even also inidicate which transactions are smart contract interactions or settings changes.

@posthnikova
Copy link

@lukasschor @KristinaMayman I added icons to the list https://invis.io/C8TFLRFU4ZV#/384309988_28_Transactions_-incoming-_. There is no commonly used icon for custom transactions. I used the same settings icon like in "Settings change".

@lukasschor
Copy link
Member Author

Looks good. Another icon that might also work for custom transactions is something like this
Screenshot 2019-09-26 at 11 40 45
But I think it's fine if we use the gear icon.

On that note, I think Zerion is doing a good job displaying information in the transaction list. Might be worth taking some inspiration from them in the future. Especially, how to provide additional information about Protocol/integrations interactions (Moonpay, Compound).
IMG_7889
IMG_7891
IMG_7890

@KristinaMayman
Copy link
Member

KristinaMayman commented Sep 26, 2019

I would also prefer if we found a separate icon for custom transactions, just to avoid confusion. Maybe this one can also be an option?
puzzle (1).png

Agree on Zerion, I heard some good feedback about their tx list during the DappCon tests.

@posthnikova
Copy link

@lukasschor @KristinaMayman I have many options.

image.png

image.png

image.png

image.png

I prefer the "<>" one.

@KristinaMayman
Copy link
Member

Thanks for showing all the options! I agree the "<>" one works best.

@posthnikova
Copy link

New icon is on zeplin: https://zpl.io/2vPnBm7

@lukasschor lukasschor added the Feature 👑 New functionality label Oct 1, 2019
@lukasschor lukasschor reopened this Oct 16, 2019
@lukasschor lukasschor added Major Needs to be fixed for immediate next public release. and removed Critical Only for bugs in released apps, needs to be fixed asap and hotfix needs to be shipped. labels Oct 16, 2019
@fernandomg fernandomg self-assigned this Nov 26, 2019
@lukasschor lukasschor removed their assignment Nov 28, 2019
@fernandomg
Copy link
Contributor

@lukasschor, is there an endpoint in safe-relay that returns the incoming Transfers for the different tokens?

Or shall we poll for incoming Transfers in the UI only?

Given this last scenario, we will have two different sources of truth (safe-relay and the blockchain itself), may this lead to an inconsistent state that you can think of?

Finally, if we go with the polling for incoming transfers approach, shall we persist those in the safe-relay as we are doing with the outgoing txs?

@mmv08
Copy link
Member

mmv08 commented Dec 4, 2019

@fernandomg there's an endpoint in transaction history service: https://safe-transaction.staging.gnosisdev.com/

@fernandomg
Copy link
Contributor

fernandomg commented Dec 4, 2019

@mikheevm, thanks! 🤦‍♂️

@fernandomg
Copy link
Contributor

@mikheevm, from what I can see in the response for both endpoints (incoming-transactions and transactions) the only way to merge and sort both structures is by querying the blockNumber's timestamp from the blockchain.

Is there a way that the incoming-transaction endpoint returns that value already? So I can use it to compare it with the date in the transactions' returned object

@mmv08
Copy link
Member

mmv08 commented Dec 4, 2019

@fernandomg let's ask @Uxio0 if adding a timestamp to the backend is easy. I wouldn't like to do it on the frontend because we poll txs every 5s and querying timestamps every 5s isn't the best thing to do.

@Uxio0
Copy link
Member

Uxio0 commented Dec 5, 2019

@fernandomg @mikheevm I sorting is the only thing you need, I can just return the blockNumber in the transactions returned objects

@Uxio0
Copy link
Member

Uxio0 commented Dec 5, 2019

I already added the blockNumber

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature 👑 New functionality Major Needs to be fixed for immediate next public release.
Projects
None yet
Development

No branches or pull requests

6 participants