Transaction Clean #212
Transaction Clean #212
Conversation
There was a problem hiding this comment.
Do we expose these constants anywhere? It seems like devs might need them for some of the functions (namely Transaction.prototype.sign).
There was a problem hiding this comment.
A fair point, we don't yet. Thoughts on how they should be exposed? Transaction.SIGHASH_ALL?
There was a problem hiding this comment.
That would work. Is this more of a Transaction thing, or a Script thing?
There was a problem hiding this comment.
Hash type modifiers are purely a Transaction thing. They are added to the signatures, but are most important in the hashing of the Transaction for signing.
There was a problem hiding this comment.
Transaction.SIGHASH_ALL it is then.
There was a problem hiding this comment.
@kyledrake I'll expose these in the follow up TransactionBuilder pull request, simply because it is necessary for them to be exposed in that PR.
|
I think the SIGHASH constants should be exposed somewhere for devs ( |
In turn also removes the inherent calculation of tx.hash after deserialization.
This is a wallet abstraction.
These tests were still passing despite being incorrect.
|
Rebased on master. Added fixes for the |
src/transaction.js
Outdated
There was a problem hiding this comment.
The error message here suggests that we are just expecting Transaction. It could be a Transaction or transaction id as a string.
4 participants
Chained on #211.
This pull request reworks the internals of
Transaction, removing redundant code, abstractions and overall does a cleanup.Much of the commits are self explanatory, and it should be noted this pull request does not rework the
Transactiontests to be data driven. That is still to be done.Comparison link: https://github.com/dcousens/bitcoinjs-lib/compare/tests...txclean