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

state of mango program, thoughts on refactoring/clean-up in 2022 #101

Open
microwavedcola1 opened this issue Dec 30, 2021 · 12 comments
Open

Comments

@microwavedcola1
Copy link
Contributor

microwavedcola1 commented Dec 30, 2021

placeholder issue, will update with some thoughts later, some pain points I recently encountered

  • having to figure out yet another low level component e.g. bincode
  • need to constantly scroll across 5k lines of processor.rs while working on anything every few mins
  • new contributors would prefer contributing to an anchor codebase, Update: anchor seems to have a fallback function thingy, which could help gradual migration
  • tests are not covering whole program
  • should we keep on pushing features irrespective of when/if we rewrite/gradually migrate to anchor?

would be great if people could add their own suggestions, we can consolidate, prioritize and start chopping them off

@microwavedcola1 microwavedcola1 changed the title wip: big bang rewrite in anchor v gradual rewrite in anchor v just refactoring current codebase state of mango program, thoughts on refactoring/clean-up in 2022 Dec 30, 2021
@mschneider
Copy link
Contributor

It’s probable that the value of anchor integrations could be unlocked by a simple cpi library that simplifies interacting with mango similar to how serum is integrated

@dafyddd
Copy link
Contributor

dafyddd commented Jan 9, 2022

I agree scrolling across all these lines is a bit cumbersome. At this point I just have to use ctrl f for everything. But one advantage of current setup is simplicity and logic. Every instruction has an equivalent function in processor.rs. A refactor that would continue to be simple and logical would be nice if you can think of one.

Do you know all the drawbacks of going to Anchor? It would be useful to even just see the difference in execution speed

Tests were started by another dev who didn't see it all the way through. I think we ultimately moved on to testing in devnet because it takes just as much time and seems more realistic. If you could clean up the tests we could start using it again. By clean up I mean removing unnecessary files and making sure all instructions are hit and it's relatively fast to run the tests.

@ckamm
Copy link
Contributor

ckamm commented Jan 10, 2022

Something I just came across was that some function signatures involving AccountInfo (like pub fn init_vals(..., open_orders_ais: &[AccountInfo; MAX_PAIRS]) in HealthCache) make mango logic unnecessarily hard to reuse externally.

The reason is that AccountInfos contain mut references and can sometimes be impossible to create without extra copies. Refactoring to parse these accounts outside and then only feeding in the results would simplify that a lot. I'll probably do that directly since it makes my current project easier.

@dafyddd
Copy link
Contributor

dafyddd commented Jan 10, 2022

@ckamm Interesting point. I believe it's the way it is right now because a lot of those AccountInfo objects are actually of the zero key. The new way of passing these in is to use Option<&OpenOrders> or a Vec of the same. But older code required users to just pass in all the open orders accounts in a big array and leave zero key for the ones not in margin basket. Just something to keep in mind when refactoring.

@microwavedcola1
Copy link
Contributor Author

todo: cleanup branches which have been abandoned, not-relevant anymore

@microwavedcola1
Copy link
Contributor Author

todo: some of the instructions dont have a corresponding helper instruction, useful for programs composing on top of mango, double check, and add missing ones

@microwavedcola1
Copy link
Contributor Author

microwavedcola1 commented Jan 11, 2022

todo: something like anchor debug would be nice while developing tests, so that we dont have to manually print all the keys

@microwavedcola1
Copy link
Contributor Author

ckamm: another thing for a cleanup/refactoring: passing these as arguments is error prone. It'd be nice if ChangeSpotMarketParams was a struct that could just be passed onwards to the executor function

@microwavedcola1
Copy link
Contributor Author

dark green highlights are instructions which dont have a correponding test atm

image

@microwavedcola1
Copy link
Contributor Author

whatever we do should take into account timeline for next protocol re-write, could be potential waste of effort

@microwavedcola1
Copy link
Contributor Author

the problem with using one of the accounts not from parameters but from import confuses api users, unless one enables full debug logging its hard to know that the token program also has to be sent as a account info while cpi'ing
image

@microwavedcola1
Copy link
Contributor Author

nice sdk like https://github.com/zetamarkets/fuze or spl-token sdk

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

No branches or pull requests

4 participants