Transaction: WIP, rename internal variables

[dcousens]

This is just a WIP PR, but it should show case a change I'd like to make to the internals of Transaction.

Assuming most people are using TransacitonBuilder, these changes should be no less disruptive then what has already happened.

This would make the variable naming convention consistent with common-blockchain and most naming conventions for bitcoin transactions.

Open to feedback.

[weilu]

Still a breaking change though as we did not remove Transaction from index export.

[dcousens]

No doubt. Added the tag.

[dcousens]

@weilu are you in favour though?

[weilu]

We already have a lot of breaking changes in 2.0. The argument can go either way based on that. Hell, let's just get it over and done with. Can you promise this is the last one though? ;)

[dcousens]

@weilu after #444 and this, the only remaining breaking change is adhering ECPair to #407.

[dcousens]

Will make this change for 3.0.0, 2.0.0 is disruptive enough.

[dcousens]

Other thoughts:

block.timestamp -> block.time for consistency with bitcoin/bitcoin RPC.

[weilu]

Quite a bike shed. Just curious, why are we changing bitcoinjs to be consistent with common-blockchain not the other way round? TBH some of our current names make more sense to me (e.g. hash: of course it's the transaction hash since it's the property of transaction inputs', index vs vout: what does v stand for again?)

If we want to be exactly the same as bitcoin/bitcoin RPC, whatevs, doesn't bother me much.

[dcousens]

Quite a bike shed. Just curious, why are we changing bitcoinjs to be consistent with common-blockchain not the other way round?

The common-blockchain naming conventions require less context, compared to hash and index, but its more that they had more thought going into their naming during that design than these did (as these are legacy names).
txHash as a convention is asimilar to saying txId for a transaction, instead of just a transactions `id.

s/hash/txHash is probably not worthwhile.
s/index/vout is a very common bitcoin convention and makes reasoning about it very easy. Definitely worthwhile IMHO.

IMHO, I'd be happy if we just went with s/index/vout, s/timestamp/time and s/locktime/nLockTime as the only changes in this PR, if you feel the others are less meaningful.

[dcousens]

Though even nLockTime is ambiguous in light of #507. Maybe just lockTime?

[dcousens]

@fanatid thoughts on this?

[fanatid]

@dcousens I'd like see names like in bitcoin-core/bitcoin-core rpc/BIPs.

[dcousens]

@fanatid what were they again? I use custom RPC calls throughout my stack. In any case, they don't have the JSON/Buffer confusion issue that we have to deal with, therefore I don't think it's relevant.
txId is the colloquial name for the reversed-byte-order hash of a transaction, where we use txHash for the regular-byte-order hash.

vout and scriptSig would ~match the RPC IIRC.

[dcousens]

block.timestamp -> block.time for consistency with bitcoin/bitcoin RPC.

Seems to have consensus.

vout and nLockTime too.

About it.

[dcousens]

The only reasonable changes here would be .vout and maybe block.time.
In terms of a breaking change, I can only see .vout being justifiable just due to how prolific it is and how often I get caught out by it.