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

initial compound workflow #80

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

initial compound workflow #80

wants to merge 2 commits into from

Conversation

harshraj172
Copy link
Collaborator

UI Workflow for Compound "Borrow", "Repay", "Supply" and "Withdraw".

Copy link
Collaborator

@sgzsh269 sgzsh269 left a comment

Choose a reason for hiding this comment

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

Nice stuff! Left a few comments.

One thing to note, when referencing other workflows, certain logic may only be applicable to that particular workflow so assess accordingly while copying


step1 = RunnableStep("confirm_borrow", WorkflowStepUserActionType.tx, f"{self.token} confirm borrow", self.step_1_confirm_borrow)

steps = [step1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

For single-step workflow may not be ideal to use MultiStepWorkflow abstraction, since we don't have a single step abstraction yet, you could implement something similar to what we have for the Aave workflow

Copy link
Collaborator Author

@harshraj172 harshraj172 May 10, 2023

Choose a reason for hiding this comment

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

post_body = route.request.post_data

# Intercepting below request to modify timestamp to be 5 minutes in the future to simulate block production and allow ENS web app to not be stuck in waiting loop
if "eth_getBlockByNumber" in post_body:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this special handling required for Compound?

I added it for ENS as its logic has a wait time between steps and relied on block production as a trigger to proceed to next step

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not specific to Compound, thought it is common to protocols.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's special handling for ENS, not sure about Compound, double check and if not needed, you can remove the entire overridden function _forward_rpc_node_reqs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I rechecked this, Compound is using this function

ui_workflows/compound/tests/test_compound_repay.py Outdated Show resolved Hide resolved
self.token = workflow_params['token']
self.amount = workflow_params['amount']

step1 = RunnableStep("enable_supply", WorkflowStepUserActionType.tx, f"{self.token} enable supply", self.step_1_enable_supply)
Copy link
Collaborator

Choose a reason for hiding this comment

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

For supplying tokens, if the token is non-ETH / ERC20 (like USDC, DAI, etc.), generally the user has to first give approval to the protocol to use their tokens so how is that approval step being handled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So step1 is for enabling the token's use on the platform. After step1 the user will have to approve the transaction from his/her wallet then it proceeds to step2.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok , so how is the supply of the native ETH token being handled as that shouldn't require any approval and would only require a single step?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea, so if the token is already enabled (if it native ETH or user enabled it before) then the step1 will return "success". See here: https://github.com/yieldprotocol/chatweb3-backend/blob/0b4cb5d3d8acced96062a1fd3244e5e26daba7fc/ui_workflows/compound/compound_repay.py#L78

@sgzsh269
Copy link
Collaborator

@harshraj172 Can this PR be refactored to use the new UI abstractions? You can look at Aave supply and borrow modules for reference https://github.com/yieldprotocol/chatweb3-backend/tree/dev/ui_workflows/aave/ui_integration

@harshraj172
Copy link
Collaborator Author

harshraj172 commented May 18, 2023

Okay @sgzsh269 , I refactor it according to the new UI abstractions.

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