Fix SequenceNumber for nLockTime transactions

[zmanian]

To be effective within the current Bitcoin network, the inputs to a transaction with an nLocktime must be not the standard max value.We set the sequence number of 0 if the value is max.Currently sequence numbers other than MAX_INT32 have no meaning in the Bitcoin protoco but this may change in future BIPS

[braydonf]

LGTM, with a few thoughts:

• The expect isn't needed and isn't doing much since the should will error before then.
• Could be useful to check that the default locktime is being set as expected or not set in the case that the value isn't the max.

[zmanian]

I don't entirely follow your second bullet point?

[braydonf]

Something like this in the second two tests:

var copy = new Transaction(serialized_tx);
copy.inputs[0].sequenceNumber.should.equal(Transaction.Input.DEFAULT_LOCKTIME_SEQNUMBER)

[zmanian]

My objection to having that test is that this test case doesn't actually that a transaction correctly follows the Bitcoin specifications. It merely handshakes that the code works as currently written.

As I've written the test now, the system tests that a transaction does not have a sequence number that would invalidate the effect of locktime.

But anyway I modified the test cases as requested.

[braydonf]

Sorry, I think there may be some confusion here. The test case that I was thinking should be covered is that "sequenceNumber" is the correct property (which it is), and that when the object is serialized, the property isn't lost, such as would be if the property set was "sequence" instead.

[zmanian]

Ah I get it

[braydonf]

ACK

[maraoz]

Great work @zmanian!

[braydonf]

Landed in release 0.13.2