-
Notifications
You must be signed in to change notification settings - Fork 5
feat(order): Create an ordering class for limit and recurring orders #105
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
base: main
Are you sure you want to change the base?
Conversation
…to differentiate between two versions of BaseClient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces an ordering class to support both limit and recurring orders while refactoring related utilities and updating client implementations. Key changes include:
- The addition of a new intToBytes utility in src/util/util.ts and its consolidation across the codebase.
- The creation of new order-specific modules (types, constants, and utilities) to support the ordering functionality.
- Updates to the TinymanBaseClient in several modules to use generics and improve error handling for missing application IDs.
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/util/util.ts | Added intToBytes helper with bitwise logic and ESLint disable/enable comments. |
src/util/client/base/* | Refactored BaseClient to use generics and improved error checking. |
src/order/* | Introduced order utilities, types, and constants for limit and recurring orders. |
src/liquid-stake/, src/governance/ | Updated import paths and removed duplicate intToBytes implementations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gulcinuras looks good overall! Left a few comments
Also added GitHub Copilot AI as a reviewer. It's not replacement for real reviewers yet but it is useful for catching small stuff like typos, or forgotten TODOs etc.
`${process.env.PUBLIC_URL}/contracts/order_approval.teal` | ||
); | ||
const clearSourceResponse = await fetch( | ||
`${process.env.PUBLIC_URL}/contracts/order_clear_state.teal` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we replace PUBLIC_URL with real values? I am not sure if js-sdk repo has .env file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will use this util function to create the order app. Currently the compiled programs are encoded and stored as constants. Once the contracts are public, I will update the urls 👍🏾
src/order/index.ts
Outdated
const sp = await this.getSuggestedParams(); | ||
|
||
const orderBoxName = this.getOrderBoxName(orderId, type); | ||
const order = await this.getBox(orderBoxName, "Order"); | ||
|
||
if (!order) { | ||
throw new Error("Order not found"); | ||
} | ||
|
||
const txns = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const sp = await this.getSuggestedParams(); | |
const orderBoxName = this.getOrderBoxName(orderId, type); | |
const order = await this.getBox(orderBoxName, "Order"); | |
if (!order) { | |
throw new Error("Order not found"); | |
} | |
const txns = [ | |
const orderBoxName = this.getOrderBoxName(orderId, type); | |
const order = await this.getBox(orderBoxName, "Order"); | |
if (!order) { | |
throw new Error("Order not found"); | |
} | |
const sp = await this.getSuggestedParams(); | |
const txns = [ |
This is minor but in this version in case of order is not found, we wouldn't await for suggestedParams unnecesarily
src/order/index.ts
Outdated
return totalFee; | ||
} | ||
|
||
async putOrder({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to add some jsdoc style comments especially for the functions that client uses to explain what they do and when they should be used.
For example,
async putOrder({ | |
/** | |
* Creates a buy order with the given params | |
* @param duration - Duration in seconds after which the order will expire | |
* ... other params | |
*/ | |
async putOrder({ |
Also adding simple examples of how to create recurring or trigger order using this class would be very useful. It can be even added to README.md if adding it under examples/
would take some time
…ignerTransactions util to base client
…rotected to public
No description provided.