/trades endpoint does not expose data about buyer/seller of asset (BaseIsSeller is misnamed)

I’ve done some deep testing — /trades does not expose any information about who bought and sold the asset. With the data returned by /trades it is impossible to figure out who bought/sold which asset (unless you go to the operation link in the result of the /trades) BaseIsSeller should be called BaseIsMaker i.e. BaseIsSeller refers to the account that was the “maker” of the offer.

The below is a log line for a trade where GCB7WIQ3TILJLPOT4E7YMOYF6A5TKYRWK3ZHJ5UR6UKD7D7NJVWNWIQV is the “maker” and the “bid” was consumed by GCZHHDBVPDIZ7LD4O5MYZRJWCJDTZVU5PIUHQKRJVF46DX2XIEMCU7HL, i.e. the result should have been buy but BaseIsSeller is false indicating that GCB7WIQ3TILJLPOT4E7YMOYF6A5TKYRWK3ZHJ5UR6UKD7D7NJVWNWIQV was the seller, which is not actually true

sdexAsset=native/COUPON:GBMMZMK2DC4FFP4CAI6KCVNCQ7WLO5A7DQU7EC7WGHRDQBZB763X4OQI
tradeAsset=native/COUPON:GBMMZMK2DC4FFP4CAI6KCVNCQ7WLO5A7DQU7EC7WGHRDQBZB763X4OQI
sdexTradingAccount=GCB7WIQ3TILJLPOT4E7YMOYF6A5TKYRWK3ZHJ5UR6UKD7D7NJVWNWIQV
tradeBaseAccount=GCZHHDBVPDIZ7LD4O5MYZRJWCJDTZVU5PIUHQKRJVF46DX2XIEMCU7HL
tradeCounterAccount=GCB7WIQ3TILJLPOT4E7YMOYF6A5TKYRWK3ZHJ5UR6UKD7D7NJVWNWIQV
BaseIsSeller=false
offerID=25567894
baseOfferID=25568442
counterOfferID=25567894
resulting orderAction: sell

So two changes:

[ ] BaseIsSeller is misnamed, should be BaseIsMaker (maker or owner, same thing)
[ ] should expose information about which asset was bought (or sold) by the maker account (or owner account)

I've dug into this a bit.

I believe there is also a highly confusing field name in the Trade Effect: seller.

Here's a trade from /trades:

        "id": "114887149647728641-0",
        "paging_token": "114887149647728641-0",
        "ledger_close_time": "2019-11-12T00:35:14Z",
        "offer_id": "116870611",
        "base_offer_id": "4726573168075116545",
        "base_account": "GA6OQWAET6LTPXQOMKUC6RX7WN3PBPPAM6W4O7SYS4JYT6YUPG5RXK7W",
        "base_amount": "236.4747000",
        "base_asset_type": "native",
        "counter_offer_id": "116870611",
        "counter_account": "GC2PMVNFJSJO2EKE5HPJEPO2CSD6W64DFZZTPINR5E5YQMNKP3GR55VM",
        "counter_amount": "57749.9999919",
        "counter_asset_type": "credit_alphanum4",
        "counter_asset_code": "LAX",
        "counter_asset_issuer": "GAZZB5KPOWEWK4C5S5H5B6YPR2EQMUIJEFFBNQ6MWYYAG7DOEELBOHLW",
        "base_is_seller": false,
        "price": {
          "n": 488424343,
          "d": 2000000
        }

Here's the same trade from /effects:

        "id": "0114887149647728641-0000000002",
        "paging_token": "114887149647728641-2",
        "account": "GC2PMVNFJSJO2EKE5HPJEPO2CSD6W64DFZZTPINR5E5YQMNKP3GR55VM",
        "type": "trade",
        "type_i": 33,
        "created_at": "2019-11-12T00:35:14Z",
        "seller": "GA6OQWAET6LTPXQOMKUC6RX7WN3PBPPAM6W4O7SYS4JYT6YUPG5RXK7W",
        "offer_id": 116870611,
        "sold_amount": "57749.9999919",
        "sold_asset_type": "credit_alphanum4",
        "sold_asset_code": "LAX",
        "sold_asset_issuer": "GAZZB5KPOWEWK4C5S5H5B6YPR2EQMUIJEFFBNQ6MWYYAG7DOEELBOHLW",
        "bought_amount": "236.4747000",
        "bought_asset_type": "native"

and finally, the operation that generated the effect, grabbed from https://horizon.stellar.org/operations/114887149647728641

  "id": "114887149647728641",
  "paging_token": "114887149647728641",
  "transaction_successful": true,
  "source_account": "GA6OQWAET6LTPXQOMKUC6RX7WN3PBPPAM6W4O7SYS4JYT6YUPG5RXK7W",
  "type": "manage_buy_offer",
  "type_i": 12,
  "created_at": "2019-11-12T00:35:14Z",
  "transaction_hash": "2a19685b8aee75511dbc7dc8af350f3abf156f3b9a1a5f0eec0fd80bf5ccbd31",
  "amount": "57749.9999919",
  "price": "0.0040949",
  "price_r": {
    "n": 40949,
    "d": 10000000
  },
  "buying_asset_type": "credit_alphanum4",
  "buying_asset_code": "LAX",
  "buying_asset_issuer": "GAZZB5KPOWEWK4C5S5H5B6YPR2EQMUIJEFFBNQ6MWYYAG7DOEELBOHLW",
  "selling_asset_type": "native",
  "offer_id": 0

From the operation, we see the source_account is GA6O, so we know that's the taker. If it's not obvious, a trade occurs the instant a taker's offer is created that crosses the maker. We also see that the selling_asset is native, so GA6O had XLM to start with, and traded it for LAX.

The /trades endpoint gets this right. base_account is GA6O and base_asset is native, so we see that GA6O started out with XLM, just like the operation shows.

The /effects endpoint gets this wrong. It claims the seller is GA6O, and that the sold_asset is LAX. My interpretation of these fields is that really account is the one who did the selling, and so seller should really be named buyer or counter_account.

So combining with @nikhilsaraf's list, here are the todo items:

[ ] in /trades, base_is_seller is misnamed, should be base_is_maker
[ ] in /effects, seller should be renamed to buyer or counter_account

I also think that /trades is properly communicating which account initially held which asset in the trade, so no other changes are needed there.

@ire-and-curses FYI. This is a fairly serious bug since calling the buyer the seller basically guarantees the client will fail to use the endpoint correctly.

@tomquisel In my initial comment I stated that:

/trades does not expose any information about who bought and sold the asset.

My takeaway from the above comment is that the /trades endpoint is always listing the amount and asset values that was held by the base/counter accounts before the trade took place (i.e. sold as part of the trade action). With that information it clarifies my confusion [1].

However, If we rename the fields to more clearly indicate that it was sold vs. bought as a result of the trade then it would be much more intuitive. An intentionally verbose example to demonstrate the explicit naming to make it more intuitive: something like base_amount_sold, base_asset_type_sold, base_asset_code_sold, etc.

[1] I had examples while testing that this may not have always been the case. If I find any such examples then I will post it here

Great point @nikhilsaraf, we should state that assumption that the amount is the amount held just before the trade took place, either by renaming the fields or at least improving documentation. @ire-and-curses this seems like something to tackle as part of making the API useable.

Adding this for posterity, with some sample code to help figure this out:

In Kelp we look at /effects to get data about which asset was sold, and we check the context of the effect via the account field (source code).

As Tom mentioned, it is the account field that is selling or buying the asset specified in the effect, and the seller field on the effect can be misleading and should be renamed to counter_account.

You can also figure out which account bought/sold which asset by looking at the /operations endpoint, as described in the comment above:

From the operation, we see the source_account is GA6O, so we know that's the taker. If it's not obvious, a trade occurs the instant a taker's offer is created that crosses the maker. We also see that the selling_asset is native, so GA6O had XLM to start with, and traded it for LAX.

However, note that the operation could be a path payment instead of a trade type operation which may introduce different formats for you to consider.

Base can be a maker or a taker, and also a seller or a buyer of its base asset. All four combinations are perfectly valid.

stellar / go

/trades endpoint does not expose data about buyer/seller of asset (BaseIsSeller is misnamed) #1514