Skip to content

feat: add api v2 support#287

Open
tomiiide wants to merge 19 commits intomainfrom
LF-10832-add-v2-api-support
Open

feat: add api v2 support#287
tomiiide wants to merge 19 commits intomainfrom
LF-10832-add-v2-api-support

Conversation

@tomiiide
Copy link
Copy Markdown
Contributor

This PR:

  1. closes https://lifi.atlassian.net/browse/LF-10832
  2. closes https://lifi.atlassian.net/browse/LF-10833
  3. adds support for the API V2 endpoints.
  4. updates the analytics endpoint to use the v2 endpoint

Why was it implemented this way

  • The config.apiUrl type is updated to string | { v1: string, v2 : string} , this way is it easy to extend. For example if we have a v3 API url, we just add it to the config.
  • A new getApiUrl(version) getter method is added to the config for safe and dynamic fetching of the apiUrl. It returns the url for different API versions. For backward compatibility, when the config.apiUrl is non versioned, and set to a string, it is returned by default.

@tomiiide tomiiide self-assigned this Jul 24, 2025
@tomiiide tomiiide requested a review from chybisov July 24, 2025 10:18
@effie-ms effie-ms self-requested a review July 24, 2025 11:40
@tomiiide tomiiide requested review from chybisov and effie-ms August 7, 2025 09:17
options?: RequestOptions
): Promise<TransactionAnalyticsResponse> => {
): Promise<PaginatedResponse<StatusResponse[]>> => {
if (!wallet) {
Copy link
Copy Markdown

@yolodevz yolodevz Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomiiide I know that's not part of the PR, but is this check actually correct? Wallet is optional. Recent transactions list call (visible on the main explorer page) pulls the recent transactions, while wallet specific ones are visible after we search by wallet.

ie. If we don't provide the wallet, it's still fine, and even necessary for the explorer homepage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yeah, this would break when we are just fetching all recent transactions.

@chybisov it makes sense to remove this check then no?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove that check if that's okay for the API. Please check within the widget as well, maybe we should move it there to avoid making requests when wallet is not connected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants