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

Split SQLite Wallet into separate package #2679

Closed

Conversation

devhawk
Copy link
Contributor

@devhawk devhawk commented Mar 22, 2022

As per #2678, the EF + SQLite dependency bloats the uncompressed size of Neo.dll + depenencies (as generated by dotnet publish) by 5x (28MB vs 6.2MB). Initial test of compressed package size shows a 60% reduction (17.6MB vs 7MB for Neo.Compiler.CSharp compressed nuget package).

EF + SQLite are only used for legacy UserWallet. GIven the significant size bloat this dependency add and the relatively low usage pattern for UserWallet, it seems worth it to move UserWallet to a separate package. Tools + libraries that don't need UserWallet support (Neo.Compiler.CSharp, Neo Debugger, Neo Blockchain Library) see a significant size reduction while tools and libraries that do need UserWallet support (RpcServer, NeoCli, NeoGui, NeoExpress) can continue to get that support by adding the new package dependency.

cc @erikzhang @shargon
fixes #2678

This was referenced Mar 22, 2022
@erikzhang
Copy link
Member

I think it should be moved to a plugin.

@devhawk
Copy link
Contributor Author

devhawk commented Mar 27, 2022

I think it should be moved to a plugin.

I think that's a good idea - as suggested by @Liaojinghui and tracked by #2564 - but I don't think we have enough time to make a change that fundamental before releasing v3.2.

The change in this PR is a simple packaging one. Packages that don't use UserWallet like NCCS make no change but get >50% space savings. Packages that do use UserWallet (RpcServer, NeoCli) only have to add a single package dependency and have all the same functionality they have today.

Can we merge this PR now, realize the package size savings for the v3.2 release and work on a new wallet abstraction + module for v3.3?

@erikzhang
Copy link
Member

It cannot be merged because it creates sqlite-wallet. I don't think it's necessary to create an extra package.

@devhawk
Copy link
Contributor Author

devhawk commented Mar 29, 2022

It cannot be merged because it creates sqlite-wallet. I don't think it's necessary to create an extra package.

@erikzhang Would you support removing SQLite wallet support from neo.dll completely? We could move the UserWallet code to neo-cli so users can still migrate SQLite wallets. I actually like that approach better because it would also remove the SQLite dependency from RpcServer

@erikzhang erikzhang closed this May 9, 2022
@devhawk devhawk deleted the devhawk/sqlite-wallet-package branch April 29, 2023 20:47
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.

Slim down neo package size
2 participants