steembots / steemsign

sign steem transaction (work in progress)
1 stars 0 forks source link

Provide a bytebuffers implemention #3

Closed willbanks closed 8 years ago

willbanks commented 8 years ago

This issue must be completed to close #1 This is similar to issue #2 and requires issue #2 to be complete before this one can be finished, but can be worked in parallel. You must flatten and normalize the npm bytebuffers implementation from here https://github.com/dcodeIO/bytebuffer.js/tree/master/src this should be done in a new subdirectory named "bytebufferjs"

Once this is flattened and normalized update the index.html file and submit

jtault commented 8 years ago

Should I make use of the bytebuffer.js file already in our repository, or should I just start fresh with https://github.com/dcodeIO/bytebuffer.js/tree/master/src?

jtault commented 8 years ago

Update: I just finished up the Long class, moving on to bytebuffers.

jtault commented 8 years ago

I think this issue is ready to be closed. I wrote my own versions of most of the tests in https://github.com/dcodeIO/bytebuffer.js/blob/master/tests/suite.js and ByteBuffers class seems to be working.

willbanks commented 8 years ago

@jtault Wow! You have been absolutely rocking this implementation stuff! I've been taking my time to do code reviews to make sure nothing is going to fail in an undefined way. Lots of moving pieces involved and I wanted to ensure we aren't doing any stomping or breaking. Lol I'm still code reviewing the node buffers implementation. Didn't think you would make that much progress, but you're really hitting it out of the park. Keep on it, I'm going to open up a couple more issues for this since someone brought a potential XSS vulnerability to light that we will need to address.

jtault commented 8 years ago

Thanks! This is the first I've ever used JavaScript, so I'm REALLY scraping this together. I'll leave the code reviews to you :) I've been trying to do as many of the tests as possible along the way as I piece this together, but I have to rewrite them to work with the pure JS, so I've been grabbing the low hanging fruit and just doing the tests that were really easy to adapt to the pure JS implementation. The only tests that have actually failed so far have been with the StringDecoder class which I'm working with now, when using CESU-8 (not sure exactly what that means). It seems to work for all the others.

willbanks commented 8 years ago

Ohh that's super important for you to grok... https://en.wikipedia.org/wiki/CESU-8

jtault commented 8 years ago

Thanks I gave that a read through. Not sure what the issue I'm having is. Maybe you can give it a quick look if you get a chance. A minimum case error is below. The test should work with the latest push if you just put the test in the body in index.html.

var decoder = new StringDecoder('utf8'); var buffer = new Buffer('EDA0BDEDB18D', 'hex'); // THUMBS UP SIGN (in CESU-8)
var s = ''; s += decoder.write(buffer.slice(0, 1)); s += decoder.write(buffer.slice(1, 2)); s += decoder.write(buffer.slice(2, 3)); // complete lead surrogate
alert(s === '');

willbanks commented 8 years ago

Your test appears to be saying variable s is exactly the same object as an empty string. That won't work because you've put data in it. Try doing a console.log on s and seeing if it pops up with the correct character? Pretty sure you're looking for == equivalence test and maybe the thumbs up character is in your code. But if so I'm reasonably certain you can't have non ascii in your source code.

jtault commented 8 years ago

It seems like my console.log is not able to display some encodings? It says it should be a thumbs up sign, by I only ever see an empty square, of a square with a couple small letters in it, which I think means can't render it?

The test.js was mainly scratch space I was using for individual tests. I'll try to add test suites for the different components which might help for understanding usage too.

jtault commented 8 years ago

I just pushed a full set of tests for the nodebuffer implementation. It's practically every test from feross' buffer repository, just rewritten for no dependencies.

jtault commented 8 years ago

Just pushed another set of tests for the ByteBuffers class. I never implemented a deepEquals comparison function, so I just console.log both arguments and make sure everything in the console shows up in pairs... kind of hackish, but oh well.

willbanks commented 8 years ago

Ok I just caught this and did a pull before pushing my closure changes. There was a merge conflict so I tried a clean pull to see what's going on here, because the merge message was confusing.

Probably not related to the conflict but in the clean pull... There are two files referenced in the html that are missing from your push. ecc/emitter.js and ecc/stream.js Yet tests are passing without them??

Are these vesitigial or did they serve a purpose?

jtault commented 8 years ago

Those are bother part of the ecc library that I had started on flattening. I think there is still a lot I have to do for that. And then also the serializer. I was confused by your other message. Don't we still need the ecc and serializer in order to sign transactions?

willbanks commented 8 years ago

Uhh I flattened those too. That's what I was saying about merge conflicts and I didn't see you were working on it until it was too late. My merge conflict was in ecc lib, I've got serializer flattened already. Think I may have stomped on you there, sorry about that. Tell you what. I'll back out my ECC stuff, you finish ECC and I'll push up serializer.

Also I talked to @pharesim and told him you can claim the cash bounty on bountysource in addition to your half of the SBD from the fundraise. So you go ahead and do that once ECC is flattened and pushed up.

jtault commented 8 years ago

Ok sounds good. I'll try to finish up ECC today. This has actually been a really fun project for me. I've never contributed to a project like this. It's cool to be a part of making a community tool like this. I'm not sure what the plans are moving forward, but I would be interested in making this into a user friendly web page for download and convenient offline signing. I'd happily split the rewards from the posts with you and pharesim.

willbanks commented 8 years ago

@jtault Yeah I think he's probably trying to do something like www.bitaddress.org or www.bip32.org Steembots needs the offline signing ability too. The existing libraries are pretty bulky. We're obviously focused more on the AI side of things though.