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

Add general RFQ accept wire msg type #937

Merged
merged 6 commits into from
Jun 7, 2024
Merged

Add general RFQ accept wire msg type #937

merged 6 commits into from
Jun 7, 2024

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Jun 7, 2024

Closes #914

This is the final PR towards closing #914 .

This PR introduces a new quote accept wire message type that can be used by both buy and sell accept messages. Effectively using a single wire message type for both buy and sell.

In this PR we also remove a redundant field from the buy/sell message types.

ffranr added 6 commits June 7, 2024 15:55
This commit adds a quote accept message type which serves as a single
wire message type for both buy and sell quote accept messages.
Type `sellAcceptMsgData` is redundant and can be removed following the
introduction of type `acceptWireMsgData`.
Type `buyAcceptMsgData` is redundant and can be removed following the
introduction of type `acceptWireMsgData`.
The `AssetAmount` field is redundant as it is a copy of the value found
in `rfqmsg.BuyAccept.Request.AssetAmount`. It is therefore removed in
this commit.
The `AssetAmount` field is redundant as it is a copy of the value found
in `rfqmsg.SellAccept.Request.AssetAmount`. It is therefore removed in
this commit.
@ffranr ffranr added the rfq label Jun 7, 2024
@ffranr ffranr requested a review from guggero June 7, 2024 15:31
@ffranr ffranr self-assigned this Jun 7, 2024
@dstadulis dstadulis added this to the v0.4 milestone Jun 7, 2024
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! tACK, verified against the integration test 💯

// Sig is a signature over the serialized contents of the message.
Sig tlv.RecordT[tlv.TlvType3, [64]byte]

InOutRateTick acceptInOutRateTick
Copy link
Member

Choose a reason for hiding this comment

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

nit: missing Godoc comments.

Copy link
Member

Choose a reason for hiding this comment

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

Will address in my PR.

@guggero guggero merged commit 4bcdbde into main Jun 7, 2024
14 checks passed
@guggero guggero deleted the rfq-accept-wire-msg branch June 7, 2024 16:18
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.

[feature]: RFQ Wire message updates: split asset_amt, implicit asset_id types reduction
3 participants