Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial XBR protocol implementation #424

Merged
merged 18 commits into from
May 21, 2019
Merged

Conversation

om26er
Copy link
Contributor

@om26er om26er commented May 11, 2019

Implements the XBR Buyer and Seller

There are things that still don't work, we still have a sample.js file that needs to be removed before landing it.

@oberstet
Copy link
Contributor

I closed above PR, but added CI test coverage for using native binary payloads across JS and Py via

AutobahnJS supports use of native binary values in application payload args/kwargs
- IOW: this "should work" (calling the market maker with native bytes). we could also add CI tests actually running a crossbarfx node with market maker .. more work though;)

@oberstet
Copy link
Contributor

to enforce using CBOR, a WAMP connection can be created like this:

var config = {
    url: "ws://127.0.0.1:8080/ws",
    realm: "realm1",
    serializers: [new autobahn.serializer.CBORSerializer()]
};

var connection = new autobahn.Connection(config);

@om26er om26er changed the title [WIP] XBR Seller Initial XBR protocol implementation May 19, 2019
@oberstet
Copy link
Contributor

yeah, this looks good! basic class structure is there and uses the new dependencies for actual crypto. guess it is quite close to full interop with ABPy based XBR stuff .. awesome=)

@om26er
Copy link
Contributor Author

om26er commented May 19, 2019

The Buyer and Seller both are implemented now (close to ABPy), but require feedback on method signatures (added inline comment).

Also @oberstet is it fine to do a release right after this lands ?

lib/xbr/buyer.js Outdated
this._keyPair = nacl.box.keyPair();
};

SimpleBuyer.prototype.start = function(session, consumerID, on_success, on_failure) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oberstet Need your input on this API signature. Currently it takes two callbacks on_success and on_failure as parameters because start() makes a call to session.call() and returns instantly, hence we can't really return the result as we do in the Python implementation.

Are you fine with that approach or do we want to use something different here (and in other places below).

Copy link
Contributor

Choose a reason for hiding this comment

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

start should return a proper promise (not take callback args), and then you can fire the promise with success or error later on ..

Copy link
Contributor

Choose a reason for hiding this comment

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

rgd new release: sure!

@om26er
Copy link
Contributor Author

om26er commented May 20, 2019

OK, now moved from callback(s) approach to deferred as used by other parts of the library.

Note: Moved around small part of the library code into utils.js.

@oberstet
Copy link
Contributor

@om26er should we merge this or do you want to piggyback more onto? if you wanna merge, go ahead (merge yourself)! looks good

@om26er
Copy link
Contributor Author

om26er commented May 21, 2019

Ok, landing this now. More fixes/changes to follow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants