search

safe-global / safe-wallet-web

Safe{Wallet} – multisig EVM wallet
https://app.safe.global
GNU General Public License v3.0
337 stars 401 forks source link

[EIP-1271] Poll messages according to `messagesTag` #1403

Closed iamacook closed 9 months ago

iamacook commented 1 year ago

What is the feature about

Safe info is currently cached on the backend and, as such, the messagesTag does not update accordingly. Temporary polling logic was implemented that needs to be reverted once the caching has been solved.

The list of requirements

Reversion of https://github.com/safe-global/web-core/pull/1402.

Links

iamacook commented 1 year ago

I'm going to move this out of the Todo column for now as the backend blocker has yet to be worked on.

katspaugh commented 1 year ago

messagesTag is available in SafeInfo now.

Screenshot 2023-05-15 at 14 18 31
iamacook commented 1 year ago

I just tested this and it doesn't seem to update. @fmrsabino, is the result of get_last_modified_mesage cached?

Maybe we should hold off on this until migrating to the new gateway.

katspaugh commented 1 year ago

Oh, I see. It's been available all along, but cached.

fmrsabino commented 1 year ago

I just tested this and it doesn't seem to update. @fmrsabino, is the result of get_last_modified_mesage cached?

Maybe we should hold off on this until migrating to the new gateway.

Requesting the last modified message is not cached by itself but the result returned by the endpoint GET /v1/chains/<chain_id>/safes/<safe_address> is cached (1h).

Unfortunately there are no hooks to invalidate the Safe Info when messages get updated. This means that you will only see an update if Safe Info was invalidated by other means (transaction queued, executed, etc...).

Is this blocking you? As far as I know no hooks will be implemented from Safe{Core} at the moment but on our side we can adjust the caching requirements.

iamacook commented 1 year ago

Is this blocking you? As far as I know no hooks will be implemented from Safe{Core} at the moment but on our side we can adjust the caching requirements.

This isn't blocking, it's more of an optimisation.

We are currently making a new request for messages every 15s and wanted to rely on the tag instead. I don't see an issue with it as is but it would be nice to reduce this.

fmrsabino commented 1 year ago

Is this blocking you? As far as I know no hooks will be implemented from Safe{Core} at the moment but on our side we can adjust the caching requirements.

This isn't blocking, it's more of an optimisation.

We are currently making a new request for messages every 15s and wanted to rely on the tag instead. I don't see an issue with it as is but it would be nice to reduce this.

We can tackle this in the Safe Client Gateway. It would be a matter of adjusting the cache timeouts (we can reduce them as long as it does not impact the Safe Transaction service in a negative way).

iamacook commented 1 year ago

It would be a matter of adjusting the cache timeouts (we can reduce them as long as it does not impact the Safe Transaction service in a negative way).

If there's the potential for a negative effect then I think we can leave it as is, or do you think the frequest polling is worse?

fmrsabino commented 1 year ago

It would be a matter of adjusting the cache timeouts (we can reduce them as long as it does not impact the Safe Transaction service in a negative way).

If there's the potential for a negative effect then I think we can leave it as is, or do you think the frequest polling is worse?

Ideally there would be no polling but I think this is a bigger discussion that will involve more changes.

I'd not change the polling right now on the web side. I'd try to address it in the backend first (figuring out what a good cache-policy might be for the safe info) before we try to address the polling on the frontend side.

katspaugh commented 1 year ago

Since it seems it won't be worked on by the backend, I'll close this.

iamacook commented 1 year ago

Since it seems it won't be worked on by the backend, I'll close this.

I added this to the forthcoming NestJS gateway.

fmrsabino commented 1 year ago

Since it seems it won't be worked on by the backend, I'll close this.

We no longer cache payloads returned to the clients on the Safe Client Gateway. This means that the Safe Info is always computed on each request.

We do have a caching layer when interacting with services such as the Safe Transaction Service but we do not cache Safe Messages so you will always get an updated messagesTag.