Page MenuHomePhabricator

Start differenciating txid and txhash
ClosedPublic

Authored by deadalnix on Dec 5 2017, 16:46.

Details

Summary

Introduces the YxId and TxHash types. This will ensure type safety for cases requiring to use an id and cases requiring to use a hash.

To quote D665 :
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
make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Dec 5 2017, 16:46
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 5 2017, 16:46
jasonbcox accepted this revision.Dec 5 2017, 18:21
jasonbcox added a subscriber: jasonbcox.

LGTM

dagurval accepted this revision.Dec 5 2017, 20:19
This revision is now accepted and ready to land.Dec 5 2017, 20:19
This revision was automatically updated to reflect the committed changes.

I am sorry for leaving this on table, but I must say I am bit disappointed about the procedure here.

I am aware some people like this idea, but don't we actually need support and agreement?

MalFix is a much more high impact change than the other things on ABC's and BU's midterm plan but it isn't there in the announcement; yet we're just going to put it in? Am I the only one who thinks that that is a bit odd?

I agree it seems premature. I'm not convinced it is solving a problem anyone has; should be low priority IMO until it's shown to be a) not useful in itself, and b) blocking other useful stuff people actually are asking to do. I've seen no evidence of either.