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

[Serum] Populate seqNum on Event and implement loadFillsSince #195

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lidatong
Copy link

Thank you for implementing this client library for Serum! I really appreciate the open-source efforts to make the Dex accessible, and I'd like to make my own contribution.

Description

This PR makes two changes:

  1. Populates the seqNum on the Event type in decodeEventQueue
  2. Implements a new function loadFillsSince

Why

It appears based on git blame that seqNum was added after decodeEventQueue, and decodeEventQueue was never updated.

I think it would be useful to populate the seqNum given it is available as a field, and there isn't a way to uniquely determine whether you've processed a Fill without it.

I also implemented an additional loadFillsSince API function, since there is already functionality to decodeEventsSince, and I think it would useful to be able to loadFillsSince a given seqNum

Testing

I manually against the Serum Validator node:
https://solana-api.projectserum.com

Goal was to test that the seqNum is calculated correctly, being careful to make sure it handles the u32overflow case.

Here is a sample test output. Notice that head seqNum is 26178226, count is 4, user-requested limit is 5.

Doing the logic and calculating the expected case by hand:

Header seqNum: 26178226
Count: 4
Ending seqNum: 26178226 + 4 - 1 = 26178229
Starting seqNum: endSeqNum - limit = 26178225

The test output below correctly indicates the modified decodeEventQueue returns 5 fill events with the seqNum starting from 26178225:

{
  head: 967,
  count: 4,
  seqNum: 26178226
}
// decodeEvents: requested limit 5


[
  {
    orderId: <BN: 6c0d1fffffffffefbe0a9>,
    seqNum: 26178225
  },
  {
    orderId: <BN: 5b706fffffffffefbe0af>,
    seqNum: 26178226
  },
  {
    orderId: <BN: 4f414fffffffffefbe0b0>,
    seqNum: 26178227
  },
  {
    orderId: <BN: 630defffffffffefbe0b3>,
    seqNum: 26178228
  },
  {
    orderId: <BN: 82fc00000000001041f4a>,
    seqNum: 26178229
  }
]

@lidatong lidatong changed the title [Serum] Populate seqNum on fills and implement loadFillsSince [Serum] Populate seqNum on Event and implement loadFillsSince Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant