Alternative to rocksdb: Added jammdb as another lightweight option for wallet storage #1004
Conversation
|
I fixed that issue. Please resolve conflicts |
|
Also, can you please change the base branch to 1.1? We'd probably release that together with other updates. |
|
I don't think the merge went okay. Also, regarding the |
|
ahh yes. I understood my error. Fixing it now. It works after replacing () with {}. cargo check also works! Thanks for the input. |
|
The merge is seriously eff'd. You may want to start over if you don't feel that you can fix it |
|
I dont understand what remains now to be fixed. |
|
I have resolved the newer conflicts. Its very hard to understand the cryptic and unregulated comments as well as hard to follow two lines of action coming in from different devs. If you feel like merging this in the up stream, let us know, otherwise we will keep maintaining the fork, but I think there is no point for us now to waste our time beautifying and "eff'ing" your code in this PR. Also, would like to know where to report code of conduct violations. |
|
Hi @mighty840,
I've read everything again and honestly don't see anything cryptic at all. Can you give me an example?
We require 2 approvals from the team (and even sometimes 4) to merge PRs. This is a very standard practice so you will always have different devs giving their opinion. Please have a look at ALL our other PRs and you will see that this one hasn't been treated any different.
We are not sure yet, we need to spend some time investigating that library before we take any decision.
I am sorry you feel this way but we are not going to reduce our quality standards. Again, this PR is treated as any other PR, internal or external.
Feel free to send a mail to [email protected]. |
|
Hi @mighty840, sorry for the delay, we have been super busy with stardust on mainnet and work on 2.0. We have taken the time to think about this and for now it seems that the best solution is for you to continue maintaining your fork. With 2.0, we have the opportunity to do breaking changes and completely revisit our storage layer so we will definitely take your input regarding IoT devices in consideration. Thanks for your understanding. |
|
@thibault-martinez works for us. Keep us posted if you need to merge it later. We will try to keep our fork patched as far as we can. |
Thank you for your understanding. I will close the PR until we start working on the database layer for 2.0, is that fine with you? Also, develop won't change too much now so it should be fairly easy to maintain your fork. |
|
Yes. I am closing the PR now from my side. |
Description of change
Added support for JammDB as storage provider, in addition to RocksDB and memory storage for the wallet module.
See Jammdb here
Motivation
Currently, rocksdb is too bulky for usage on end-devices like IoT Edge devices, android, ios Mobile phones and successive builds are not guaranteed due to breaking changes coming from up stream. Jammdb offers a simpler and light-weight option. Rocksdb can still be used and is also the default.
ETO GRUPPE wanted to have own integrated wallet and hence the fork was created for adding support with JammDB. After testing and integration with different bindings for android, ios and capacitorjs, it was found to be stable and can be used as an alternative to rocksdb.
As maintaining the fork was no longer necessary it was decided to contribute these changes back to the IOTA-SDK and also allow everyone else to use the jammdb variant. The rocksdb feature is still the default and jammdb is just another optional feature which can be used.
By removing dependency from the fork, it would be easier to always pull the latest update from IOTA-SDK rather than maintaining the fork.
Type of change
Choose a type of change, and delete any options that are not relevant.
How the change has been tested
Tested with wallet integration in various android and iOS apps @ ETOGRUPPE.
Change checklist
Tick the boxes that are relevant to your changes, and delete any items that are not.