Skip to content
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

Rework token prices #6925

Merged
merged 21 commits into from Mar 22, 2023
Merged

Rework token prices #6925

merged 21 commits into from Mar 22, 2023

Conversation

sl1depengwyn
Copy link
Collaborator

@sl1depengwyn sl1depengwyn commented Feb 17, 2023

Close #6547
Close #6855

Motivation

It happened that now we have no token price feature in master branch. Also, we sorted token balances by value that is not very useful for user. Additionally, we had issue of paginating balances of ERC-1155 or ERC-721 tokens when user have the same amount of different token ids of the same token

Changelog

To be able to sort and paginate token balances I implemented new token balance fetcher, changed pagination params for token balances, removed redundant functionality like manual sorting of token balance after fetching from the database since now the are fetching already in right order. Also, I removed KnownTokens module and functionality as new token price fetched does not use it.

Docs update blockscout/docs#117

Address page tokens before:
image
after:
image

Top tokens page before:
image

after:
image

Checklist for your Pull Request (PR)

@sl1depengwyn sl1depengwyn changed the title Mf sort token balances by value Rework token prices Feb 17, 2023
@sl1depengwyn sl1depengwyn marked this pull request as ready for review February 17, 2023 19:04
config/runtime.exs Outdated Show resolved Hide resolved
config/runtime.exs Outdated Show resolved Hide resolved
config/runtime.exs Outdated Show resolved Hide resolved
config/runtime.exs Outdated Show resolved Hide resolved
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

@sl1depengwyn does it make sense to display price on "top tokens page" /tokens as well and make the same default sorting?

@sl1depengwyn
Copy link
Collaborator Author

@sl1depengwyn does it make sense to display price on top tokens page /tokens as well and make the same default sorting?

You mean sort by total supply * token price? In this case I think it make sense

@vbaranov
Copy link
Member

@sl1depengwyn does it make sense to display price on top tokens page /tokens as well and make the same default sorting?

You mean sort by total supply * token price? In this case I think it make sense

yes

@sl1depengwyn sl1depengwyn force-pushed the mf-sort-token-balances-by-value branch 3 times, most recently from f0adee0 to 96f63be Compare March 7, 2023 08:36
@sl1depengwyn sl1depengwyn force-pushed the mf-sort-token-balances-by-value branch from e67673f to 0e3ab05 Compare March 7, 2023 16:06
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

/verified-contracts page doesn't open with the changes

Request: GET /verified-contracts?type=JSON
** (exit) an exception was raised:
    ** (KeyError) key :market_cap not found in: %Explorer.Chain.Token{__meta__: #Ecto.Schema.Metadata<:loaded, "tokens">, name: "Seizon", symbol: "Seizon", total_supply: #Decimal<7573>, decimals: nil, type: "ERC-721", cataloged: true, holder_count: 3362, skip_metadata: nil, total_supply_updated_at_block: nil, fiat_value: nil, circulating_market_cap: nil, contract_address_hash: %Explorer.Chain.Hash{byte_count: 20, bytes: <<166, 205, 39, 40, 116, 238, 124, 135, 46, 182, 104, 1, 239, 246, 39, 132, 192, 177, 50, 133>>}, contract_address: #Ecto.Association.NotLoaded<association :contract_address is not loaded>, inserted_at: ~U[2023-03-06 23:59:07.423871Z], updated_at: ~U[2023-03-08 11:53:03.304706Z]}
        (block_scout_web 5.1.1) lib/block_scout_web/templates/verified_contracts/_contract.html.eex:58: BlockScoutWeb.VerifiedContractsView."_contract.html"/1
        (phoenix 1.5.13) lib/phoenix/view.ex:472: Phoenix.View.render_to_iodata/3
        (phoenix 1.5.13) lib/phoenix/view.ex:479: Phoenix.View.render_to_string/3
        (block_scout_web 5.1.1) lib/block_scout_web/controllers/verified_contracts_controller.ex:27: anonymous fn/2 in BlockScoutWeb.VerifiedContractsController.index/2
        (elixir 1.14.0) lib/enum.ex:2468: Enum."-reduce/3-lists^foldl/2-0-"/3
        (block_scout_web 5.1.1) lib/block_scout_web/controllers/verified_contracts_controller.ex:26: BlockScoutWeb.VerifiedContractsController.index/2
        (block_scout_web 5.1.1) lib/block_scout_web/controllers/verified_contracts_controller.ex:1: BlockScoutWeb.VerifiedContractsController.action/2
        (block_scout_web 5.1.1) lib/block_scout_web/controllers/verified_contracts_controller.ex:1: BlockScoutWeb.VerifiedContractsController.phoenix_controller_pipeline/2

@sl1depengwyn
Copy link
Collaborator Author

/verified-contracts page doesn't open with the changes

Request: GET /verified-contracts?type=JSON
** (exit) an exception was raised:
    ** (KeyError) key :market_cap not found in: %Explorer.Chain.Token{__meta__: #Ecto.Schema.Metadata<:loaded, "tokens">, name: "Seizon", symbol: "Seizon", total_supply: #Decimal<7573>, decimals: nil, type: "ERC-721", cataloged: true, holder_count: 3362, skip_metadata: nil, total_supply_updated_at_block: nil, fiat_value: nil, circulating_market_cap: nil, contract_address_hash: %Explorer.Chain.Hash{byte_count: 20, bytes: <<166, 205, 39, 40, 116, 238, 124, 135, 46, 182, 104, 1, 239, 246, 39, 132, 192, 177, 50, 133>>}, contract_address: #Ecto.Association.NotLoaded<association :contract_address is not loaded>, inserted_at: ~U[2023-03-06 23:59:07.423871Z], updated_at: ~U[2023-03-08 11:53:03.304706Z]}
        (block_scout_web 5.1.1) lib/block_scout_web/templates/verified_contracts/_contract.html.eex:58: BlockScoutWeb.VerifiedContractsView."_contract.html"/1
        (phoenix 1.5.13) lib/phoenix/view.ex:472: Phoenix.View.render_to_iodata/3
        (phoenix 1.5.13) lib/phoenix/view.ex:479: Phoenix.View.render_to_string/3
        (block_scout_web 5.1.1) lib/block_scout_web/controllers/verified_contracts_controller.ex:27: anonymous fn/2 in BlockScoutWeb.VerifiedContractsController.index/2
        (elixir 1.14.0) lib/enum.ex:2468: Enum."-reduce/3-lists^foldl/2-0-"/3
        (block_scout_web 5.1.1) lib/block_scout_web/controllers/verified_contracts_controller.ex:26: BlockScoutWeb.VerifiedContractsController.index/2
        (block_scout_web 5.1.1) lib/block_scout_web/controllers/verified_contracts_controller.ex:1: BlockScoutWeb.VerifiedContractsController.action/2
        (block_scout_web 5.1.1) lib/block_scout_web/controllers/verified_contracts_controller.ex:1: BlockScoutWeb.VerifiedContractsController.phoenix_controller_pipeline/2

Should be fixed in adb5d7a

dynamic(
[ctb, t],
is_nil(^fiat_balance) and
(t.type > ^type or
Copy link
Member

Choose a reason for hiding this comment

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

Why did you decide to remove ordering by value?

@sl1depengwyn sl1depengwyn force-pushed the mf-sort-token-balances-by-value branch from adb5d7a to f014491 Compare March 16, 2023 10:04
@nikitosing nikitosing force-pushed the mf-sort-token-balances-by-value branch from f014491 to adb5d7a Compare March 16, 2023 12:43
@sl1depengwyn sl1depengwyn force-pushed the mf-sort-token-balances-by-value branch 2 times, most recently from 622843e to cdc40d0 Compare March 16, 2023 19:29
@vbaranov vbaranov self-requested a review March 20, 2023 16:05
@vbaranov vbaranov temporarily deployed to Tests March 20, 2023 18:54 — with GitHub Actions Inactive
@nikitosing nikitosing temporarily deployed to Tests March 22, 2023 11:16 — with GitHub Actions Inactive
@vbaranov vbaranov merged commit f3bb60f into master Mar 22, 2023
19 checks passed
@vbaranov vbaranov deleted the mf-sort-token-balances-by-value branch March 22, 2023 11:32
fx0x55 pushed a commit to FunctionX/blockscout that referenced this pull request May 10, 2023
* Drop `KnownTokens` and `TokenExcahngeRate` modules

* Market related: add request and refactor config

* Add `TokenExchangeRates` module

* `:usd_value` -> `:fiat_value`

* Update `Token`  and `CurrentTokenBalance` modules

* Drop manual adding of price to tokens

* Update pagination in the web app

* Drop manual sorting of token balances

* Update doc for exchange_rates

* Fix credo and format

* Fix tests

* Add envs

* Update CHANGELOG.md

* Add regression test for token balance pagination

* Fix format and update .dialyzer-ignore

* Fix some typos

[no ci]

* Sort tokens by market cap

[no ci]

* `market_cap` -> `circulating_market_cap`

* Display circulating market cap on top tokens page

[no ci]

* Update tests

* Fix review

(cherry picked from commit f3bb60f)
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.

Address tokens pagination issue Address page -> tokens tab: sort items by value DESC, name ASC by default
3 participants