Sequence number needs to be a nonmax value for locktime to be effective. #1316
Conversation
This sets all input sequence numbers to a non max value on setting a locktime
|
why not set them to 0 instead of 0xFFFFFFF0? |
This sets all input sequence numbers to a non max value on setting a locktime
|
Updated... |
lib/transaction/transaction.js
Outdated
There was a problem hiding this comment.
A few things:
- This may not serialize because I think the property for the sequence number on an input is
.sequenceNumber. - There should be tests to cover these statements
- Indentation is incorrect (should be two spaces, as similar to the statements above)
|
Yep I'll write up the test cases and update. Thanks! |
|
Just FYI, Transaction 224660f2196fd22becb54f7c69f7a0ca07526e456276ef9f000c31a783275ab8 on testnet has 0 sequence number and was created with a locktime. Appears to be a correct tx. Working on test cases for the pull request |
- Restores InvalidIndexCantDeriveHardened error in spec - Passes hardened argument in HDPublicKey (in case the feature is attempted to be used similarly to HDPrivateKey) - Fixes undefined error with InvalidLength - Fixes tests to check for the error type - Various formatting cleanup improvements
bitcore should be able to run inside a web worker. There were two minor issues preventing bitcore from running inside a web worker. The first was that lodash was outdated, and that version of lodash had a problem with web workers. The second was that the wrong version of ripemd160 was being called, because global.window does not exist inside a web worker (global.self does instead). A better way to check if you are in a browser is with process.browser.
…ktime trasnactions
when setting a locktime
|
updates with tests |
|
Please make sure to rebase on master ( |
There was a problem hiding this comment.
Using only should I think will work here (and the others), e.g.:
transaction.inputs[0].sequenceNumber.should.not.equal(Transaction.Input.DEFAULT_SEQNUMBER);
|
I'm just going to open a cleaner commit on a new pull request. From correctness for the current Bitcoin protocol point of view, any non max sequence number would be correct. When/if CHECKSEQUENCEVERIFY gets implemented, any non-max field should be accepted in the field. |
|
#1324 is a cleaner commit |
This sets all input sequence numbers to a non max value on setting a locktime
Proposal to fix #1315