Feature/add coin xrp

[okipol88]

Hi all,

I am working on adding XRP to airgap-coiin-lib and airgap wallet in general.
Currently I belive I have finished the coin-lib part. Any suggestions welcome as I do not use TypeScript (at all up till now) and Javascript (ok I had some time with it once). Not to mention this cross app framework and npm ;).

[codecov-io]

Codecov Report

Merging #3 into master will decrease coverage by 0.32%.
The diff coverage is 58.87%.

@@            Coverage Diff             @@
##           master       #3      +/-   ##
==========================================
- Coverage   61.96%   61.64%   -0.33%     
==========================================
  Files          47       49       +2     
  Lines        2190     2401     +211     
  Branches      201      221      +20     
==========================================
+ Hits         1357     1480     +123     
- Misses        765      848      +83     
- Partials       68       73       +5

Impacted Files	Coverage Δ
src/serializer/unsigned-transaction.serializer.ts	100% <ø> (ø)	⬆️
src/index.ts	100% <100%> (ø)	⬆️
src/utils/supportedProtocols.ts	81.25% <100%> (+1.25%)	⬆️
src/serializer/utils/toBuffer.ts	100% <100%> (ø)	⬆️
src/protocols/xrp/XrpProtocol.ts	55.26% <55.26%> (ø)
src/utils/xrp/xrpKeyPair.ts	83.33% <83.33%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 324c7aa...2df4b6f. Read the comment docs.

[okipol88]

There is a problem I am facing with bignumber when used by airgap-coin-lib in the vault 👎.

I have a reproducable example:
This works fine on its own
https://github.com/okipol88/airgap-coin-lib/tree/feature/add-coin-xrp
When it is linked to be used here - iit fails with the aforementioned error
https://github.com/okipol88/airgap-vault/tree/feature/add-coin-xrp

I've seen a similar issue here (I guess):

hildjj/node-cbor#88

Originally posted by @okipol88 in XRPLF/xrpl.js#925 (comment)

[dcale]

why uppercase?

[okipol88]

why uppercase?

It somehow did not work for me in the unit tests. Having a second look I think in this case this should just be the same as the protocol Identifier. Small tweak I guess.

[AndreasGassmann]

Hi, thank you for opening this pull request and investing your time into improving our library. It's much appreciated!

I didn't have much time to look at it, but the base already looks solid. A couple of issues:

• We are currently in the process of minimising the number of dependencies we use. You added quite a few new ones, one of which is your personal fork of is sjcl. Is it possible to reduce the number of dependencies and re-use existing packages or replace old packages by re-using one of the new ones? In either case, before merging we will have to review those dependencies.
• We currently cannot upgrade the typescript version because of incompatibilities with our apps. Luckily, we are already in the process of upgrading both our apps so we can do that. We already have an internal branch open where we change the typescript version, together with some other changes. So we need to merge our changes to master before we can merge this.
• We will also need to review and test your changes thoroughly. We already have a tight schedule for our next few sprints, so we will most likely not be able to do that for some time, just to let you know. We will also have to get familiar with the XRP protocol so we can understand the specifics of what you did.
• Are you in any way affiliated with XRP or are you a community member who would simply like to see XRP support in AirGap?

Thanks again for taking your time to work on this.

[okipol88]

Hi, thank you for opening this pull request and investing your time into improving our library. It's much appreciated!

I didn't have much time to look at it, but the base already looks solid. A couple of issues:

• We are currently in the process of minimising the number of dependencies we use. You added quite a few new ones, one of which is your personal fork of is sjcl. Is it possible to reduce the number of dependencies and re-use existing packages or replace old packages by re-using one of the new ones? In either case, before merging we will have to review those dependencies.
• We currently cannot upgrade the typescript version because of incompatibilities with our apps. Luckily, we are already in the process of upgrading both our apps so we can do that. We already have an internal branch open where we change the typescript version, together with some other changes. So we need to merge our changes to master before we can merge this.
• We will also need to review and test your changes thoroughly. We already have a tight schedule for our next few sprints, so we will most likely not be able to do that for some time, just to let you know. We will also have to get familiar with the XRP protocol so we can understand the specifics of what you did.
• Are you in any way affiliated with XRP or are you a community member who would simply like to see XRP support in AirGap?

Thanks again for taking your time to work on this.

ok so one by one:

• sjcl can go down totally -> I wanted to use it because I saw it used hmm perhaps Wietse Wind?. Anyway I got away not using it so that is a bad NoNo from my side ;(. I have to remove it ;) no Problem.

• Typescript version was a must have at least from my point of view -> I could not get it working otherwise so we just have to wait

• This is fully understandable. I also went in deep into the air-gap related ins. I totally support this idea even on another branch not master. I did some tries on submitting the transactions and got the correct answer (not transaction invalid but e.g. account not found).

• I am just a community member who would like to see XRP in. I like your Idea of the air-gap and would like not to struggle with creating a new approach.

So a big subnotes:

• The tests go OK but I have some trouble with the real airgap-vault ... it complains that it requires a buffer when retrieveing the address from public key.

• ripple-lib has some dependency of lib to an really old bignumber.js. I will try to mend that.

• Best way from my side is to make it all working - vault, airgap-coin-lib etc.

[AndreasGassmann]

So I finally managed to clean up the branch we've been working on internally and make it public on github. You can find it here: https://github.com/airgap-it/airgap-coin-lib/tree/release/v0.5.0

It's still a work in progress, but the overall structure shouldn't change much.

There are a lot of changes, but most of them are related to linting and the structure of the project:

• The "lib" folder is now called "src"
• To be consistent, we will name all protocols with their full name, so we will change "AEProtocol" to "AeternityProtocol" to be consistent
• We may or may not include some kind of dependency management change where we commit some of the dependencies we use to have full control over the source code and improve security (I will let you know if we do this).

There are 2 more things we have planned that are not yet started, but will happen soon:

• We will add more functionality to the protocols to allow for more features, specifically "chain" related things like "getLatestBlocks" or "getTrainsactionsForBlock", etc. The methods we have now are mainly connected to a specific account (creating an address, getting infos about an address, signing a tx, etc). We want to make it broader to include data that is not linked to a specific account but rather the whole blockchain. This will most likely not affect you (maybe apart from some renaming), because we will gradually add those new methods and they will not be required in the beginning. I'll let you know when we know more.

• We are planning to add support for "generic actions", so we can define actions that a user can take on a certain protocol (eg. importing tokens for ETH, delegating for XTZ, etc.). The idea here is that it will allow us to have the logic in the airgap-coin-lib and only add the UI in the apps, currently we have everything in the apps. Does ripple have any "special actions" that a user might want to be able to do?