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

Fund manager might get stale #2685

Closed
arajasek opened this issue Jul 30, 2020 · 6 comments
Closed

Fund manager might get stale #2685

arajasek opened this issue Jul 30, 2020 · 6 comments
Assignees
Labels
P2 P2: Should be resolved

Comments

@arajasek
Copy link
Contributor

I think the fund manager has the potential to get into a bad state. For instance, if the deal that required the funds for which EnsureAvailable was called fails, then there'll be more available than the fund manager thinks there is.

I don't see anyway the fund manager can be informed "oh, nvm, deal failed, increase the available balance", but maybe this happens somehow?

@hannahhoward
Copy link
Contributor

hannahhoward commented Jul 30, 2020

@arajasek it seems like ensure funds is keeping a cache of balances, but the only thing it's caching is reading from the StateManager which is not ... a super long operation worth caching. Is there any reason not to just delete this cache and just read directly from the statemanager?

@hannahhoward
Copy link
Contributor

@magik6k or anyone who has visibility into the rationale for that cache -- are you ok with this? If so I'll just do that.

@arajasek
Copy link
Contributor Author

@hannahhoward I only discovered this exists yesterday, but I think this exists to provide some "foresight". If the order goes deal1 is processed, deal1's balance updates land on state, deal2 is processed, deal2 updates land in state, your proposal would be fine.

But what if it goes deal 1 is processed, deal 2 is processed, deal 1 lands in state, deal 2 lands in state? In that case, I'm guessing use state as your "source of truth" wouldn't work.

The question is how likely that is -- what happens between EnsureAvailable being called and the changes landing?

@hannahhoward
Copy link
Contributor

ah ok makes sense.

@arajasek
Copy link
Contributor Author

arajasek commented Aug 4, 2020

Update from @hannahhoward

after investigating the code for the fundmanager today, it looks like it was doing an extremely incomplete version of what we’re now doing in markets — tracking how much total is needed for deals. Now that we’re doing it in markets it’s not needed in fund manager — and we may actually see double adds of funds in the reset. But also, yes there was minimal tracking of on chain stuff and the value it tracks will get very stale. I’m working on an update across the board.

@arajasek arajasek added the P2 P2: Should be resolved label Nov 6, 2020
@arajasek arajasek mentioned this issue Nov 6, 2020
2 tasks
@jennijuju
Copy link
Member

resolved in #4736

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 P2: Should be resolved
Projects
None yet
Development

No branches or pull requests

4 participants