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

[ECO-624] Fix async dedupe aggregator logic #525

Merged
merged 3 commits into from
Oct 8, 2023
Merged

[ECO-624] Fix async dedupe aggregator logic #525

merged 3 commits into from
Oct 8, 2023

Conversation

alnoki
Copy link
Member

@alnoki alnoki commented Oct 7, 2023

#518

  • Aggregate both maker and taker side of a fill event
  • Update fill aggregation SQL
  • Abstract fill/change event total order aggregation sequence

Testing

Updated config.yaml:

health_check_port: 8085
server_config:
  processor_config:
    type: econia_transaction_processor
    econia_address: 0xeeee0dd966cd4fc739f76006591239b32527edbb7c303c431f8c691bda150b40
  postgres_connection_string: postgres://econia:econia@postgres:5432/econia
  indexer_grpc_data_service_address: http://streamer:50051
  indexer_grpc_http2_ping_interval_in_secs: 60
  indexer_grpc_http2_ping_timeout_in_secs: 10
  auth_token: dummy_token
  starting_version: 0

Started local compose:

# From Econia repo root.
docker compose --file src/docker/compose.dss-local.yaml up

Opened:

http://0.0.0.0:3000/limit_orders?order=price.desc,last_increase_stamp.asc&market_id=eq.1&side=eq.ask&order_status=eq.open

Ran trade.py to completion, refreshing the browser after each step:

# In another terminal, also from repo root.
cd src/python/sdk
poetry install
poetry run trade

After script was done, checked http://0.0.0.0:3000/limit_orders?order=price.desc,last_increase_stamp.asc&market_id=eq.1&side=eq.ask&order_status=eq.cancelled:

Screenshot 2023-10-07 at 13 17 54

Using https://chrome.google.com/webstore/detail/json-viewer/gbmdgpbipfallnflgajpaliibnhdgobh/related on Brave

@alnoki alnoki requested review from CRBl69 and elliottdehn October 7, 2023 19:34
@alnoki alnoki marked this pull request as ready for review October 7, 2023 19:34
SET remaining_size = $1, last_updated_at = $4
SET
last_updated_at = $4,
order_status = 'open',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would you set order status to open ? It seems not very useful since an order cannot be re-opened. It can also introduce errors if cancel and change events are not emitted or processed in error. I think there is no downside to not setting the status to open so why set it to open ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider:

  1. User opens order with remaining size 10
  2. User changes remaining size to 15
  3. 10 fills

If the events are aggregated out of order, (e.g. event 1, then 3, then 2), the order will be marked closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 0eb778a

@CRBl69 CRBl69 merged commit dfd6a34 into ECO-484 Oct 8, 2023
@CRBl69 CRBl69 deleted the ECO-624 branch October 8, 2023 16:55
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.

2 participants