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

removes 'clear*' methods from the walletDb #5639

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

hughy
Copy link
Contributor

@hughy hughy commented Nov 12, 2024

Summary

the walletDb has serveral leftover implementations of methods that clear datastores

these methods are unused

not only are these methods unused, but they should not be used: calling 'clear' on a datastore has the potential to block the wallet and node process while deleting from the database. data are deleted from these stores incrementally in the background using 'cleanUpDeletedAccounts' for this reason

Testing Plan

N/A, methods unused

Documentation

Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference
)? If yes, link a
related documentation pull request for the website.

[ ] Yes

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and label it with breaking-change-rpc or breaking-change-sdk.

[ ] Yes

the walletDb has serveral leftover implementations of methods that clear
datastores

these methods are unused

not only are these methods unused, but they _should not_ be used: calling
'clear' on a datastore has the potential to block the wallet and node process
while deleting from the database. data are deleted from these stores
incrementally in the background using 'cleanUpDeletedAccounts' for this reason
@hughy hughy requested a review from a team as a code owner November 12, 2024 22:51
@hughy hughy merged commit ab377a4 into staging Nov 12, 2024
13 checks passed
@hughy hughy deleted the chore/hughy/clear-walletdb-clears branch November 12, 2024 23:03
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.

3 participants