Skip to content
This repository has been archived by the owner on Apr 5, 2021. It is now read-only.

Neon-js light wallet wrapper #108

Merged
merged 7 commits into from
Dec 20, 2017
Merged

Conversation

slipo
Copy link
Collaborator

@slipo slipo commented Dec 7, 2017

Description

This adds a wrapper class over neon-js to provide light wallet support. A few notes:

Because the required functionality for a light wallet isn't really supported by core neo nodes and requires accessing CoZ sites running NeonDB, I do believe @lllwvlvwlll's guidance was right and we should offload this integration to neon-js.

Local storage wallet support can be built on top of this as an alternation data access method. We have transaction history and balance support already from local data. One open question is in what format to we return the data. I would argue we should return data the same as neon-js. I'm doing this for now.

Some of the methods used by neon-js return or use as parameters internal classes such as "Transactions" ... we don't quite have the equivalent right now and it would be a bit of work to implement them. What to do here should also be discussed.

Motivation and Context

Wallet support: #78
Integrate neon-js: #101

How Has This Been Tested?

For now, there's an example script that shows how it works.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@rockacola
Copy link
Collaborator

I see your neon-js reference and usage, however you may be missing package.json updates for your PR.

@lllwvlvwlll
Copy link
Member

@slipo Could you add test coverage?

@rockacola rockacola mentioned this pull request Dec 11, 2017
@snowypowers
Copy link
Member

just saw this so gonna drop some comments:

  1. I am leaning towards dropping support for all transactional methods in neonDB file. So stuff like doTransferToken, doClaimAllGas and doSendAsset will be deprecated in the near future. The reason is that they dont belong there anymore. neonDB should be an API layer that wraps the external data for neonjs to utilise. Replacements can be found in api/core instead.

  2. I thought this was for NEP6 Wallet lol. Confusing to have similar names but differing functions.

  3. Looking forward to a neoJS file in the future. It should be structured similarly to how neonDB or neoscan is structured now so that neonjs can choose which provider to use and drop it in.

@lllwvlvwlll
Copy link
Member

@snowypowers I think we can merge this for now and track against changes to neon-js and neondb. Eventually, I think project will actually invert with there being neo-js nodes running which neon-js interfaces with.

Copy link
Member

@lllwvlvwlll lllwvlvwlll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. Sorry it took so long to get in!

@lllwvlvwlll lllwvlvwlll merged commit ff71ec2 into CityOfZion:develop Dec 20, 2017
@slipo slipo deleted the neon-js-wallet branch December 22, 2017 12:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants