Skip to content

Conversation

@rubensayshi
Copy link
Contributor

add failing unittest to prove the error I'm trying to fix in #369

@dcousens
Copy link
Contributor

dcousens commented Mar 2, 2015

Disregard previous comments, accidentally read this as 3 of 3, not 2 of 3.

@dcousens
Copy link
Contributor

dcousens commented Mar 2, 2015

@rubensayshi I have added c79fecf to show case this test without the need for it to be an integration test.

What is shown there is the expected txHex, it verifies that no OP_0 should be present in the scriptSig (for a buildComplete); and as such, before bcf8d01 was a failing test.

Please ACK this and I'll close this PR. :)

edit: moved changes into a PR, see #372

@rubensayshi
Copy link
Contributor Author

hmm, I changed the 2of2 integration test to 2of3, I think that's a better test case since it has a greater chance of failing than a 2of2 (for various reasons)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.24% when pulling c52420a on blocktrail:2of3-failing-test into b132714 on bitcoinjs:master.

@dcousens dcousens modified the milestone: 2.0.0 Mar 5, 2015
@dcousens dcousens changed the title add 2of3 multisig test that fails because of OP_0 Change 2-of-2 integration test to 2-of-3 Mar 5, 2015
@dcousens
Copy link
Contributor

dcousens commented Mar 5, 2015

@rubensayshi I'm OK with this change, but I don't think it covers any less or more than 2-of-2, it simply covers the other case (fully signed vs partially signed).

However this does show case a better example of multisig to the typical user, so I'll update the README example after to merge.

dcousens added a commit that referenced this pull request Mar 5, 2015
Change 2-of-2 integration test to 2-of-3
@dcousens dcousens merged commit 87a3803 into bitcoinjs:master Mar 5, 2015
@dcousens
Copy link
Contributor

dcousens commented Mar 5, 2015

@rubensayshi to follow that line of thought, I changed it to a less traditional 2-of-4 which would better illustrate some parts of the API and provide a larger contrast to the example above it, the 2-of-2.

Naturally, it also tests something less common which is a bonus I guess.

@rubensayshi
Copy link
Contributor Author

awesome!

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.

3 participants