Skip to content

Niv/cdp cli#99

Open
nmimran99 wants to merge 4 commits intomainfrom
niv/cdp-cli
Open

Niv/cdp cli#99
nmimran99 wants to merge 4 commits intomainfrom
niv/cdp-cli

Conversation

@nmimran99
Copy link
Collaborator

@nmimran99 nmimran99 commented Dec 23, 2022

Add CLI Commands to interact with CDP Pool.
Currently only written for wstETH CDP pool.
A generic CDP Pool interaction with different token is coming up.

commands:
basic command
npx ts-node cli/cli -c <networkId> -m <mnemonic> -n <accountNumber>

Dictionary:
depositToken = [ steth | weth | eth ]
amounts = without decimals. e.g. for 2 ETH = 2, for 1200 PHO = 1200

Write commands:
<baseCommand> modules cdp open wsteth <depositToken> <collateralAmount> <debtAmount>
<baseCommand> modules cdp add-collateral wsteth <depositToken> <collateralAmount>
<baseCommand> modules cdp remove-collateral wsteth <collateralAmount>
<baseCommand> modules cdp add-debt wsteth <debtAmount>
<baseCommand> modules cdp remove-debt wsteth <debtAmount>
<baseCommand> modules cdp close wsteth
<baseCommand> modules cdp liquidate wsteth <cdpOwner> * To be called by liquidator *

Read command:
<baseCommand> modules cdp overview wsteth
<baseCommand> modules cdp position wsteth <cdpOwner>

General notes:
The caller will have to manually get the deposit token, meaning converting ETH to WSTETH/STETH/WETH. If the caller is calling to the mainet fork, the approval to use these token by the wrapper contract will be done automatically, otherwise the caller will have to manually approve the wrapper contract to transferFrom.

The CLI was tested with deposit token as stETH only for now. I will further test with other tokens.

Copy link
Contributor

@davekay100 davekay100 left a comment

Choose a reason for hiding this comment

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

Mostly small changes. I am going to also just do a run through of all the write commands myself tomorrow just to do a quick double check on them working.

There is one thing we should add now:

  • i think we would do well to create an example README in the CLI or at the root of the project with an example CLI command for each CLI command created. this is easier for anyone to review, and recreate these contracts, rather than reading the code and piecing together over a few minutes how to write teh command. can you add this?

And one thing for the future

  • i dont think we need to do cast calls on everything. we should use typescript to its advantage. maybe create a typescript type for in types.ts which is export interface Modules. and then we can pass that into PhotonContracts interface, which then will exist in `CLIEnvironment. we don’t have to update it now, we can merge this since it works. but i think this is a good future enhancement. it involves a lot less string manipulation.


export const mineCommand = {
command: 'mine',
describe: 'deploy contracts from deployParams.json',
Copy link
Contributor

Choose a reason for hiding this comment

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

describe should be updated. looks like a copy paste error


export const cdpCommand = {
command: 'cdp',
describe: 'Photon protocol modules',
Copy link
Contributor

Choose a reason for hiding this comment

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

this describe message needs to be updated

startTime.toString().concat(` (${new Date(Number(startTime) * 1000).toLocaleDateString()})`)
])
table.push(['status', status.toString()])
table.push(['Total Collateral', toReadablePrice(totalCollateral)])
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be helpful to include Total Collateral in ${cliArgs.tokenType} or something like that, just as an indicator what the collateral is. for me it just says 27.228

table.push(['Total Collateral', toReadablePrice(totalCollateral)])
table.push(['Total Debt', toReadablePrice(totalDebt)])
table.push(['Collateral Ratio', collRatio.slice(0, -3).concat('%')])
table.push(['feesCollected', toReadablePrice(feesCollected)])
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2023-01-03 at 5 36 33 PM

i think this is being displayed wrong with toReadablePrice(). i see this massive number, i think it should be 10^18 less

'startTime',
startTime.toString().concat(` (${new Date(Number(startTime) * 1000).toLocaleDateString()})`)
])
table.push(['status', status.toString()])
Copy link
Contributor

Choose a reason for hiding this comment

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

right now I just see status: 1. i think cuz this is a table for displaying reading, we should just replace all the numbers like 1 to their actual status ACTIVE, which an enum


table.push(['CDP Owner', cdpOwner])
table.push(['Debt', toReadablePrice(debt)])
table.push(['Collateral', toReadablePrice(collateral)])
Copy link
Contributor

Choose a reason for hiding this comment

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

as suggested above, this makes it easier to read:

  table.push([`Collateral in ${cliArgs.tokenType}`, toReadablePrice(collateral)])

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

Comments