Reviewed 16 of 16 files at r1.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @charlie-wasp)

src/model/offer.ts, line 47 at r1 (raw file):

  public get paging_token() {
    return this.id;
  }

And do we need Offer model now that we have Offer entity?

src/orm/entities/offer.ts, line 39 at r1 (raw file):

  public get paging_token() {
    return this.id;

I think default ordering for top-level offers query is (price, id) so paging token should include price component in this case. We still want ordering by id only for account { offers } queries.

src/orm/repository/offer.ts, line 17 at r1 (raw file):

      .where("offers.selling = :selling", { selling })
      .andWhere("offers.buying = :buying", { buying })
      .getRawOne();

Ordering by price and selecting the first row would guarantee that we hit the index

src/schema/offers.ts, line 71 at r1 (raw file):

      seller: AccountID
      selling: AssetCode
      buying: AssetCode

I think it would be reasonable to make selling and buying mandatory for top-level offers query, and also remove seller parameter here since it could now be queried as account { offers }.

src/schema/resolvers/offer.ts, line 93 at r1 (raw file):

      }

      return makeConnection<Offer>(await paginate(qb, paging, "offers.id"));

To reiterate, I think seller is not necessary here, selling and buying are required and default sorting should be (price, id).

src/schema/resolvers/trades.ts, line 20 at r1 (raw file):

  const offers = await getRepository(Offer).findByIds(ids);

  return ids.map(id => offers.find(o => o.id === id) || null);

This likely won't work because quite often after a trade have already happened, the offers would no longer exist in the database. Probably for now the best solution is to just return the offer ids in trades, without links to the actual offers.

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @charlie-wasp and @nebolsin)

src/model/offer.ts, line 47 at r1 (raw file):

Definitely no! Simply forgot about it

Reviewable status: 12 of 22 files reviewed, 6 unresolved discussions (waiting on @charlie-wasp and @nebolsin)

src/orm/entities/offer.ts, line 39 at r1 (raw file):

Working

src/orm/repository/offer.ts, line 17 at r1 (raw file):

Working

src/schema/offers.ts, line 71 at r1 (raw file):

Done

src/schema/resolvers/offer.ts, line 93 at r1 (raw file):

Working

src/schema/resolvers/trades.ts, line 20 at r1 (raw file):

Done

Reviewable status: 12 of 22 files reviewed, 6 unresolved discussions (waiting on @charlie-wasp and @nebolsin)

src/orm/repository/offer.ts, line 17 at r1 (raw file):

Hmm, I ran explain select min(price) as min_price from offers where sellingasset = 'AAAAAU1PQkkAAAAAPHEwK55l2uMBECcQxzsOUsAWg1YwXwD+ZUTDWZEbZWQ=' and buyingasset = 'AAAAAA==', and it seems that index is used already. It says -> Index Only Scan using bestofferindex on offers (cost=0.41..597.64 rows=546 width=8)

Reviewable status: 10 of 23 files reviewed, 6 unresolved discussions (waiting on @charlie-wasp and @nebolsin)

src/orm/entities/offer.ts, line 39 at r1 (raw file):

Done.

src/schema/resolvers/offer.ts, line 93 at r1 (raw file):

Done.

Reviewed 13 of 13 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @charlie-wasp)