Granola-Team / mina-indexer

The Mina Indexer is a re-imagined version of the software collectively called the "Mina archive node."
Apache License 2.0
19 stars 10 forks source link

`total_num_canonical_blocks` is likely to be misunderstood and misused #1557

Open n1tranquilla opened 1 month ago

n1tranquilla commented 1 month ago

Here is a query where we are limiting to the first 6k blocks, however the result for total_num_blocks is the length of the entire blockchain.

Query:

query BlocksQuery($query: BlockQueryInput!, $limit: Int = 10, $sort_by: BlockSortByInput!) {
  blocks(query: $query, limit: $limit, sortBy: $sort_by) {
    total_num_blocks
    total_num_canonical_blocks
    total_num_supercharged_blocks
    blockHeight
    globalSlotSinceGenesis
    transactions {
      coinbase
    }
    snarkFees
    txFees
    canonical
  }
}

Variables:

{
  "limit": 10000000,
  "sort_by": "BLOCKHEIGHT_DESC",
  "query": {
    "blockHeight_lte": 6000
  }
}

Response:

{
  "data": {
    "blocks": [
      {
        "total_num_blocks": 359614,
        "total_num_canonical_blocks": 359604,
        "total_num_supercharged_blocks": 142247,
        "blockHeight": 6000,
        "globalSlotSinceGenesis": 8478,
        "transactions": {
          "coinbase": "1440000000000"
        },
        "snarkFees": "0",
        "txFees": "1000000",
        "canonical": true
      },
...
n1tranquilla commented 1 month ago

The data is probably right, just not presented correctly or in the right place. We need to rethink our total_num_* implementations and see if they belong somewhere else.

Isaac-DeFrain commented 1 month ago

This datum is about the total number of canonical blocks, similarly for the others. What are you expecting? Number of (canonical/supercharged/etc) blocks bounded by the query?

Isaac-DeFrain commented 1 month ago

These total_num_* quantities probably belong in the summary

n1tranquilla commented 1 month ago

These totalnum* quantities probably belong in the summary

Yes, as a user in the front end, I expected the query for that block height to give me the correct value for total_num_* and was surprised by the result, so I think your thoughts referenced above are on the right track. As a real-life user, I thought this data was "wrong", but I know enough to know it just belongs in a different place in the API 😉

I'll assign back to you for now.

Isaac-DeFrain commented 1 month ago

Okay. This sort of response requires some additional support in the database. We'll need to keep CFs for block height -> number of * blocks at that height in order to answer the general query.

Isaac-DeFrain commented 1 month ago

@n1tranquilla where does this issue stand?

n1tranquilla commented 1 month ago

This is a presentation issue with the data. It's not strictly wrong, it's just presented in a way that it might be used improperly. I think we need to deprioritize this issue. Moving to bottom of backlog.