Modules / Testing

[JP Richardson]

Hey guys, I've created most (if not all) of the smaller modules necessary for a robust and complete Bitcoin library to be part of the CryptoCoinJS distributed library. Before I make anymore serious modifications, I'll be focusing on adding tests. Tests will help to facilitate our joint efforts and will also help to communicate the expectations of each module to future users.

TL;DR: Tests make working on code with a team much easier.

Let me know if you have any thoughts or ideas.

Thanks.

[JP Richardson]

Oh yeah, obviously for your own modules that you started in the Github cryptocoinjs org, do as you see best fit :)

[Sidney Zhang]

Hey JP,

Thanks for the awesome work. This is incredible!

[Brooks Boyd]

Agreed; I've started adding tests for my most recent additions, using the Mocha testing framework that you were using, but I'm thinking I'll not require terst, since the Node Assert classes can do just as well, without requiring another dependency.

--
You received this message because you are subscribed to the Google Groups "Bitcoin Hackers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to BitcoinHacker...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Brooks Boyd]

I've started looking over your "btc-script" and "btc-transaction" recent additions, as I'm starting to conceptualize how a "btc-blockchain" module would act, and it looks like those will be great to use, but they're dealing with byte arrays by default. Do you have plans to convert them to Buffers? They're also using the "convert-hex" and "convert-string" libraries, which should be switched over to "binstring", right?

I'm also wondering if there should be a generic "blockchain" library that could apply to all cryptocurrencies and then a "btc-blockchain" library that specifies it for Bitcoin (and other libraries for individual cryptocoins)? I envision a generic "blockchain" manager library would have the ability to set a genesis block, push new blocks onto the chain referencing a parent block on the chain, and provide lookups along the chain and use a database library to store it all. I think the generic library would not care about forking; it would store all blocks on all forks, and it would provide methods for some parent library to trigger a move to a different fork (it would keep a pointer to the current main chain, and the ability to update that to a new head, and the ability to snip off a fork and discard it). It would need to keep a pool of "orphans" (whose parent/previous block is not known yet) as well, and attempt to join them in whenever a new block is added. What do you think of that? Would that be universal enough to cross multiple cryptocurrencies?

Brooks

On Tuesday, January 14, 2014 6:01:17 PM UTC-6, JP Richardson wrote:

[JP Richardson]

Regarding the two new modules, they are direct ports from BitcoinJS. (Kyle Drake's version or Vitalik's version... or maybe a mixture). But that was the easiest thing to do, get them ported over and split before making any modifications. I'm OK with the buffer modication and binstring modification now, but I'd like to put int tests as well. Tests are a high priority for these modules so that they can be seen as reliable.

Regarding the "blockchain" module, I think it probably makes sense to start with just a "btc-blockchain" module and then overtime, extract out the relevant parts into a generic module. Anything else may feel like over-engineering. To be fair though, I'm not even close to as familiar to the structure of the blockchain as you or Sidney. So it's entirely up to you.

--

You received this message because you are subscribed to the Google Groups "Bitcoin Hackers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to BitcoinHacker...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
Follow me on Twitter: http://twitter.com/jprichardson

[Sidney Zhang]

Hi Brooks

Since JP has been super busy. Do you mind reviewing these pull requests for me?

https://github.com/cryptocoinjs/eckey/pull/5

https://github.com/cryptocoinjs/btc-script/pull/2

https://github.com/cryptocoinjs/btc-transaction/pull/1

They are mostly small fixes / bugs. I discovered them when I used the libs to build transactions for an app that I am currently writing.

Note: I realized there is a bit of a messy diff file, that's because my editor automatically removes trailing whitespaces. I will make sure to separate the diffs next time

[JP Richardson]

Hey Sidney... sorry. You should have permissions to make the modifications or accept the PRs yourself, right?

On Tuesday, January 14, 2014 6:01:17 PM UTC-6, JP Richardson wrote:

[Sidney Zhang]

Hey JP,

No need to say sorry!! I do have permission to merge the PRs

But since I am new to this project. I wanted to make PRs first and get acquainted to the process before directly merging code myself.

[Brooks Boyd]

It probably a good idea to have a "primary" developer for each library if there's multiple people working on it at once, do the feature code on a side branch, and do a pull request to at least get an "ACK" out of the other developers and have the primary developer merge the branch in. We've been trying to implement a formal "Code Review" process at my day job, which I can see the benefit of, though it does slow down quick little changes.

Brooks

--

[Brooks Boyd]

Along those lines, I recalled reading a while ago how Github employees themselves used branches and pull requests internally and went and dug it out again. The article is mentioned in the GitHub blog here, but the main article is over here outlining the "GitHub Flow", which is for teams working on one repo, make a branch, rather than a separate fork, and make a pull request for the branch within the repo. Other team members comment through the process, and give a "ACK" of some form when it's ready to be merged into master. Sound like a good idea to utilize?

Brooks

[Sidney Zhang]

I like this idea. In fact, this is the flow that I use at the company that I work for. I never directly merge a pull whereas I always have someone else merge it as an ack.

Let's try that?

[Sidney Zhang]

I wanted to follow up on something that I notice the cryptocoinjs libs

https://github.com/cryptocoinjs/btc-script/blob/master/lib/script.js#L39

The lib seems to make assumptions about what format of the data that comes in. For example, this assumes the blockexplorer format which breaks the scriptsig into a string delimited by a space.

I think we should removing some of these assumptions over time and make the library more general

[Brooks Boyd]

Well, if you use the "fromScriptSig" method, you should be using "script sig format" as your input, so I don't think it needs to get too general, but I see it could get more flexible and accept an array of commands (bypassing the str.split()), and it should error out if the input is not the correct format, rather than blindly assuming it is.

Brooks

--

[Sidney Zhang]

I see. I just saw the part about the scriptSig format. That's fair enough.

I guess it would be good to accept more formats rarther than the one where it breaks into chunkcs with space as a delimiter

[Brooks Boyd]

I've created a pull request on the p2p-manager module, to keep with this sort of mentality: https://github.com/cryptocoinjs/p2p-manager/pull/3. A few more features of GitHub we can take advantage of I tried adding to that pull request:

Using task lists in the description of the pull request allows creating pull requests early, and clearly shows everyone that they're not ready to merge yet if they have unchecked boxes. The nice thing about the task lists is they also show up in any listing of pull requests (either the list for the project, or if you're looking at your own dashboard) as a line item of "4 tasks (0 completed, 4 remaining)".

I also added in a "Q&A" table that was inspired by the one that the Symfony team uses (I've done one contribution to that library, so had to go through their patch-submission process once), which I think helps get an overview of what the pull does.

What do you think of using those conventions in pull requests?

Brooks

[JP Richardson]

I like the table. But I'm wondering if it's a bit too much ceremony too early. I've noticed a standard that AngularJS uses with each commit message. Not sure that I think that's the way to go either. I think in the short-term, the best way to go is to relentlessly maintain CHANGELOG.md (shouldn't just be a copy of commit messages) and adhere to semver.

-JP

--

You received this message because you are subscribed to the Google Groups "Bitcoin Hackers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to BitcoinHacker...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Brooks Boyd]

Regarding testing, JP, you created your "terst" module and use it in all your test scripts, and seem rather opposed to the default NodeJS "assert" (as you mention in your README). Could you explain further what you don't like about "assert" other than the commands themselves are longer to type out? One key thing that "terst" is missing for me is the "this block should throw an error" test. If we decide to use "terst" unilaterally for cryptocoinjs test scripts, could we add that assertion, please? Or, I'd be in favor of just using the included "assert" scripts, such that there's one less dependency to install. Any thoughts?

Brooks

[JP Richardson]

It's not the typing that I mind, as most IDEs support snippets. Verbose code should serve a purpose though and I don't see how:

T (someStatement)

is not better than

assert.ok(someStatement)

Your eyes can visually see the functions of `terst` as you can scan do with your eyes. Also, `assert` doesn't work in the browser. Maybe with our choice of Browserify, this point is moot.

I typically catch errors this way:

var err = null

try {

  someStatement

} catch (e) {

  err = e

}

T (err) //assuming error is expected.

I'm fine with picking another testing library that suits our tastes as I realize that many might find terst kinda weird. So if we choose assert, then so be it. Most importantly, we just need to be consistent.

Thoughts from others?

On Wed, Feb 5, 2014 at 10:17 AM, Brooks Boyd <brook...@gmail.com> wrote:

Regarding testing, JP, you created your "terst" module and use it in all your test scripts, and seem rather opposed to the default NodeJS "assert" (as you mention in your README). Could you explain further what you don't like about "assert" other than the commands themselves are longer to type out? One key thing that "terst" is missing for me is the "this block should throw an error" test. If we decide to use "terst" unilaterally for cryptocoinjs test scripts, could we add that assertion, please? Or, I'd be in favor of just using the included "assert" scripts, such that there's one less dependency to install. Any thoughts?

Brooks

--

You received this message because you are subscribed to the Google Groups "Bitcoin Hackers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to BitcoinHacker...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Sidney Zhang]

I would like  a Throws method or something into terst for this kind of testing. 

I thinkn that would be useful.

[Brooks Boyd]

For me, though "T (somestatement)" is shorter, it's so short as to lose its clarity. If you see "T" and "F" together, they make sense and can be more easily inferred what they mean without documentation, but on their own, does "T" stand for "test", "triangulate", "true", or "television"? When there's a long line of "assert" calls, my eye just jumps over to the first period, and reads down the row from there, making it not really that hard to skim. Plus to me, using the standard package that comes with the app means a better chance that other developers will already be familiar with it, rather than including another custom module, and the more verbose function names seem to fit with the "describe/it" labeling that mocha does alongside it.

Does mocha actually work in the browser either? That might make it a moot point too. I wouldn't mind using Terst, though I think it takes non-verbosity one step further than it is useful to go.

Brooks

[JP Richardson]

> but on their own, does "T" stand for "test", "triangulate", "true", or "television"?

I don't think this is a valid concern. 1) "triangulate" or "television" don't make any sense in this context. 2) Admittedly I'm biased, but if any developer opens up a test file and sees T (someStatement), I'd bet that they'd figure it out pretty fast. Now, with other terst statements "NEQ", this may not be as true.

Yep, Mocha works in the browser. Either way, I understand that it may not be to a person's taste and thus am fine with not using it. Or maybe we just defer to using both assert and terst and over time, perhaps a winner will emerge?

[JP Richardson]

I would also be interested in looking into Tap as well...

[Sidney Zhang]

What is Tap?

Kind regards,

Sidney Zhang

Mobile: +1 415 767 8831 

[JP Richardson]

http://en.wikipedia.org/wiki/Test_Anything_Protocol

Relevant Node.js modules:

https://github.com/substack/tape

https://github.com/isaacs/node-tap

[hjdas...@gmail.com]

Sorry to bother you all , but is this group only related to hacking bitcoins or other cryptocurrencies as well ?

[Sidney Zhang]

It is mostly bitcoin at this stage. What altcoin are you hacking on?

--

You received this message because you are subscribed to the Google Groups "Bitcoin Hackers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to BitcoinHacker...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[danny]

Well im not a hacker by any means i am just interested in learning more about this

[Sidney Zhang]

Sure, you should follow the conversation. We are all working on different projects here!