-
Notifications
You must be signed in to change notification settings - Fork 92
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
Query Improvements - Making the build queries interface easier to use #473
Conversation
lgahdl
commented
Jun 14, 2023
- Doesn't need to send all amounts In/Out for all tokens, only the tokens that will be used;
- Doesn't need to send all amounts for all tokens, only the tokens that will be used;
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.
balancer-js/examples/pools/queries.ts
needs updated.
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.
How about instead of separate tokens and amounts we change the interface to accept a map? It might be easier to construct and avoids problems with accidental misattribution by ordering the arrays wrong.
{ [address: string]: string }
the same for return data. we could format results to a map, so it's easy to read / parse
I might be wrong but I think @gmbronco was suggesting we change the function input params to be a map of tokens addresses and amounts? |
You're right, sorry, will fix that |
let maxInWithoutBpt; | ||
|
||
// Remove BPT token from amounts for user data | ||
if (bptIndex > -1) { | ||
maxInWithoutBpt = removeItem(maxAmountsIn, bptIndex); | ||
} else { | ||
maxInWithoutBpt = maxAmountsIn; | ||
} |
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.
let maxInWithoutBpt; | |
// Remove BPT token from amounts for user data | |
if (bptIndex > -1) { | |
maxInWithoutBpt = removeItem(maxAmountsIn, bptIndex); | |
} else { | |
maxInWithoutBpt = maxAmountsIn; | |
} | |
let maxInWithoutBpt = maxAmountsIn; | |
// Remove BPT token from amounts for user data | |
if (bptIndex > -1) { | |
maxInWithoutBpt = removeItem(maxAmountsIn, bptIndex); | |
} |
The changes are made and John is on vacation, I'll dismiss the review so I can merge it