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 new EdgeTxInfo types #567

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Add new EdgeTxInfo types #567

merged 1 commit into from
Oct 11, 2023

Conversation

Jon-edge
Copy link
Contributor

@Jon-edge Jon-edge commented Oct 6, 2023

CHANGELOG

  • added: EdgeTxInfo types to tag known transaction types (swap, stake, etc)

Dependencies

none

Description

none

export interface EdgeTxInfoSwap {
type: 'swap' | 'swapOrderPost' | 'swapOrderFill'
orderId?: string
direction: 'from' | 'to'
Copy link
Contributor Author

@Jon-edge Jon-edge Oct 6, 2023

Choose a reason for hiding this comment

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

I question the need for this. In our exchange plugins this denotes which side is
the guaranteed amount - the payment or receive amount. In this API, we already
define a nativeAmount for both sides in EdgeTxAsset. We can just make
nativeAmount optional to denote an unknown swap destAsset amount for some order
with slippage. Fills will then have both src/dest with nativeAmount populated.

@Jon-edge Jon-edge force-pushed the jon/edge-tx-info branch 2 times, most recently from bd973b5 to 81fa834 Compare October 10, 2023 17:29
Copy link
Contributor

@swansontec swansontec left a comment

Choose a reason for hiding this comment

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

Approving this part, but we need to do more inner plumbing to get the data sent to the right place.

@Jon-edge Jon-edge enabled auto-merge October 11, 2023 00:11
@Jon-edge Jon-edge merged commit 90dd451 into master Oct 11, 2023
3 checks passed
@Jon-edge Jon-edge deleted the jon/edge-tx-info branch October 11, 2023 00:16
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