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

Astroport DCA Module (Part 2) - Submission #1

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

Conversation

0xphilipp
Copy link

@0xphilipp 0xphilipp commented Jul 31, 2022

  • I decided implementing the bot tips using user specific tip jars. Where the contract will take funds from when performing executions
    • It would also be possible to store them for each DCA strategy, but this makes topping up fees a bit difficult for a user.
  • create_dca_order is currently working with allowances. When creating a DCA strategy for the same initial asset it is currently not validated that it is enough for all DCA strategies. I also prepared some pseudo code and already added a index for this to work correctly. But then the frontend needs to have the same calculation.
    • It would work easier by not using allowances and instead depositing cw20 assets into the contract aswell. (Would also increase TVL calculation.)
 let existing_dcas: Vec<DcaInfo> = state
                .dca_requests
                .idx
                .user_asset
                .prefix((info.sender.to_string(), initial_asset.info.to_string()))
                .range(deps.storage, None, None, Order::Ascending)
                .map(|item| {
                     let (_, res) = item.unwrap();
                    res.into()
                 })
                 .collect();
  • I kept the modify_dca_order in the contract, even though I would remove it to reduce complexity (complexity is bad for smart contracts) and instead use cancel_dca_order + create_dca_order
  • The integration testing is super basic right now and only the example happy case from the example flow is in there. But the most important part in the end makes it worth it, as it checks if the balances change based on the expected output.
  • I'm a bit worried about "bad" bots. If some user sets 10% or event 5% max_slippage, a bot could do MEV, by first buying assets, then running the dca buy, and then selling again. Also bots are incentivized to take the longest swap path.
  • Docs need to be checked if I did not oversee anything.
  • For integration tests, I added paths to local astroport-core contracts, so you will need to have them cloned in a sibling directory. I'm not as deep in rust / cosmwasm how to import non-package parts of other git repositories. See Cargo.toml
astroport-router = {path = "../../../astroport-core/contracts/router"}
astroport-pair = {path = "../../../astroport-core/contracts/pair"}
astroport-token = {path = "../../../astroport-core/contracts/token"}
astroport-factory = {path = "../../../astroport-core/contracts/factory"}
  • Tomorrow I will test it on testnet
  • It would be awesome to get feedback earlier than the deadline, as I will be away on vacation for one month from 6th august, and will not have good internet.

I hereby acknowledge, that the code will be under Astroport’s license.

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.

1 participant