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

Potential leak in v17 (bank) and v18 (smart wallet) #6611

Closed
arirubinstein opened this issue Nov 30, 2022 · 3 comments
Closed

Potential leak in v17 (bank) and v18 (smart wallet) #6611

arirubinstein opened this issue Nov 30, 2022 · 3 comments
Assignees
Labels
bug Something isn't working resource-exhaustion Threats to availability from resource exhaustion attacks vaults_triage DO NOT USE wallet

Comments

@arirubinstein
Copy link
Contributor

arirubinstein commented Nov 30, 2022

on agoric-3, it appears that v17 and v18 are growing in size over time:

Screenshot 2022-11-30 at 3 37 51 PM

Screenshot 2022-11-30 at 3 37 46 PM

@arirubinstein arirubinstein added the bug Something isn't working label Nov 30, 2022
@arirubinstein arirubinstein changed the title Potential leak in v17 (bank) and v18 (zcf/psm) Potential leak in v17 (bank) and v18 (smart wallet) Dec 1, 2022
@dckc dckc added wallet resource-exhaustion Threats to availability from resource exhaustion attacks labels Dec 2, 2022
@dckc
Copy link
Member

dckc commented Dec 2, 2022

It's less surprising that vat-bank grows; #5885 is still open.

#4489 is closed, so the smart wallet should be using virtual objects to limit heap growth.

@dckc
Copy link
Member

dckc commented Dec 6, 2022

@mhofman pointed out that this closure likely keeps all representatives in the heap rather than just in virtual storage:

// watch the bank for new issuers to make purses out of
void observeIteration(E(bank).getAssetSubscription(), {

@turadg turadg self-assigned this Dec 6, 2022
@rowgraus rowgraus added the vaults_triage DO NOT USE label Jan 3, 2023
@ivanlei ivanlei added this to the Vaults RC0 milestone Feb 1, 2023
@turadg
Copy link
Member

turadg commented Feb 14, 2023

I think @dckc fixed this in #6774. There's only one observer, within the asset registry shared by all wallets:
https://github.com/Agoric/agoric-sdk/blame/9aa48736a3b158f8e8746e815f7a94887e02b46e/packages/smart-wallet/src/walletFactory.js#L64-L78

@turadg turadg closed this as completed Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resource-exhaustion Threats to availability from resource exhaustion attacks vaults_triage DO NOT USE wallet
Projects
None yet
Development

No branches or pull requests

5 participants