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

add getLedgers endpoint #111

Closed
mollykarcher opened this issue Mar 21, 2024 · 11 comments · Fixed by #303
Closed

add getLedgers endpoint #111

mollykarcher opened this issue Mar 21, 2024 · 11 comments · Fixed by #303
Assignees

Comments

@mollykarcher
Copy link
Contributor

mollykarcher commented Mar 21, 2024

Users need to be able to page through all ledgers available in RPC's retention window. The getLedgers endpoint should be able to support pagination and should provide all ledger header fields, similar to Horizon's ledgers endpoint

Scope for this will be limited to simple pagination with no filters, and we will work/propose additional filters on this as separate issues.

Request format

{
  "jsonrpc": "2.0",
  "id": <id>,
  "method": "getLedgers",
  "params": {
    "startLedger": <startLedger>,
    "pagination": {
      "limit": <limit>,
      "cursor": <cursor>
    }
  }
}

Response format

{
  "jsonrpc": "2.0",
  "id":<id>,
  "result": {
    "ledgers": [
      {
        "id": <id>,
        "hash": <hash>,
        "sequence": <sequence>,
        "transaction_count": <count>,
        "successful_transaction_count": <count>,
        "failed_transaction_count": <count>,
        "operation_count": <count>,
        "tx_set_operation_count": <count>,
        "closed_at": <time>,
        "total_coins": <count>,
        "fee_pool": <fee_pool>,
        "base_fee_in_stroops":<base_fee>,
        "base_reserve_in_stroops": <base_reserve>,
        "max_tx_set_size": <set_size>
      }
    ],
    "latestLedger": <latestLedger>,
    "latestLedgerCloseTime": <closeTime>,
    "oldestLedger": <oldestLedger>,
    "oldestLedgerCloseTime": <oldestCloseTime>,
    "cursor": <cursor / pagingToken>
  }
}

The above example represents all fields that are currently in Horizon's response. However, we should assess whether all of these are necessary for RPC.

@mollykarcher mollykarcher added this to the platform sprint 45 milestone Mar 21, 2024
@mollykarcher mollykarcher moved this from Backlog to Next Sprint Proposal in Platform Scrum Mar 27, 2024
@mollykarcher mollykarcher moved this from Next Sprint Proposal to Current Sprint in Platform Scrum Mar 27, 2024
@mollykarcher mollykarcher moved this from To Do to Backlog in Platform Scrum Apr 9, 2024
@mollykarcher mollykarcher moved this from Backlog to To Do in Platform Scrum Apr 9, 2024
@mollykarcher mollykarcher moved this from To Do to Backlog in Platform Scrum Apr 9, 2024
@janewang janewang removed this from the platform sprint 46 milestone Apr 23, 2024
@mollykarcher mollykarcher added this to the platform sprint 47 milestone May 7, 2024
@sreuland sreuland moved this from Backlog to To Do in Platform Scrum May 22, 2024
@aditya1702 aditya1702 self-assigned this May 31, 2024
@mollykarcher mollykarcher moved this from To Do to Backlog in Platform Scrum Jun 18, 2024
@mollykarcher
Copy link
Contributor Author

@janewang can you give some context on what use cases we expect this endpoint to be supporting? Asking because RPC does not currently save the ledger header info (just meta associated with a ledger), so this will be more data we'll need to start storing. Given that, and taking a critical look at all the info here, I'm not sure why a lot of it would be necessary. I think sequence/closed_at and possibly the tx counts have real uses, but not sure about all the other stuff. I also only just realized that the info Horizon's API exposes is not exhaustive of all the fields in the ledger header (see here). So it'd be good to have your perspective on what fields we should include here.

@janewang
Copy link
Collaborator

janewang commented Jul 9, 2024

Yes, we typically use getLedgers for transaction verification (to confirm if a transaction or a set of transactions have been included) and for ledger-level analysis, such as examining ledger sequence numbers, time of close/finality, fees, and getting a snapshot of the ledger at that point in time. Some users also utilize it to check network health (block time, gas, successful/failed transactions). In my opinion, most dApp use cases are covered by getTransactions, which is why that endpoint is a higher priority.

That said, information such as block-level fees, ledger close (finality), and the state of the chain at a point in time comes from block-level data. Theoretically, any state transition applied to that block level should be reproducible.

In other L1s, most RPCs ask developers to query getLedgers for the transaction hashes, then use those hashes to get transactions. We have the ability to query transactions with getTransactions. We have trained our devs to focus more on the transactions.

Regarding the header, do we store the ledger header xdr in the RPC? It has most of the info needed. If we do, we could unpack to JSON. That's the only field we need.

@janewang
Copy link
Collaborator

janewang commented Jul 9, 2024

Btw, I think ledger headers should be stored. I have seen RPC on other L1s storing the headers as well as downloading headers first before downloading blocks.

@mollykarcher
Copy link
Contributor Author

RPC does not currently save the ledger header xdr, no. So then, are all the fields in the Response format example in the description necessary? And/or do we want to include any additional fields from the ledger header?

In other L1s, most RPCs ask developers to query getLedgers for the transaction hashes, then use those hashes to get transactions. We have the ability to query transactions with getTransactions. We have trained our devs to focus more on the transactions.

It sounds to me like you're saying here that we don't have to include the transaction hashes in this API, but correct me if I'm misunderstanding that.

@janewang
Copy link
Collaborator

janewang commented Jul 9, 2024

They are not all necessary but we could simply unpack the XDR and show them. I listed the ones I think we should include for sure.

{
  "ledger_version": 12, <- We need this as the known protocol version. 
  "previous_ledger_hash": "9eac16fecd885147067b58b7684f60d216f931b813f651265bbc97de4cea313d", <- We need this pointer to the previous ledger.
  "scp_value": {
    "tx_set_hash": "6ecbf321df86b6e97698d316e14650f8aa926ff9db37c175093c407d0b32532f",
    "close_time": 1575671972, <- We need this as time of finality
    "upgrades": [],
    "ext": "basic",
  },
  "tx_set_result_hash": "5d03694bc75a286654798e6ab9851c22225904c07b2e22ec649b04c7dab0efd7", <- We need this, easier to validate the result of txs
  "bucket_list_hash": "f1f7d06efe593eb0878f35435cb8e0b67aeb61cdf84bb962759d906ea101438e", <- Probably yes
  "ledger_seq": 27146933, <- We need this
  "total_coins": 1054439020873472900, <- We need this
  "fee_pool": 18072647509661, <- We need this
  "inflation_seq": 278,  // don't need this
  "id_pool": 138084394, <- We need this
  "base_fee": 100, <- We need this
  "base_reserve": 5000000, <- We need this
  "max_tx_set_size": 1000, <- We need this
  "skip_list": [ 
    "a1236d1d724d24a296941b609003352c1c739736232254b4c70a633feb828bbe",
    "9f423e7dc204f2d3ec6dc7b3b5a936fe5bacdc236cd6667ad453ff2ff1164714",
    "17e8a36d1060a87de7ec16af7e3cc9901ec59a5dae38720b16c28719554d9397",
    "ea765f22071804c9d946439308b74f3bbeb27aaa4386abbd2ab3dedd83d33b7c",
  ],
  "ext": "v0",
}

@janewang
Copy link
Collaborator

janewang commented Jul 9, 2024

It sounds to me like you're saying here that we don't have to include the transaction hashes in this API, but correct me if I'm misunderstanding that.

On the Horizon API, we provide a href to the transactions of this ledger:

{
  "transactions": {
   "href": "[https://horizon-testnet.stellar.org/ledgers/2/transactions{?cursor,limit,order}](https://horizon-testnet.stellar.org/ledgers/2/transactions%7B?cursor,limit,order})",
   "templated": true
}

We could do the above. If not, we should return a list of transaction hashes, and the user could iterate via getTransaction.

@Shaptic
Copy link
Contributor

Shaptic commented Sep 18, 2024

@janewang we're about ready to start work on this but I wanted to kick off the schema discussion again so that we're all on the same page.

Since we have xdr2json support now via xdrFormat: "json" on all other endpoints, I would suggest the following, highly simplified schema:

// request: getLedgers(startLedger, pagination?: { limit; cursor });
// response:
interface GetLedgersResponse {
  ledgers: LedgerResponse[];
  latestLedger: number;
  latestLedgerCloseTime: number;
  oldestLedger: number;
  oldestLedgerCloseTime: number;
  cursor: string;
}

interface LedgerResponse {
  hash: string;
  sequence: number;
  closeTime: number;
  closeMeta: string; // xdr.LedgerCloseMeta
  header: string; // xdr.LedgerHeader
};

Users interested in the details of each field in this response can just use JSON expansion, while anyone programmatically interested can derive all data from the header or its parent meta.

If a user is interested in doing transaction pagination, we can also return a single transactionCursor that they can pass to getTransactions to start viewing the transactions starting from the first-returned ledger.

@janewang
Copy link
Collaborator

janewang commented Sep 18, 2024

This makes sense to me, unpacking xdr.LedgerHeader with XDR to JSON is cleaner. cc @johncanneto

@aditya1702
Copy link
Contributor

@Shaptic +1 on the response schema. However, I dont understand the transactionsCursor part. If a user wants to start viewing the transactions starting from a ledger, they can just pass it as the startLedger in getTransactions right?

@Shaptic
Copy link
Contributor

Shaptic commented Sep 18, 2024

@aditya1702 oh yeah, good point! Since transactions are more granular than ledgers, the cursor would always point to the beginning. I was just thinking of a way to address @janewang's concern about ledger -> transaction pagination but you're right that it's just startLedger shared among both.

One thing I would like to highly recommend we do, for this and all other endpoints, is to elaborate in detail how to inspect commonly-necessary fields using the SDKs. We can add this to the documentation for every endpoint accordingly. For example, if a user wants the transaction hashes, transaction envelopes, operation counts, etc. we should outline how to extract them out of the metadata.

Also, just to clarify: the xdr.LedgerHeader is a subset of the xdr.LedgerCloseMeta. I thought it might be helpful to add it at the top level so that we commit to its existence in the response schema rather than have users adapt to varying XDR structures if they choose the JSON approach, so they can do something like

curl ... { ..., "xdrFormat": "json", ... } | jq '.ledgers[] | .ledgerHeader'

We can just keep it deep in the meta as well if folks would like.

@aditya1702
Copy link
Contributor

One thing I would like to highly recommend we do, for this and all other endpoints, is to elaborate in detail how to inspect commonly-necessary fields using the SDKs. We can add this to the documentation for every endpoint accordingly. For example, if a user wants the transaction hashes, transaction envelopes, operation counts, etc. we should outline how to extract them out of the metadata.

@Shaptic Yes definitely. This will be very helpful for the partners and anyone trying to get info from XDRs.

Yeah I agree, we should add it in the response even if it is present in meta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants