Bitcoin core tests and fixed exception testing #199
Conversation
|
Given that The use case is selectively removing either an opcode or (eventually, not supported for now) a data block. |
All the `invalid2` tests have been removed as they were not invalid base58check. They were actually valid in some cases. They will be re-integrated in more specific bitcoin core tests in relation to Address/ECKey respectively.
These weren't broken as such, but they weren't distinctly checking that the right exception was thrown either.
Also fixes the bug when the sequence number is 0 and `TransactionIn.defaultSequence` is used; resulting in an undefined sequence number as it is undefined.
To save on us building a hash map with which to check the inputs, instead I just ensure that the order of the inputs is the same as it is in the serialized transaction.
Adds asserts to ensure only valid hashes are created (until the implementation is complete). Also uses `Script.without` to remove OP_CODESEPARATOR from the Scripts as required by the protocol.
Not all tests are added yet, but this represents a significant portion.
|
+1. Awesome getting some of the Bitcoin Core tests in! |
There was a problem hiding this comment.
Why not just convert every array-like chunk in chunks into a buffer after line 372? then the type check won't be necessary here and below. And when chunks are fully bufferified, just remove the conversion code we are good to go. minor point given that this is still WIP
There was a problem hiding this comment.
Exactly what will happen, just avoided for now because this would require changing a lot more code.
In fact, the conversion won't even be necessary, we'll never accept Arrays in the first place once Script is "bufferified".
|
LGTM |
Bitcoin core tests and fixed exception testing
4 participants
This pull request primarily adds a large portion of the core Bitcoin tests (aka, test data from the reference client), and makes some minor changes to
Transaction.hashForSignatureto ensure it can pass these tests.To do this, some new functionality was added:
Script.fromChunks, required only bywithoutfor now, this function will likely be the backbone behind the entire ofScript, avoiding the necessity toparsescripts, and removing all mutable functionality withinScript.Script.prototype.without, similar in functionality toCScript.findAndDeletein the reference client, however fundamentally just a filter over thescript.chunks. This was made easy byScript.fromChunks.bufferutils.readPushDataIntbufferutils.writePushDataIntbufferutils.pushDataSizeAll with appropriate tests. (
Script.fromChunkswill be receiving more, but it covers the basics for now).Finally, a change required to pass the tests was the fix up of the
TransactionconstantSIGHASH_ANYONECANPAY, which appears to have been incorrect since the0.1.3tag (and no doubt since this libraries creation).edit: It also fixes many of the tests which had incorrect exception handling, or tests that were throwing on the wrong exceptions etc.