coinbase / mesh-specifications

Specification files for the Mesh Blockchain Standard
Apache License 2.0
324 stars 92 forks source link

Make "Operations" field optional on Transaction Model and block_identifier optional in BlockResponse #68

Closed HelloKashif closed 3 years ago

HelloKashif commented 3 years ago

Summary: Make "Operations" field on Transaction Model optional so we can efficiently return /search/transactions response with just the tx hashes instead of calling /block/transaction on EACH tx to get the operations data AND Make block_identifier optional in BlockResponse: https://www.rosetta-api.org/docs/models/BlockTransaction.html

Motivation The recent addition of /search/transactions indexer api opened up a use case of getting "all txs by an address". We currently enable this by storing all txs by address in a mongo database

"address": {
   id: "232323"
  "transactions": [ hash1, hash2 ]
}

Notice that we only store the hashes not the full transaction data. Now getting this data is easy.

POST /search/transactions
{
    ...
     "account_identifier": {
        "address": "addr_12312112112"
    }
}
Response:
{
    ....
    "transactions": [
        {
            "transaction_identifier": {
                "hash": "hash1",
            }
        },
        {
            "transaction_identifier": {
                "hash": "hash2"
            }
        }
    ]
}

However https://www.rosetta-api.org/docs/models/Transaction.html states that "operations" field is required in the response of transaction. Which means we will have to query /block/transaction by this hash for EVERY tx in our list. This could be thousands of query for some addresses.

Also sometimes we don't care about the operations value and just want to get the list of transaction hashes by account so its quite inefficient to return all the operations when not needed.

With the same logic https://www.rosetta-api.org/docs/models/BlockTransaction.html the "block_identifier" should be optional as returning this for each tx would require extra db calls

Proposal: Make "Operations" field optional in https://www.rosetta-api.org/docs/models/Transaction.html This doesn't break any implementations as they were already sending it anyways. And for the /search/transactions api we will omit sending it saving us thousands of extra api calls. AND Make block_identifier field optional in https://www.rosetta-api.org/docs/models/BlockTransaction.html

patrick-ogrady commented 3 years ago

However https://www.rosetta-api.org/docs/models/Transaction.html states that "operations" field is required in the response of transaction. Which means we will have to query /block/transaction by this hash for EVERY tx in our list. This could be thousands of query for some addresses.

We added "paginaton" fields so that any indexer implementation could limit the number of items returned in a single query (regardless of what the caller specifies in the request). Did you not feel this was sufficient to prevent these thousands of queries?

Also sometimes we don't care about the operations value and just want to get the list of transaction hashes by account so its quite inefficient to return all the operations when not needed.

Could you elaborate on when they wouldn't be needed? Returning the fully populated Transaction in the response was an optimization to remove the need to make another call to /block/transaction when populating a view.

With the same logic https://www.rosetta-api.org/docs/models/BlockTransaction.html the "block_identifier" should be optional as returning this for each tx would require extra db calls.

I'd recommend always storing BlockIdentifier with TransactionIdentifier in your database. Some networks (even Bitcoin) have blocks that contain TransactionIdentifiers seen in other blocks. Without BlockIdentifier included, it is impossible to differentiate between which transaction a query refers to.

Proposal: Make "Operations" field optional in https://www.rosetta-api.org/docs/models/Transaction.html This doesn't break any implementations as they were already sending it anyways. And for the /search/transactions api we will omit sending it saving us thousands of extra api calls. AND Make block_identifier field optional in https://www.rosetta-api.org/docs/models/BlockTransaction.html

To make this happen, we'd need to define a new SearchTransaction model to replace the BlockTransaction model currently being used where certain fields could be made optional. We use BlockTransaction extensively with the Data API and don't want to give the impression that certain fields are no longer required (when they would really only optional for the Indexer API).

HelloKashif commented 3 years ago

@patrick-ogrady thanks. I agree on all your points now. Upon putting more thoughts on this I decided to just use pagination and limit the results. So this request should be closed.

Could you elaborate on when they wouldn't be needed? Re...

I was considering like an explorer page where we only rendered the list of transactions/addresses ids and then when the user clicked on one then we fetched the full details. In that case just returning the tx hashes or the addresses list would have been faster.

But I agree this will complicate the api a lot and we can get by just by using limits/offsets. Let's close this. Thanks for the consideration.