search

ripple / ripple-lib-extensions

[Deprecated] A collection of extensions to ripple-lib. Instead, see https://github.com/XRPLF/xrpl.js
https://github.com/XRPLF/xrpl.js
22 stars 31 forks source link

Add messagesigner #11

Closed sublimator closed 9 years ago

sublimator commented 9 years ago

Unfortunately, Message has some dependencies on ripple-lib types, and apis. Thus, to avoid making much changes (not much shelf life left on this code), the module just exports a factory for the Message class parameterised with (ripplelib, sjcl).

It wouldn't be ideal for this to depend upon a particular version of ripple-lib, encoded in the package.json and yet the application to depend upon another, and yet that dependency would be ideally versioned. Lament factory :/

The tests were also taken from ripple-lib, and updated. The devDependencies declares a specific version of ripple-lib. (ripple-vault-client declares ripple-lib dependency of ~0.10.0, yet when deployed, via webpack, uses whatever ripple-client uses, which is ~0.12.5-rc2 )

Anyway, this is just another step towards removing the dependency on sjcl-extended in ripple-lib. This repo was mentioned as a possible home. How long would this stay in this form? At some point we'll be basing the Message implementation on elliptic and co.

@clark800 A penny for your help/thoughts ?

clark800 commented 9 years ago

I think it looks good. Could you add a section to the README to explain what it is and how to set it up (for example sjcl has to be an instance of scjl-extended)?

We still have to figure out how to separate sjcl-extended from ripple-lib. If sjcl-extended also was passed an instance of sjcl, then I think we could fully separate it.

sublimator commented 9 years ago

We still have to figure out how to separate sjcl-extended from ripple-lib. If sjcl-extended also was passed an instance of sjcl, then I think we could fully separate it.

I didn't quite understand that

sublimator commented 9 years ago

Could you add a section to the README

Will do!

clark800 commented 9 years ago

I was referring to the fact that ripple-lib currently requires sjcl-extended even though it should only have to depend on sjcl. We should be able to make it depend on just sjcl if we make sjcl-extended use dependency injection for sjcl and then only the blob-vault code will depend on sjcl-extended.

sublimator commented 9 years ago

requires sjcl-extended even though it should only have to depend on sjcl

I'm still not quite understanding you.

The only way I can parse this, is that you're under the impression that Message is the final thing in ripple-lib that depends upon the sjcl-extended functionality. There's a few other little things like canonical signatures, point encoding (for publc keys), DER encoding etc, etc.

ripple-lib will pretty much have to depend on sjcl-extended until we start using elliptic (which we are getting closer to now) I think vault-client should be updated to explicitly depend upon sjcl-extended and not expect ripple-lib to export sjcl. From there we can transition it to using elliptic + a modular build of sjcl

sublimator commented 9 years ago

See: https://github.com/ripple/ripple-vault-client/issues/26 See: https://github.com/sublimator/ripple-vault-client/tree/messagesigner

clark800 commented 9 years ago

The only way I can parse this, is that you're under the impression that Message is the final thing in ripple-lib that depends upon the sjcl-extended functionality.

Yeah, when I looked at it, it seemed like the few parts of sjcl-extended that ripple-lib needed (other than what Message needed) were just encodings (like DER), not really crypto, and might more appropriately live in ripple-lib than sjcl-extended, or another library could be used. But if we can get rid of sjcl soon then you can ignore this.

clark800 commented 9 years ago

LGTM

sublimator commented 9 years ago

might more appropriately live in ripple-lib

Ah OK. Yeah, elliptic & co will pretty much cover our bases for all things ripple-lib needs except utf8 encoding it seems :)

I thought about it and I don't really want to get into the quagmire of vault-* stuff (at least yet), so would like to just remove Message.js from ripple-lib now, and we can deal with upgrading vault-client I guess next time ripple-client version of ripple-lib is upgraded. I'll open some issues there with

If we get ripple-lib switched over to elliptic, we can also better spend time on a newer version of Message.js that supports ed25519.

clark800 commented 9 years ago

Sounds great