Move price registry sync to start of Observation#586
Conversation
| } | ||
|
|
||
| // Ensure that the source price registry is synchronized with the onRamp. | ||
| if err = r.ensurePriceRegistrySynchronization(ctx); err != nil { |
There was a problem hiding this comment.
Mentioned that in a previous PR so copypasting :D
in that case prepareTokenExecData and eventually ensurePriceRegistrySynchronization are never called.
@dimkouv @jarnaud why is that a problem? At this moment, the source price registry is used only in a single place, so what's the point of eagerly synchronizing that? I understand that in case of adding any other reference to the source price registry might end up trying to call a stale object which is bad. On the other hand, we will keep refreshing cache with every observation even if we don't need to do anything with tokens, but maybe this is not a bad thing
There was a problem hiding this comment.
It's not really problem, but in order to figure this out (if there's a problem) I had to look at the call stack to make sure the price registry is not used before the ensurePriceRegistrySynchronization call.
Results are cached so I believe it's good to make the logic cleaner to avoid future bugs.
3 participants
Motivation
Solution