Safe-react: Release 3.4.0 full regression issues

[liliya-soroka]

• [ ] Known address v1 - broken image is displayed if no logo for know address on back-end - https://app.zenhub.com/workspaces/safe-multisig---web-5ce554debb310a35b2f8b6f8/issues/gnosis/safe-react/2133

• [ ] Known address v1: mix of data from known address list and address book - https://app.zenhub.com/workspaces/safe-multisig---web-5ce554debb310a35b2f8b6f8/issues/gnosis/safe-react/2135 Note: addressbook has higher priority than known addressbook always

• [x] The app is crashed - https://app.zenhub.com/workspaces/safe-multisig---web-5ce554debb310a35b2f8b6f8/issues/gnosis/safe-react/2136

• [x] Tx failed with default gas values in xDai - https://app.zenhub.com/workspaces/safe-multisig---web-5ce554debb310a35b2f8b6f8/issues/gnosis/safe-react/2137. - won't fix

• [x] Gas limit is inconsistent right now https://github.com/gnosis/safe-react/issues/2146 - the fix on back-end was done

[francovenica]

I think the issue "Known address v1: mix of data from known address list and address book" is the same case as I addressed in this comment
https://app.zenhub.com/workspaces/safe-multisig---web-5ce554debb310a35b2f8b6f8/issues/gnosis/safe-react/2113#issuecomment-811458931

[liliya-soroka]

@francovenica , yes, agree with you and you was completely right in the comment, but :

1. the PR was closed without solving it
2. The addressbook goes first - even on the transactions list the safe name should be used instead of the name and logo from known addresses.

[nicosampler]

@liliya-soroka

Known address v1 - broken image is displayed if no logo for know address on back-end

This one is not a bug, we did that on purpose, there is a discussion on the issue.

[nicosampler]

@liliya-soroka

Known address v1: mix of data from known address list and address book

The same for this one.

[tschubotz]

@nicosampler

Known address v1 - broken image is displayed if no logo for know address on back-end
This one is not a bug, we did that on purpose, there is a discussion on the issue.

The backend provides an image in reallly all cases, i.e. the image icon is a required field which is not nullable in the ABI?
I tried to find the discussion but couldn't find it in #1638 - could you link me to it pls?

Known address v1: mix of data from known address list and address book
The same for this one.

In the ticket desciption of #1638 it says "Local address book names should overrule the human readable name that comes from the backend." hence I would expect this. Could you elaborate why this was done differently?

[nicosampler]

@tschubotz

1. https://github.com/gnosis/safe-react/pull/2113#issuecomment-812151859

1. https://github.com/gnosis/safe-react/pull/2113#issuecomment-811916591

[tschubotz]

Ah, in the PR, didn't check there, thanks for linking.

1. [#2113 (comment)](https://github.com/gnosis/safe-react/pull/2113#issuecomment-812151859)

Hmm, in our backend the logo is optional, right @Uxio0? I think the frontend needs to check if the logo exists to be honest. And if it desn't, we need the <> placeholder.

2\. [#2113 (comment)](https://github.com/gnosis/safe-react/pull/2113#issuecomment-811916591)

I don't get why we would now build it differently on mobile and web then to be honest. But seems like this is expected then 🤷

[liliya-soroka]

@tschubotz , just as a note to align two platforms, on mobile it was agreed to show address indeticon + known address name if the address is located in the known address list without logo (transactions list and on transaction details )

[tschubotz]

if the address is located in the known address list without logo

Now I'm complete lost :D

You mean "if the address is located in the address book without logo"?

[liliya-soroka]

I mean "Known address" list on back-end in Django administration (the logo is optional , only the name is required )

[nicosampler]

Hmm, in our backend the logo is optional, right @Uxio0? I think the frontend needs to check if the logo exists to be honest. And if it desn't, we need the <> placeholder.

if not logo was provided by the backend, it will show the identicon, but if the logo was provided and the link is broken, we are not setting a placeholder. I think that if the image is broken, we should report it, not patch it.

[tschubotz]

if not logo was provided by the backend, it will show the identicon, but if the logo was provided and the link is broken, we are not setting a placeholder. I think that if the image is broken, we should report it, not patch it.

So you are saying the client gateway should just not return a logoURI in that case ,right?

@jpalvarezl @rmeissner Is this how the client gateway is built? i.e. it would only return the logoURI if there is actually a logo for the known address present?

(Example is nonce 442 on this Safe: https://safe-client.rinkeby.staging.gnosisdev.com/v1/safes/0x1230B3d59858296A31053C1b8562Ecf89A2f888b/transactions/history)

[nicosampler]

no, perhaps the URL exists but is broken for some reason. Like it happens on Rinkeby for some images, there is a link, but when you open that link it returns a 503 I think.

[tschubotz]

It returns a 403. And that's normal AWS behavior also when the file doesn't exist afaik.

[nicosampler]

sorry, it's a 403.

This is an example, on Rinkeby the backend returns this image: https://gnosis-safe-token-logos.s3.amazonaws.com/0x9Ea007843318B9EcD85f93eCC55D4e19143f007A.png and it is broken.

So, if we face that scenario, what should we do?
a. omit the error and use the identicon (we will hide the error)
b. render the broken image as it is now (on mainnet this error is no common at all)

[tschubotz]

This is an example, on Rinkeby the backend returns this image: https://gnosis-safe-token-logos.s3.amazonaws.com/0x9Ea007843318B9EcD85f93eCC55D4e19143f007A.png and it is broken.

I'm not sure what you mean by "broken" - it's simply not available, right? :)

So either the client gateway needs to not return the logo url when there is no image. Or the frontend needs to filter it out. That's the 2 options I see. Hence my question, on how the client gateway does it.

[Uxio0]

There's a difference between contract logos and token logos on the core services, as they were implemented in a different way:

• contract logoUri is null if not logo was set.
• token logoUri will always return an address, even if file does not exist (will get a 403 from S3).

@jpalvarezl @rmeissner I think that could be the problem with some contracts having logos that are not "valid"

[tschubotz]

So, if we face that scenario, what should we do?
A. omit the error and use the identicon (we will hide the error)
B. render the broken image as it is now (on mainnet this error is no common at all)

I think we should go with option A since

* if file does not exist (will get a 403 from S3).

As Uxio said.

The images aren't "broken" as in corrupted. But they are just not existing.

[tschubotz]

There are separate issues open for the 2 open questions. -> closing