TxId and uint256 to represent merkle hashes are used interchangeably, causing confusion. This diff fixes that for the TxId-side of things. It may be worth revisiting to strongly type merkle hashes as well.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- txid
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 2739 Build 3589: Bitcoin ABC Buildbot (legacy) Build 3588: arc lint + arc unit
Event Timeline
Comment Actions
This does not seem right. What will happen if the TxID is internally changed to something other than the transaction hash?
Comment Actions
In either case, changes will need to be made in this file to accommodate a change in TxId. This diff makes it explicit which variables are TxId vs generic merkle hashes. Note that I didn't change all uint256's (see CalcHash, for example). The way I saw it, is this code was not clear at what point a TxId becomes a generic hash. I was hoping to make that explicit. As I pointed out in the diff summary, it may be worth making the merkle hashes strongly typed as well in order to further enforce that distinction, though I see that as less valuable.
Comment Actions
My general point is that *currently* the MerkleTree is defined to be calculated from the transaction hashes, not the transaction IDs. (even though they are currently the same value)
Comment Actions
Jason, The types for TxID and TxHash were created to separate the two concepts.
Merkle tree stuff it should use TxHash
The two types were created as a pre-cursor to MalFix, for now they are identical, but could be different in the future.
https://reviews.bitcoinabc.org/source/bitcoin-abc/browse/master/src/primitives/transaction.h$16
Comment Actions
After some discussion offline, we came to the conclusion that this change may not be accurate, but it's hard to know at this point in time:
deadalnix [4:44 PM] It's not clear if TxId or TxHash should be used in CMerkleBlock because we did not decided on a comitment scheme in case we fix malleability
Abandoning this revision and will probably rewrite it in the far future.