Proposal: Simplified Requests, Flexible Responses

[FredKSchott]

Following up from #799:

@intelliot: @FredKSchott I'm in favor of returning all API data to the consumer, as you said. Historically, RippleAPI was designed independent of rippled. However, moving forward, we'd like to unify their APIs and make them as similar as possible. Please feel free to take a stab at this!

So I've experimented a bit and have a proposal. Even with 100% proper typing, I'm nervous to make any big changes to the existing API methods and the interfaces/data that they already return. There are a lot of interconnected parts internally, so I'm not just worried about breaking external consumers with a lot of small, hard to detect/message changes.

This proposal aims to reduce the complexity & maintainance costs of having a complete secondary ripple-lib API on top of the standard Rippled API:

• Unify the ripple-lib & rippled APIs without breaking existing users
• "Simplified API Request Interface":
  • Provide access to all request options, not just a filtered selection
  • Reduce complexity & high maintenance costs for the core team
• "Flexible API Response Interface":
  • Provide access to all response data, not just a filtered selection
  • Continue to provide easy formatting

Proposal: Simplified API Request Interface

All custom API methods (getTransactions(), getTrustlines(), etc) are deprecated in favor of just two new general-purpose API methods:

class RippleAPI:

  // Merge {...data, command} into a single request and send to the server.
  // Return the response object directly from the server.
  async request(command: string, data?: Object): Response;

  // Like request(), but requests multiple pages until the `limit.max` number
  // of `limit.collect` objects are returned from the response.
  async requestAll(command: string, data?: Object, limit?: {max: number, collect: string}): Response[];

Examples

- return api.getAccountInfo('r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59');
  return api.request('account_info', {account: 'r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59'});

- return api.getOrders('r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59');
  return api.request('account_offers', {
    account: 'r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59',
    ledger_index: 'validated',
  });

- return api.getOrderbook('r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59', {
-   "base": {"currency": "XRP"},
-   "counter": {"currency": "USD", "issuer": "rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59B"}
- });
  return api.requestAll('book_offers', {
    taker: 'r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59'
    taker_gets: {'currency': 'XRP'},
    taker_pays: {'currency': 'USD', 'issuer': 'rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59B'},
  }, {max: 100, collect: 'offers'});

Benefits

• Already in use today, but is undocumented / not publicly supported: How to listen to events on the XRP Ledger (transaction, ledger, account) by a special account? #809 (comment)
• Eliminates a ton of ongoing documentation and maintenance cost for the team.
• More gradual learning curve, no need to learn both rippled & ripple-lib and map concepts.
• New Rippled API commands are supported immediately, no update/upgrade required.
• Doesn't break existing users, existing methods are phased out / deprecated slowly.
• Can still provide helpers to format/validate request & response data (see below)

Support for Request Validation

Request validation can be easily moved into the general request()/requestAll() method:

async request(command: string, data: Object) {
  validateRequest(command, data); // Throw/warn if `data` is invalid for `command`
  // make request...
}

Support for Typing

Overloading the request function signature would allow powerful typing support for request()/requestAll() arguments and response format:

class RippleAPI:
  async request(command: 'account_info', data: AccountInfoRequest): AccountInfoResponse;
  async request(command: 'account_offers', data: AccountOffersRequest): AccountOffersResponse;
  async request(command: 'book_offers', data: BookOffersRequest): BookOffersResponse;
  async request(command: string, data: Object): Response {
    // implementation
  }

Proposal: Flexible API Response Interface

While moving closer to the Rippled API spec is a goal, there are still times where we want to handle/format/parse the response data into an easier-to-use form. Existing API methods handle some properties automatically, but at the expense of flexibility. If the consumer needs other data that the method does not return, they are out of luck.

Data Objects are simple wrappers around raw API response objects, that give the user access to all raw data from the API response (okay for most properties) plus the same helpful formatting that consumers are already used to.

Here's a partial example of what the Offer class would look like:

class Offer {
  data: OfferRaw;
  constructor(offer: OfferRaw) {
    this.data = offer;
  }
  // return a direction string based on the offer's sell flag
  getDirection(): string {
    return (this.data.flags & flags.Sell) === 0 ? 'buy' : 'sell';
  }
  // return data.taker_gets if the direction is 'buy', otherwise data.taker_pays
  getAccountGets(): Amount;
  // return data.taker_pays if the direction is 'buy', otherwise data.taker_gets
  getAccountPays(): Amount;
  // ...
}

And then here it is in use, flexible enough to handle any properties that the user wants to work with:

import {Offer, parseTimestamp} from '...';

const response = await api.request('account_offers', { account: 'r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59' });

for (const offerRaw of response.offers) {
  // Create an "offer" Data Object for the API response object
  const offer = new Offer(offerRaw);
  // Most data can be read directly from the data object
  const seq = offer.data.seq;
  // Get special formatted data from the API response object
  const direction = offer.getDirection();
  const accountGets = offer.getAccountGets();
  // Some general formatters/parsers can exist for common data types
  const expirationTime = parseTimestamp(offer.data.expiration);
  // Result: Log whatever information you want!
  console.log(`${seq} ${direction}: ${accountGets.value} ${accountGets.currency} (exp. ${expirationTime})`);
}

Feedback?

What do you think? Would love your thoughts @intelliot & @contributors.

[intelliot]

@FredKSchott This looks really good.

I've been thinking about this proposal all morning, and it's very similar to what I had in mind. In fact, it's better - I hadn't thought about using literal values as types. That provides us with type checking for existing APIs while also allowing new commands to be used. Perfect.

We won't need to make breaking changes (existing methods can be phased out at some point during the next year) and further API improvements/simplifications can be made at the rippled layer.

👍 from me.

@wilsonianb @shekenahglory @mDuo13 What do you think?

[mDuo13]

This is awesome. I'm totally on board with this. One thing that'll require some work beyond the proposed spec is offline signing though. The API should not use the rippled APIs by default when it can do the signing operation without network access. We should continue to support the existing APIs for this.

• Multisigning and channel claim signing too.
• And key generation.
• Also, as a safety feature, ripple-lib should fail with an error if you try to do a sign request (or similar) when connected via a non-encrypted connection, except to whitelisted "internal" hosts. (For example, it's OK to use unencrypted HTTP when your rippled instance is on localhost, IPs in the intranet ranges like 10.1.* and 192.168.*, etc.) Honestly this maybe should have its own spec for validating this stuff.

You've already touched on timestamp conversion. Other formatters that should be supported (this is all implemented already, but may need to be adapted to the new interface):

• Decimal XRP to drops & back
• Convert Flags integers to maps of booleans and back. This requires some context since the same flag values have different meanings. For example, AccountSet flag 0x00010000 is tfRequireDestTag, but AccountRoot flag 0x00010000 is lsfPasswordSpent.
• ASCII/UTF-8 encoding/decoding of Memos, Domain, some others
• reading balance changes / outcomes from transaction metadata

[FredKSchott]

@intelliot nice! That's good to hear this isn't too far off.

@mDuo13 👍 That's a great point. None of those really map 1:1 with rippled API commands either, so it definitely makes sense to leave the offline logic as-is for now. Where they do make requests, we can refactor those to use this new request() API.

Down the road, it would be nice to make it even more explicit which of those methods are truly offline-only. Right now it's hard to tell which require a connection and what data they'll fetch from it. Also easy to forget which require the "offline" option.

Also great point about Flags, it sounds like they should be handled by the Data Object (ex: every object has a parsed flags property object)

[FredKSchott]

Thanks for the feedback! I'll hopefully find some downtime over the holidays to get started on a few of these

[FredKSchott]

@intelliot @mDuo13 I had a bunch of time over the 🦃 holidays to work on this and have a first branch about ready for PR.

A preview of it is up, but I ran into a problem early on:

Flow doesn't support function overloading. I was so used to using TypeScript for JS typings that I'd just assumed Flow supported this as well. I actually ran into a few different issues while digging into the code base, including some obvious violations & bugs that Flow just wasn't detecting.

Has anyone on the team used TypeScript before? Having used both, TypeScript is really miles ahead of Flow:

• More features, including function signature overloading
• More expressive types
• Much better at detecting type violations & bugs
• Simpler model overall (build-time checking vs. running a validation server in the background)
• Can distribute definitions with the library so that consumers get typing support for their dependencies (In VSCode, even plain JavaScript consumers get usage hints in the editor)
• Active community of third-party definitions for dependencies not written in TypeScript

Would you consider TypeScript for ripple-lib? This is probably something you all want to talk about as a team, but I think it's the right choice for something complex and financially-focused like ripple-lib, even separate from this larger proposal. I'm happy to discuss offline more as well if you have any questions.

---

I was able to make the move to TypeScript without too much trouble (two full days of flying without wifi didn't hurt). It involved two main changes to the codebase:

Common.js -> ESM (require() -> import, module.exports -> export)
develop...FredKSchott:esm
TypeScript uses ESM syntax almost exclusively. It's "strict" by default, and a much more standard explicit syntax that is easier for tooling/bundlers/browsers to optimize for. I was able to do this conversion first with the existing Babel setup, separate from any TS work, and I'd recommend this change regardless.

Flow -> TypeScript
FredKSchott/ripple-lib@esm...FredKSchott:typescript
Flow and TypeScript use an almost exactly similar type format, which is nice. Most of the work consisted of fixing type violations & bugs that had gone undetected in Flow. I've erred on the side of caution and documented any issues that weren't easy fixes with TODOs vs. trying to fix them here and introducing breaking changes.

Let me know what you think, happy to create PRs from these branches if you want to move forward.

[FredKSchott]

@intelliot friendly ping! Would love to hear your thoughts on this. Happy to discuss offline as well.

[intelliot]

Thanks!

Our team hasn't used TypeScript, but I'm looking into it, and I think it's a good language. These branches look like a great start!

I'll reach out to you via email as well.

[intelliot]

This looks good. We would like to move to TypeScript - we'll review PRs when ready.

[sublimator]

TypeScript rocks, you'll never wanna fo back to raw JS :)

[FredKSchott]

Sounds good, I'll get those two branches into PRs

[tuloski]

Dear god no!
If you change again all the interfaces now that I have ported all the old ripple-lib code to RippleAPI I'll leave ripple forever and sell all my XRP and spam FUD on reddit and DDoS the validators :).

[FredKSchott]

@tuloski this work is being done specifically so that you can continue to use the current interface. Each method is being kept and rewritten to use the new general request() method internally. If ripple-lib ever did officially deprecate them, they wouldn't need to be removed, just moved to a separate helper library and continue to work as-is.

Also, threats against developers or the projects they work on are not appreciated or helpful. Please learn better Open Source manners if you plan to continue contributing.

[tuloski]

@FredKSchott I put a smile in the comment because I was joking :)

So do you want to keep the old "getAccountInfo", etc, and just re-implement internally with request/requestAll, such that using request one can use the standard rippled request also? Sounds good. I understood the request was going to replace old methods.

Btw the old ripple-lib already had the same interfaces as rippled. Do you know why rippleAPI modified it to have a separate interface? I agree that having the same common format with rippled answers is very good.

[FredKSchott]

@tuloski Sounds good, tone is hard on Github :)

Correct, that's the immediate-term plan. Personally, I'd eventually like to see the current methods pulled out of ripple-lib and moved to a separate library (since they now use the preferred public request() interface) but if this ever happened it would be done specifically to be as easy as possible for existing users.

[sublimator]

haha, I knew adding this extra "RippleAPI" layer would lead to just more trouble and ultimately more to learn :) You can't escape the lower levels when debugging ...

[FredKSchott]

Excited to see progress in #816! Thanks for the reviews @intelliot.

I've been thinking about an added bonus while I've been adding typing info for different api requests/responses:

Right now, the API is documented on the docs site via markdown. We redefine those interfaces in 1. the docs site, 2. rippled, and 3. ripple-lib (either in Flow or TypeScript). Manually keeping all of these in sync adds extra maintenance + risks bugs/developer issues where they get out of date from each other. It would be great to keep a single source of truth that rippled & the docs site & rippled-lib could all reference and test against:

• rippled: defines it's API request/response format via different JSON Schema. Tests its own tests against those requests/responses.
• docs site: generate the request/response tables automatically from that JSON Schema.
• ripple-lib: generate typescript definitions automatically from that JSON Schema using a tool like json-schema-to-typescript

This would be a longer-term project, but the work being done in FredKSchott/ripple-lib@typescript...FredKSchott:request-interface is setting the stage if we end up wanting to go down that road. Those api TypeScript definitions could be converted to JSON Schema and moved to rippled to become the single source of truth.

[mDuo13]

I've been pushing for a while to get rippled to define JSON Schemas for requests and responses so it could validate input and output. That's not so easy with the way rippled's RPC code is currently architected. But if we did, then yeah, the docs site could easily generate response/request tables from said schemas; the code to do that already exists.

[sublimator]

@mDuo13

It would totally be a worthwhile project, getting the schemas in place for the rippled api, even if they didn't use them to validate right away.

[sublimator]

further API improvements/simplifications can be made at the rippled layer.

The API layer should be separated from rippled. I think things would move a lot faster if you had "dumb" peers (speaking to rippled via binary) implemented in some higher level language, where the integration guys could actually dogfood the API.

As flawed as this "wash over the rippled api quirks" approach was, it was done for a reason. There's a bunch of inconsistencies in the rippled api.

[sublimator]

With a plugin architecture or course ... for community commands ...

[sublimator]

But that would be way too cool and make too many people happy

[tuloski]

If the goal is to remove all the "high level" request and move everything to "low level" request then here we are.
But I don't see why the lib cannot provide at the same time high level request as:

• getBooks
• autobridged books
• Manage subscriptions and maintain books
• Manage subscriptions and filter for example only payment transactions...
• Signing transactions
• ...

Because sincerely making everything a request is not much more than opening a ws and sending the raw request as described in the rippled API. The big problems the user have is in parsing/filtering the data received.

[intelliot]

We want users to be aware of the rippled API and make requests directly. The library does provide helper methods to make it easier to create requests and parse/filter responses.

getBooks is supported with formatBidsAndAsks (coming soon) and getOrderbook (legacy).

I’m not sure what we need to do to support autobridged books, but it’s something I’d like to look into.

Providing helpers for managing subscriptions, maintaining books, and filtering would be great too.

Transaction signing is already supported as well. There’s some code cleanup to do, and it would be great to reduce our dependencies as well.

Thanks for the feedback!

[intelliot]

formatBidsAndAsks is now available. Docs: https://xrpl.org/rippleapi-reference.html#formatbidsandasks

Closing this issue since we've made big strides in this direction, but feel free to open a new issue for any additional 'convenience' helpers that are desired.