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

dex: inventorize useful ABCI events to index #3645

Closed
Tracked by #1724
erwanor opened this issue Jan 22, 2024 · 5 comments · Fixed by #3796
Closed
Tracked by #1724

dex: inventorize useful ABCI events to index #3645

erwanor opened this issue Jan 22, 2024 · 5 comments · Fixed by #3796
Assignees

Comments

@erwanor
Copy link
Member

erwanor commented Jan 22, 2024

As part of #3504, we ported all of our event recording to use the ProtoEvent interface. We should run through a few indexing use cases (e.g. an explorer, a price monitoring tool, an event-based trading bot etc.) to check if the ABCI event coverage of the DEX is complete.

@conorsch
Copy link
Contributor

conorsch commented Feb 2, 2024

Nice to have for 65 (#3554), but really we can ship without it, and follow up with a point release. The downside of shipping without it is that anyone who wants this functionality would need to reset their node and resync in order to have the older events indexed. We accept that as a downside if it helps us ship 65 sooner.

@zbuc
Copy link
Member

zbuc commented Feb 7, 2024

Explorer Events

  • Position created
  • Position reserves changed
  • Position closed
  • Position withdrawn
  • Position reward claimed
  • Swap
  • Swap Claim
  • Batch swap executed
  • Arbitrage execution

Price Monitoring Events

  • Many of the above events are also useful
  • Best priced position changes

Event based trading bot

  • Nothing different comes to mind for me here apart from what is already outlined

@hdevalence
Copy link
Member

Position reserves changed

Rather than framing this as "position reserves changed", i think it would be useful to frame it as a "position execution", since that's what causes the position reserves to change. That way we can emit events on all the micro-executions as well as for the overall batch execution.

@zbuc
Copy link
Member

zbuc commented Feb 8, 2024

Going through the code right now and checking off completed events:

  • Position created
  • Position executed against
  • Position closed
  • Position withdrawn
  • Position reward claimed skipped for now and a comment added since reward claims are unimplemented
  • Swap
  • Swap Claim
  • Batch swap executed
  • Arbitrage execution
  • Best priced position changes skipping this because implementation is tricky and indexers could implement this based on existing position events.

@zbuc
Copy link
Member

zbuc commented Feb 8, 2024

Noticed this comment on dex::event::position_close:

    // TODO: should we have another event triggered by the position manager for when
    // the position is actually closed?

I'm not sure what the distinction of "actually closed" here is, but I haven't gone through the call chain yet. Leaving this comment to remind myself.

@zbuc zbuc closed this as completed in #3796 Feb 9, 2024
zbuc added a commit that referenced this issue Feb 9, 2024
This adds code to emit ABCI events for the following DEX actions:


- Position created
- Position executed against
- Position closed
- Position withdrawn
- Swap
- Swap Claim
- Batch swap executed
- Arbitrage execution

Closes #3645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Status: Testnet 65: Deimos
Development

Successfully merging a pull request may close this issue.

4 participants