-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add reserves list command to CLI #205
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
Conversation
|
The If JSON output is needed, this could be done by either changing how big decimal types are serialized , or by returning a json-serializable type from the command |
a426c14 to
0d2ec24
Compare
|
thinking of add more commands as well like users(--user address), like user summary, positions, suppplies, borrows.., depends on the feedback here 👀 |
|
Hey @Trynax sorry for the late response to this PR. We are kinda pretty busy on the v4 polishing aspects for the coming mainnet launch, so didn't have a lot of time to look into this PR in detail. From a quick scan, it seems it's overall in order with few things to fix. I'll see if I can put some time aside in the coming days to leave some comments. |
Thanks for the feedback @cesarenaldi I understand that you've been busy. I'll work on adding more commands to the brandg, so you can probably review them all together, or maybe we should conclude on this one first.👀 |
|
@Trynax we do have some plans around the CLI and other commands, can you please open an issue to discuss the commands you intend to add from a pure usage signature perspective? Just to want to make sure there is not unnecessary overlap and things align with the other work planned for this CLI. |
| function formatApy(apy: { normalized: unknown }, decimals = 4): string { | ||
| return `${parseFloat(String(apy.normalized)).toFixed(decimals)}%`; | ||
| } |
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.
| function formatApy(apy: { normalized: unknown }, decimals = 4): string { | |
| return `${parseFloat(String(apy.normalized)).toFixed(decimals)}%`; | |
| } | |
| function formatApy(apy: PercentNumber, decimals = 4): string { | |
| return `${apy.normalized.toFixed(decimals)}%`; | |
| } |
| `$${item.summary.supplied.fiatAmount.value}`, | ||
| `$${item.summary.borrowed.fiatAmount.value}`, |
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.
| `$${item.summary.supplied.fiatAmount.value}`, | |
| `$${item.summary.borrowed.fiatAmount.value}`, | |
| `${item.summary.supplied.fiatAmount.symbol}${item.summary.supplied.fiatAmount.value.toFixed(2)}`, | |
| `${item.summary.borrowed.fiatAmount.symbol}${item.summary.borrowed.fiatAmount.value.toFixed(2)}`, |
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.
Formatting can be fixed with pnpm lint:fix from the root of the repo, or installing Biome.js extension on your IDE to format on save.
|
Thanks for the review @cesarenaldi , I've addressed the feedbacks. Note: I had to change the currency formatting to use I'll open an issue regarding the cli soon. |
Also, I'm unable to test the cli commands right now(including the previous ones) looks like the graphql api hasn't been deployed with the new schema yet👀 (getting "Unknown field 'exchange'" errors on both production and staging). |
You need to rebased on latest Also make sure you rebuild the solution since the CLI will use the built version of the subpackages. |
@cesarenaldi I Just realized the cli default to production, which still has the old schema: # Production has old schema (
$ curl -s -X POST https://api.v4.aave.com/graphql \
-H "Content-Type: application/json" \
-d '{"query":"{ __type(name: \"Erc20Amount\") { fields { name } } }"}' | jq '.data.__type.fields[].name'
"amount"
"fiatAmount" # Old field
"fiatRate" # Old field
"isWrappedNative"
"token"
# Staging has new schema
$ curl -s -X POST https://api.v4.staging.aave.com/graphql \
-H "Content-Type: application/json" \
-d '{"query":"{ __type(name: \"Erc20Amount\") { fields { name } } }"}' | jq '.data.__type.fields[].name'
"amount"
"exchange" # New field
"exchangeRate" # New field
"isWrappedNative"
"token"so I had to hardcode the cli to use staging to test the cli and it works fine, but i suppose the right one should be using the ENVIROMENT set in .env i can add that to the PR if that's fine. |
|
yeah, we should update production at some point this week. Generally speaking staging env is not meant for general usage, it's mostly reserver for development/testing our end. Would be interesting if we could figure out something with the base class where it adds a |
8bd5742 to
9cb04fb
Compare
Yeah think this would be better, implemented it already. usage would be like: |
packages/cli/src/common.ts
Outdated
|
|
||
| BigInt.prototype.toJSON = function () { | ||
| return this.toString(); | ||
| }; |
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.
@Trynax can you please clarify the reason for this patch?
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.
@Trynax can you please clarify the reason for this patch?
It fix the --json flag error when any response have a bigint value, without it it produce TypeError: Do not know how to seriaize a BigInt
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.
I would rather considering traversing the data structure and return a string in place of BigInt instances instead of patching a global at the module level of one of the modules.
There is a toSuccessJson protected method that could be overridden by any command to achieve this sort of things.
A relatively simple traversing traversing that, no matter the shape of the json: unknown, finds any leaf that are instance of JS BigInt (typeof x === 'bigint') and return the toString of it.
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.
@cesarenaldi Thanks for this.
I've added a func that recursively traverses the data structure
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.
Thank you @Trynax for the change.
ee88f21 to
ab8ca11
Compare
|
@Trynax tests are failing due to the lack of secrets/env variables (GH omits them when running on forks). Given the nature of your changes I have no reason to think there is a actual issue under the hood. I would only need you to add a changeset. Run (from root folder): and select Write a comment starting with:
as needed, and commit the file. For example: like your PR title is already a good entry. |
|
gm @cesarenaldi, just followed up and added the changeset. |
|
Thank you @Trynax merging this. |
Nice, thank you😁 hopefully more to come. |

Hey, good day,
I noticed the cli package and was going through it realized there's a lot more commands that could be added to make it even more useful.
I added
reserves listcommandThe command has flexible query options:
--spoke_id- List reserves for a specific spoke--hub_id- List reserves for a specific hub--chain_id- List reserves for a specific chain--hub_addresswith--chain_id- List reserves for a hub by addressThe command displays infos like:
Follows the existing command structure and patterns used in
hubs listandspokes listand updated the cli readme.screenshot from usage below: