-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce calculateAmountReceived latency #3905
Comments
Complexity: 13 Powered by Parabol |
Urgency: 4 Powered by Parabol |
Urgency: 3 Powered by Parabol |
Complexity: 21 Powered by Parabol |
Impact: 4 Powered by Parabol |
Urgency: 4 Powered by Parabol |
Complexity: 21 Powered by Parabol |
Impact: 5 Powered by Parabol |
Urgency: 3 Powered by Parabol |
@alexwhte how can i get these metrics on my local side? |
@liu-zhipeng I used performance.mark() API to get these splits within the function. |
Working on bypassing RPC call for calculating amount received. |
Fixing coverage of tests. |
Ready for merge into main. |
Problem
Connext is not even showing in results 30% of the time (est) because our API takes too long
See analysis in here #3895
Based on these initial profiling results, calculateSwap is the biggest contributor of latency. We call it (up to) 2 times per call, once to calculate origin swap and once to calculate destination swap.
In the calculateSwap function, the direct contract call here contains the majority of the latency:
const minAmount = await connextContract.calculateSwap(key, tokenIndexFrom, tokenIndexTo, amount);
Impact
25% of our volume last week came from LiFi alone.
We are recommended 40% of the time when shown, but only even a shown option 70% of the time
CalculateSwap is involved in 1665 ms (63%) of the latency
Proposed Solution
How well do you think a cache would do here, that updates by reading the contract multiple times a second?
NOTE: Need to decide which calls we optimize for if not just
calculateSwap
Acceptance Criteria
From an end user perspective, the below criteria must be met to consider this done:
The text was updated successfully, but these errors were encountered: