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

Evaluate risks of keeping all the state in memory #1618

Closed
kelsos opened this issue Jun 2, 2020 · 3 comments · Fixed by #2110
Closed

Evaluate risks of keeping all the state in memory #1618

kelsos opened this issue Jun 2, 2020 · 3 comments · Fixed by #2110
Labels
discussion 💬 Issues that need discussion and decision taking sdk 🖥

Comments

@kelsos
Copy link
Contributor

kelsos commented Jun 2, 2020

Description

In our current approach, the whole SDK state is kept in memory while it currently works fine and it is not a big problem, we keep adding things. As we keep adding more state that is kept in memory and persisted in local storage.

In #1607 the balance proofs received and send for each channel will be also added in the state. We have seen in the python client that extended usage keeps increasing the database size. For the light client, all this data is also active in memory.

We should evaluate any possible risks or problems that might arise from this approach.

Acceptance criteria

Tasks

  • [ ]
@kelsos kelsos added sdk 🖥 discussion 💬 Issues that need discussion and decision taking labels Jun 2, 2020
@andrevmatos
Copy link
Contributor

Thank you very much for creating this issue. First, I think it isn't going to be a problem for the foreseable future, as you said, since we only store very simple objects, and the on-chain operations gas and off-chain fees requirements would be limitants for attackers to abuse state space constraints. Proactively, we could and should put safeguards in place to prevent some specific peer/partner/sender/channel state becoming unreasonably big.
Ideally, the whole state would be stored out-of-memory, but that would not be practical/possible with redux. Also, some state is kind of required to be readily available, since they're synchronously needed for specific validations, like latest channel state. Beyond that, I think we must first identify members which have potential to grow limitlessly, and find ways to store them somewhere else. They're still part of state, and would need to be put together and re-hydrated on state download/upload, but at least most of it wouldn't be needed to be available in memory all the time and could be requested by some kind of async service.

Some of the best candidates could be:

  • older (non-latest) balanceProof from us and partners (possibly needed for settleChannel)
  • old (settled) channels (not currently used for anything, hard to be abused since requires onchain txs)
  • completed sent and received transfers (unlocked off-chain, already unlocked on-chain, expired; needed only to avoid secret[hash] reuse and startup's emit for history)
  • matrix rooms with non-partners (e.g. initiators; we could have a policy in place to un-monitor users we don't care for anymore, and possibly leave rooms with them and clean state; would help both with redux and matrix's state sizes)

The rest of the state is mostly temporary (although required to be persisted across restarts), like pendingTx, or are of ~constant size (like pfs's ious), so the proposal would be to have some IndexedDB async service on RaidenEpicDeps (initially, possibly replaceable by a full-fledged database, e.g. for cli full clients), and if needed, fetch/search any value at runtime (e.g. searching old balanceProofs if a channel gets closed with a non-latest balanceHash of ours by partner). This part of the state would then be considered like a service/API, which needs to be async waited for results, not different e.g. from sending a message on transport.

@christianbrb
Copy link
Contributor

@kelsos We are now tackling this issue with #2044 - Thanks for bringing it up :)

@kelsos
Copy link
Contributor Author

kelsos commented Aug 7, 2020

Good to know 👍

@andrevmatos andrevmatos mentioned this issue Aug 26, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion 💬 Issues that need discussion and decision taking sdk 🖥
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants