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

feat: silk basket query #139

Merged
merged 3 commits into from
Apr 26, 2024
Merged

feat: silk basket query #139

merged 3 commits into from
Apr 26, 2024

Conversation

AustinWoetzel
Copy link
Collaborator

@AustinWoetzel AustinWoetzel commented Apr 26, 2024

Contains:

  • Silk basket message
  • service
  • error handling/retry
  • unit tests
  • docs

Copy link

changeset-bot bot commented Apr 26, 2024

🦋 Changeset detected

Latest commit: e0ffc1f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@shadeprotocol/shadejs Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -15,6 +15,7 @@ This page contains a list of deployed contracts.
| Lend Vault Registry (V2) | secret1qxk2scacpgj2mmm0af60674afl9e6qneg7yuny | ac5d501827d9a337a618ca493fcbf1323b20771378774a6bf466cb66361bf021 |
| Lend Vault Registry (V3) | secret1wj2czeeknya2n6jag7kpfxlm28dw7q96dgqmfs | d837f716de3732a4118fbcb6d4cd0ef1d84ee83fef924f27b7c2a821f8528b39 |
| Lend Stability Pool | secret1wdxqz26acf2e6rsac8007pd53ak7n8tgeqr46w | 4dcdce6a2f88ef2912b9988119b345b096909aa4ba3881eff19358d983c40210 |
| Index Oracle (SILK) | secret1552yh3rplmyrjwhcxrq0egg35uy6zwjtszecf0 | 8d2b439383091ecb7806757a2b202e0056e542ade67951a0d5c352e74ce416cc |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: formmating

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

minBlockHeightValidationOptions,
}).pipe(
map(parseSilkBasketAndPricesResponseFromQueryRouter),
switchMap((data) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better with a try-catch type of setup?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

better is probably subjective. I don't really see it as an error, because nothing really went wrong, service is operating as expected under those conditions using the output of query1 to feed query2. You're maybe alluding to the design choice of adding that parsing status type, and that is maybe a bit confusing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And I guess to add one more thing, if we did want to do it as an error, then I would need to pass parameters for query #2 back through the error message and I don't necessarily love that solution either.

@AustinWoetzel AustinWoetzel merged commit cb72124 into develop Apr 26, 2024
4 checks passed
@AustinWoetzel AustinWoetzel deleted the silk-basket-query branch April 26, 2024 21:04
Copy link

@antonymous-a antonymous-a left a comment

Choose a reason for hiding this comment

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

Just a documentation nit and a proposal for a new type for SilkBasket & { status: SilkBasketParsingStatus }

@@ -0,0 +1,76 @@
import { SilkBasket, SilkBasketParsingStatus } from '~/types/contracts/silkBasket/model';

const silkBasketWithMissingPricesParsed: SilkBasket & { status: SilkBasketParsingStatus } = {

Choose a reason for hiding this comment

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

refactor SilkBasket & { status: SilkBasketParsingStatus } into its separate type for better porting

| Lend Vault Registry (V1) | secret18y86hldtdp9ndj0jekcch49kwr0gwy7upe3ffw | 148a525ec7bffedfc41cbc5339bf22d9e310d49b65831a269c86774fb732948c |
| Lend Vault Registry (V2) | secret1qxk2scacpgj2mmm0af60674afl9e6qneg7yuny | ac5d501827d9a337a618ca493fcbf1323b20771378774a6bf466cb66361bf021 |
| Lend Vault Registry (V3) | secret1wj2czeeknya2n6jag7kpfxlm28dw7q96dgqmfs | d837f716de3732a4118fbcb6d4cd0ef1d84ee83fef924f27b7c2a821f8528b39 |
| Lend Stability Pool | secret1wdxqz26acf2e6rsac8007pd53ak7n8tgeqr46w | 4dcdce6a2f88ef2912b9988119b345b096909aa4ba3881eff19358d983c40210 |
| Lend Stability Pool | secret1wdxqz26acf2e6rsac8007pd53ak7n8tgeqr46w | 4dcdce6a2f88ef2912b9988119b345b096909aa4ba3881eff19358d983c40210 |
| Index Oracle (SILK) | secret1552yh3rplmyrjwhcxrq0egg35uy6zwjtszecf0 | 8d2b439383091ecb7806757a2b202e0056e542ade67951a0d5c352e74ce416cc |

Choose a reason for hiding this comment

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

Rename Index Oracle to Peg Index Oracle for better readability of docs

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.

3 participants