Start refactoring txid -> txhash
AbandonedPublic
Actions

Authored by tomtomtom7 on Nov 13 2017, 13:47.

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Summary

Introduces the TxHash type, and starts refactoring txid to txhash. This will be spread over multiple commits.

This is the basis of MalFix with naming txhash/txid.

It is quite tricky to ensure no txid is kept where txhash is meant.
As txid will be rare, I propose for safety to fully rename txid to
txhash, eliminating txid usage before reintroducing txid to COutpoint.

Test Plan

Refactoring only

Diff Detail

Repository

rABC Bitcoin ABC

Branch

master

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1097
Build 1097: arc lint + arc unit

Event Timeline

tomtomtom7 created this revision.Nov 13 2017, 13:47

Owners added a reviewer: Restricted Owners Package.Nov 13 2017, 13:47

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 13 2017, 13:47

I would like to note that I still have reservations to this solution compared to the SIGHASH solution. The risk of changing the meaning of txid must be carefully considered.

deadalnix added a subscriber: deadalnix.Nov 13 2017, 15:42

deadalnix added inline comments.

src/primitives/transaction.h
29	It is not correct to use the TxHash in the outpoint. It should be an TxId , to identify the transaction we are spending from. The whole problem we are trying to solve here is that the TxHash is mutable. We should have TxId and TxHash to be the same thing at first, but if we ensure that the right one is used at the right place with the help of the type system, then it'll be easy to differentiate them in the future.

tomtomtom7 added a child revision: D670: Refactoring GetId() to GetHash().Nov 13 2017, 16:22

deadalnix mentioned this in D755: Start differenciating txid and txhash.Dec 5 2017, 16:46

because this seemed to be stuck, I took the initiative to unstuck it via D755 .

This revision now requires changes to proceed.Dec 5 2017, 16:48

deadalnix mentioned this in rABCf71bd4dab0e5: Start differenciating txid and txhash.Dec 6 2017, 13:00

tomtomtom7 abandoned this revision.Dec 7 2017, 09:27

tomtomtom7 added inline comments.

src/primitives/transaction.h
29	Yes. But see comment: It is not going to be easy to do the full refactoring carefully. There is quite a risk of leaving txid's that should be txhashes. Hence I propose, to first fully refactor txid to txhash. (which corresponds to the current rules). Afterwards change txhash to txid for Outpoints. This is my assessment from initial investigation for the full refactor at D614. Is that acceptable?