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

RFQ enrich incoming accept messages with sent requests #931

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Jun 6, 2024

In this PR, we enrich incoming accept messages with corresponding sent requests. This involves storing outbound requests before sending and matching incoming accept message responses based on the RFQ quote ID. This matching process is performed in the RFQ stream service, ensuring that other RFQ services only encounter accept messages with fully populated fields, including the new request field.

As a result, transporting the asset ID in the accept message is no longer necessary. The asset ID can be found in the request message.

Removing the asset ID field from the accept wire message is important because it is redundant and isn't clear whether it is out_asset or in_asset. It also does not have a asset group key counterpart.

@ffranr ffranr added the rfq label Jun 6, 2024
@ffranr ffranr self-assigned this Jun 6, 2024
@ffranr ffranr requested review from guggero and GeorgeTsagk June 6, 2024 15:02
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Very nice, LGTM 🎉

ffranr added 3 commits June 6, 2024 16:13
This commit stores outgoing requests in the RFQ stream service and
retrieves them when handling corresponding quote accept messages. The
retrieved request is then set in a field on the quote accept message.
This commit deprecates the asset ID field in the accept message.
Instead, it uses the asset ID from the request field within the accept
message. This change allows for the removal of the asset ID field from
the accept message.
With the request message now included in accept messages, the asset ID
field in accept messages is redundant. This commit removes those
redundant asset ID fields.
@ffranr ffranr force-pushed the rfq-match-accept-to-requests branch from 60c8ed1 to d671060 Compare June 6, 2024 15:14
Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

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

LGTM

@guggero guggero merged commit 594dca0 into main Jun 7, 2024
14 checks passed
@guggero guggero deleted the rfq-match-accept-to-requests branch June 7, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants