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 Unizen DEX #360

Merged
merged 4 commits into from
Jan 14, 2025
Merged

Add Unizen DEX #360

merged 4 commits into from
Jan 14, 2025

Conversation

peachbits
Copy link
Contributor

@peachbits peachbits commented Jan 2, 2025

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Description

none

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.

Approved, but there are some questions and worries.


const UNIZEN_CONTRACTS: {
[version: string]: StringMap
} = JSON.parse(JSON.stringify(contracts))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why round-trip this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! will fix before merging

const crossChain = asMaybe(asCrossChainSwapQuote)(rawQuote)
if (crossChain != null) {
const memos: EdgeMemo[] = []
const memoValue = crossChain[0].transactionData.memo
Copy link
Contributor

Choose a reason for hiding this comment

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

The asCrossChainSwapQuote is defined using asArray, but here we only access crossChain[0]. What if there are multiple tx's in the quote? Would the customer lose money, because the funds don't get fully swapped? I would feel safer either using asTuple to enforce a single item in the array, or somehow looping over the tx's to handle the weird case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an array of quotes and the best one is the first one. I'll change improve the cleaner name and change to asTuple to enforce it's existance

@peachbits peachbits merged commit 8f0ac2e into master Jan 14, 2025
1 check passed
@peachbits peachbits deleted the matthew/unizen branch January 14, 2025 00:47
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