-
Notifications
You must be signed in to change notification settings - Fork 20
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
Leaderboard query #29
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be minor adjustments. Please in the future submit separate PRs there is a lot of seeparatee functionality in this PR
apps/block_scout_web/lib/block_scout_web/resolvers/competitor.ex
Outdated
Show resolved
Hide resolved
(SELECT claims.claimed_address AS address, COALESCE(SUM(value),0) AS value | ||
FROM address_current_token_balances, claims | ||
WHERE address_hash=claims.claimed_address | ||
AND token_contract_address_hash='\\x88f24de331525cf6cfd7455eb96a9e4d49b7f292' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these constants?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the address for stable token. I guess this query will be thrown away after the stake off so it doesn't matter much that it's hard coded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment for what this query does, or at least a link to to documentation
contract_address: token_contract_address_hash, | ||
function_name: "balanceOf", | ||
args: [address_hash], | ||
block_number: block_number | ||
block_number: block_number, | ||
gasprice: "1000000000000000000" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why gasprice here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, it complained about smaller than minimum gas price when reading from some previous blocks. Added issue here celo-org/celo-blockchain#638
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leave a comment here with a reference to the ticket
create(index(:celo_validator_history, [:validator_address, :block_number, :index], unique: true)) | ||
create(index(:celo_validator_history, [:block_number, :index], unique: true)) | ||
|
||
if Mix.env() != :test do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a common pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add an issue for removing these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update to handle new transaction fields
Reading logs that have no associated transaction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a bit more documentation and address all the comments. Given the current time sensitivity, I'll approve the PR, but in the future, please consider smaller PRs
(SELECT claims.claimed_address AS address, COALESCE(SUM(value),0) AS value | ||
FROM address_current_token_balances, claims | ||
WHERE address_hash=claims.claimed_address | ||
AND token_contract_address_hash='\\x88f24de331525cf6cfd7455eb96a9e4d49b7f292' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment for what this query does, or at least a link to to documentation
apps/explorer/lib/explorer/chain.ex
Outdated
ORDER BY score | ||
""") | ||
|
||
# IO.inspect(result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
contract_address: token_contract_address_hash, | ||
function_name: "balanceOf", | ||
args: [address_hash], | ||
block_number: block_number | ||
block_number: block_number, | ||
gasprice: "1000000000000000000" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leave a comment here with a reference to the ticket
@@ -112,16 +114,30 @@ defmodule Explorer.Celo.AccountReader do | |||
end | |||
|
|||
defp fetch_account_data(account_address) do | |||
res = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary res
{function_name, response} | ||
end) | ||
|
||
{:ok, [address]} = res["getAddressForString"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when res does not have the key?
@@ -110,6 +111,23 @@ defmodule Indexer.Block.Fetcher do | |||
struct!(__MODULE__, named_arguments) | |||
end | |||
|
|||
# defp append_logs(extra_logs, %{logs: logs, receipts: receipts}) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
%{accounts: celo_accounts, validators: celo_validators, validator_groups: celo_validator_groups} = | ||
CeloAccounts.parse(logs), | ||
%{token_transfers: normal_token_transfers, tokens: normal_tokens} = TokenTransfers.parse(logs), | ||
# %{token_transfers: fee_token_transfers, tokens: fee_tokens} = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
@@ -152,6 +183,7 @@ defmodule Indexer.Block.Fetcher do | |||
token_transfers: token_transfers, | |||
transactions: transactions_with_receipts | |||
}), | |||
# addresses = Enum.filter(addresses_raw, fn a -> a.hash != "0x0000000000000000000000000000000000000000" end), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
@@ -4,7 +4,7 @@ defmodule Indexer.Fetcher.CeloAccountsTest do | |||
|
|||
import Mox | |||
|
|||
alias Explorer.Chain.{Address, Hash, CeloAccount} | |||
# alias Explorer.Chain.{Address, Hash, CeloAccount} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
Adds the following