Skip to content

Conversation

@callebtc
Copy link
Collaborator

Summary

  • ensure Cashu wallet/database handles always close via closeResources helper
  • guard ItemManager image import streams with use{} and early returns
  • wrap MintIconCache http calls with use to auto close response/streams

Testing

  • ./gradlew lintDebug testDebugUnitTest (fails locally: Android SDK not configured in CI env)

@callebtc
Copy link
Collaborator Author

utACK

@callebtc
Copy link
Collaborator Author

callebtc commented Dec 1, 2025

Review Summary

Thanks for tightening up the resource handling. The new closeResources() helper in CashuWalletManager and the scoped use { ... } blocks in MintIconCache are good steps toward eliminating leaks.

However, the change in ItemManager.saveItemImage() introduces a regression. By wrapping the InputStream in use { ... } and then falling through the method, the function now always returns false, even when the image was saved successfully. Previously the method returned true from inside the if (inputStream != null) block. We need to keep returning true on success (and only false when no stream is available or an exception is thrown); otherwise callers will treat every save as a failure.

Once that’s addressed, the PR should be in good shape.

Scores (0–10)

  • Effectiveness: 4 (regression outweighs the leak fixes)
  • Code Style: 8 (clean, consistent Kotlin idioms)
  • Safety & Robustness: 3 (always returning false breaks callers)

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.

2 participants