Ether1Project / ethofs-sdk

MIT License
1 stars 3 forks source link

Drop ethos-sdk-web in favour of this SDK #7

Closed ghost closed 3 years ago

ghost commented 3 years ago

The only reason I can see that we have a separate SDK for web is that it does not support FS, and the user has to provide the stream rather than being handled by the SDK.

I think we can drop the web SDK repo, as it would make it a lot easier to maintain SDK, if providing/conversion to readable stream is left for the developer using the SDK rather than having it handled by the SDK.

Also, the SDK is not allowing to add user, it is just returning "ethoFS user not found" on testAuthentication, and when I addUser, it says "ethos user already exists". I am talking about the web SDK, but I guess that apart from few functions, the code is the same for both.

Also, the SDK adds the "0x" to the ethoFS key on init(ethoKey), would be great to make it test if the complete private key is being returned by the developer while using init.

I have added the dashboard to ether1 mobile wallet repo in dashboard branch. You can checkout to that branch and test it to reproduce the bug.

Also, since SDK takes into account Web3, it would be a lot handy if the common functions like sendTX, signTX that we implemented in the mobile and desktop wallets are merged into it. Also some commands like getUser (to check before adding user) should be there. But why we are adding a custom param doesn't make sense, as it would be the most developer friendly and straight forward to add user by wallet address linked to privateKey.

Let me know what you think @fallengravity and @Dev-JamesR .

I think I can work on the SDK, to make it more developer friendly and compatible with different versions (node, electron, web and Cordova) but I would need some help as I still don't get the full hang of crypto in general and all the fine details.

Let me know what you think.

Dev-JamesR commented 3 years ago

I agree with dropping the web sdk, we have done quite a bit of work on the standard SDK and with the next few updates should be able to utilize in browser.

If you can help with the SDK that would be a great load off of our priority list. I am slowly adding a bit more functionality but if can help reorganize and make it more usable that would be a great help to us!

fallengravity commented 3 years ago

Hey @newCodeRunner - apologies for the late reply on this. I think it's a great idea, if you feel like you're up to the task please can you begin working on the recommendations. Thank you!

ghost commented 3 years ago

@Dev-JamesR and @fallengravity , I have made several Changes and optimised code in "newCodeRunner" branch. However, some changes that were working around 10 days ago, have stopped working, On pinning, it says tx reverted error, any hints where I might be wrong. also if you guys can have a look at my branch, as to if you agree with the approach then I will update the readme and other stuff, before we can merge the changes to the master branch

ghost commented 3 years ago

@Dev-JamesR , I completed the implementation of the pending remarks and fixed the pinList broken issue, you can let me know other changes required in separate issues. Closing this one